From 149ae961cc76632a1dae2b41efa912556a37d121 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 25 Sep 2026 21:16:34 +1200 Subject: [PATCH] Fix bugtracker #343: saving reorders texts and shapes after an edit Diagram::toXml() writes texts, images, shapes and tables in items() order. The diagram scene uses NoIndex, and Qt's linear index sorts its item list by pointer address the first time any item is removed from the scene, which the editor does constantly (selection handles, for one). From then on the order in the saved file follows memory addresses, so moving one element reshuffles unrelated blocks and a version-control diff of the project becomes unreadable. Write those blocks in stacking order instead, read from a rect query, which Qt sorts by z and insertion order even with NoIndex. Reloading a file rebuilds exactly that order, so a resave is stable and the drawing does not change. Elements and conductors were already sorted. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G2d2Zi8BfrYRPX88zhaoFG --- sources/diagram.cpp | 82 ++++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 29604b910..01a6e63d2 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -42,7 +42,9 @@ #include "qetinformation.h" #include "qetproject.h" #include "diagramsortkeys.h" +#include #include +#include #include #include @@ -99,6 +101,43 @@ namespace { QString b = terminalSortKey(cond->terminal2); return (a <= b) ? (a + QLatin1Char('>') + b) : (b + QLatin1Char('>') + a); } + + /// Serialize @p items and append them to a new @p tag block of @p root, + /// in stacking order (@p stack_rank). items() cannot be trusted for + /// this: with NoIndex, the first removeItem() on the scene sorts Qt's + /// item list by pointer address, so from then on items() hands them over + /// in a per-run order (bugtracker #343). Stacking order is what reloading + /// the file rebuilds, so the drawing is unchanged and a resave is stable. + template + void appendInStackingOrder(QDomDocument &document, QDomElement &root, + const QString &tag, const QVector &items, + const QHash &stack_rank) + { + if (items.isEmpty()) + return; + struct Entry { int rank; QString xml_text; QDomElement xml; }; + QVector sorted; + for (T *item : items) { + Entry entry{stack_rank.value(item, INT_MAX), QString(), + item->toXml(document)}; + // Only an item the stacking query missed needs a tiebreak. + if (entry.rank == INT_MAX) { + QTextStream stream(&entry.xml_text); + entry.xml.save(stream, 0); + } + sorted.append(entry); + } + std::stable_sort(sorted.begin(), sorted.end(), + [](const Entry &a, const Entry &b) { + return a.rank != b.rank ? a.rank < b.rank + : a.xml_text < b.xml_text; + }); + + auto block = document.createElement(tag); + for (const auto &entry : sorted) + block.appendChild(entry.xml); + root.appendChild(block); + } } int Diagram::xGrid = 10; @@ -1223,37 +1262,20 @@ QDomDocument Diagram::toXml(bool whole_content, bool is_copy_command) { dom_root.appendChild(dom_conductors); } - if (!list_texts.isEmpty()) { - auto dom_texts = document.createElement(QStringLiteral("inputs")); - for (auto dti : list_texts) { - dom_texts.appendChild(dti->toXml(document)); - } - dom_root.appendChild(dom_texts); - } - - if (!list_images.isEmpty()) { - auto dom_images = document.createElement(QStringLiteral("images")); - for (auto dii : list_images) { - dom_images.appendChild(dii->toXml(document)); - } - dom_root.appendChild(dom_images); - } - - if (!list_shapes.isEmpty()) { - auto dom_shapes = document.createElement(QStringLiteral("shapes")); - for (auto dii : list_shapes) { - dom_shapes.appendChild(dii -> toXml(document)); - } - dom_root.appendChild(dom_shapes); - } - - if (table_vector.size()) { - auto tables = document.createElement(QStringLiteral("tables")); - for (auto table : table_vector) { - tables.appendChild(table->toXml(document)); - } - dom_root.appendChild(tables); + // A rect query, unlike items(), returns true stacking order (z, then + // insertion order) even with NoIndex. + QHash stack_rank; + { + const QList stacked = items( + QRectF(-1e9, -1e9, 2e9, 2e9), Qt::IntersectsItemBoundingRect, + Qt::AscendingOrder); + for (int i = 0 ; i < stacked.size() ; ++i) + stack_rank.insert(stacked.at(i), i); } + appendInStackingOrder(document, dom_root, QStringLiteral("inputs"), list_texts, stack_rank); + appendInStackingOrder(document, dom_root, QStringLiteral("images"), list_images, stack_rank); + appendInStackingOrder(document, dom_root, QStringLiteral("shapes"), list_shapes, stack_rank); + appendInStackingOrder(document, dom_root, QStringLiteral("tables"), table_vector, stack_rank); if (!strip_vector.isEmpty()) { dom_root.appendChild(TerminalStripItemXml::toXml(strip_vector, document));