diff --git a/sources/diagram.cpp b/sources/diagram.cpp index cf552d14a..271e0713f 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -40,9 +40,59 @@ #include "undocommand/addelementtextcommand.h" #include "qetinformation.h" #include "qetproject.h" +#include #include #include +namespace { + /// Format a position as a string that sorts the same way the numbers do + /// (fixed precision, so "-" and decimals compare correctly as text). + QString positionKey(const QPointF &pos) + { + return QStringLiteral("%1|%2") + .arg(pos.x(), 0, 'f', 4) + .arg(pos.y(), 0, 'f', 4); + } + + /// Sort key for Diagram::toXml()'s block: the element's own + /// diagram-local position, exactly what it's already saved as (x/y), + /// never invented or regenerated. uuid() is deliberately NOT used here: + /// for an element with no persisted uuid attribute, fromXml() invents a + /// fresh random one on every load, so sorting by uuid would still be + /// non-deterministic across process runs for any legacy file. + QString elementSortKey(Element *elmt) + { + return positionKey(elmt->pos()); + } + + /// Sort key for a terminal: its parent element's position, then the + /// terminal's own position local to that element (from the .elmt + /// definition, fixed regardless of where the element is placed). + QString terminalSortKey(Terminal *terminal) + { + if (!terminal) + return QString(); + Element *parent = terminal->parentElement(); + return (parent ? positionKey(parent->pos()) : QStringLiteral("?")) + + QLatin1Char(':') + positionKey(terminal->pos()); + } + + /// Sort key for Diagram::toXml()'s block. Built from both + /// endpoints' terminalSortKey(), not Conductor::uuid(): in every example + /// project checked, conductors have no persisted uuid attribute at all, + /// so uuid() is a freshly-minted random value on every load -- exactly + /// as unusable for cross-run determinism as the element case above, just + /// with no persisted fallback to reach for instead. Canonicalised + /// (smaller key first) since a conductor's two ends are unordered for + /// this purpose. + QString conductorSortKey(Conductor *cond) + { + QString a = terminalSortKey(cond->terminal1); + QString b = terminalSortKey(cond->terminal2); + return (a <= b) ? (a + QLatin1Char('>') + b) : (b + QLatin1Char('>') + a); + } +} + int Diagram::xGrid = 10; int Diagram::yGrid = 10; int Diagram::xKeyGrid = 10; @@ -1014,6 +1064,20 @@ QDomDocument Diagram::toXml(bool whole_content, bool is_copy_command) { } } + // items() returns items in stacking order, which is not guaranteed + // reproducible across processes (ties between same-Z items follow + // the scene's internal index, not any content-derived order) -- so + // without this, saving an unmodified project produces a different + // byte stream on every run. Elements and conductors are the two + // blocks observed to actually churn across the example corpus; + // sort them into a deterministic, content-derived order before + // serializing. This also fixes the legacy terminal-id churn below, + // since those ids are assigned sequentially in element order. + std::stable_sort(list_elements.begin(), list_elements.end(), + [](Element *a, Element *b) { return elementSortKey(a) < elementSortKey(b); }); + std::stable_sort(list_conductors.begin(), list_conductors.end(), + [](Conductor *a, Conductor *b) { return conductorSortKey(a) < conductorSortKey(b); }); + // correspondence table between the addresses of the terminals and their ids // table de correspondance entre les adresses des bornes et leurs ids QHash table_adr_id; diff --git a/sources/qetgraphicsitem/conductor.cpp b/sources/qetgraphicsitem/conductor.cpp index 4f538e91f..5711b5d1f 100644 --- a/sources/qetgraphicsitem/conductor.cpp +++ b/sources/qetgraphicsitem/conductor.cpp @@ -1011,10 +1011,14 @@ bool Conductor::fromXml(QDomElement &dom_element) //that field was introduced (see terminal1/terminal2 handling in //toXml() below). m_uuid = QUuid(dom_element.attribute(QStringLiteral("uuid"))); + m_persist_uuid = !m_uuid.isNull(); if (m_uuid.isNull()) { //Absent, empty or malformed: mint one. A null uuid is not a usable //identity -- every conductor carrying one would collide with every - //other on the conductor table's primary key. + //other on the conductor table's primary key. It's runtime-only, + //though: toXml() must not write it back out, or a legacy file + //with no conductor uuids gets a freshly different one on every + //single load-and-resave (see #754). m_uuid = QUuid::createUuid(); } @@ -1056,7 +1060,8 @@ QDomElement Conductor::toXml(QDomDocument &dom_document, { QDomElement dom_element = dom_document.createElement("conductor"); - dom_element.setAttribute("uuid", m_uuid.toString()); + if (m_persist_uuid) + dom_element.setAttribute("uuid", m_uuid.toString()); dom_element.setAttribute("x", QString::number(pos().x())); dom_element.setAttribute("y", QString::number(pos().y())); diff --git a/sources/qetgraphicsitem/conductor.h b/sources/qetgraphicsitem/conductor.h index 91ca92afb..63857e068 100644 --- a/sources/qetgraphicsitem/conductor.h +++ b/sources/qetgraphicsitem/conductor.h @@ -79,7 +79,7 @@ class Conductor : public QGraphicsObject Diagram *diagram() const; ConductorTextItem *textItem() const; QUuid uuid() const {return m_uuid;} - void newUuid() {m_uuid = QUuid::createUuid();} //create new uuid for this conductor + void newUuid() {m_uuid = QUuid::createUuid(); m_persist_uuid = true;} //create new uuid for this conductor void updatePath(const QRectF & = QRectF()); //This method do nothing, it's only made to be used with Q_PROPERTY @@ -209,6 +209,10 @@ class Conductor : public QGraphicsObject bool m_valid; bool m_freeze_label = false; QUuid m_uuid; + /// false when m_uuid was synthesized by fromXml() because the + /// file had none -- toXml() must not persist that value, or + /// every reload mints and saves a new random one (see #754). + bool m_persist_uuid = true; /// QPen et QBrush objects used to draw conductors static QPen conductor_pen;