From b085dcd5dd2846a173781ae69c3b1118bc40832a Mon Sep 17 00:00:00 2001 From: ispyisail Date: Thu, 15 Oct 2026 21:42:43 +1300 Subject: [PATCH] Pair terminals moved by one shift, and terminals sharing a place The first pass refused a pair whenever a saved terminal had two live candidates in reach. That happens when a whole drawing moves by half the spacing of its terminals (pins 10 apart moved 5), and when two terminals share a place. When one offset fits every unmatched terminal, as when a whole drawing moved, that offset now settles all pairs; terminals sharing a place pair in order. Co-Authored-By: Claude Opus 5.5 --- sources/qetgraphicsitem/element.cpp | 89 +++++++++++++++++++-- tests/qttest/tst_movedterminals.cpp | 117 +++++++++++++++++++++++++++- 2 files changed, 198 insertions(+), 8 deletions(-) diff --git a/sources/qetgraphicsitem/element.cpp b/sources/qetgraphicsitem/element.cpp index 6e1f0b0d7..91fc96b92 100644 --- a/sources/qetgraphicsitem/element.cpp +++ b/sources/qetgraphicsitem/element.cpp @@ -980,8 +980,15 @@ bool Element::valideXml(QDomElement &e) replaced or redrawn since the project was saved. A saved record gives the old docking point and orientation of its terminal, so it is paired with a live terminal of the same orientation whose docking point is - within moved_terminal_reach in x and y, when each is the only candidate - of the other. Anything less clear stays unmatched, as before. + within moved_terminal_reach in x and y. Two steps, both only where the + pairing is clear: + - a rigid offset: if exactly one offset (the same for every terminal) + maps every unmatched record onto a distinct live terminal, as when a + whole drawing was moved, all are paired by it; + - otherwise per terminal: a record and a live terminal that are each + the only candidate of the other. Several records at one place and as + many live terminals at one place are paired in order. + Anything less clear stays unmatched, as before. A pair registers the record's legacy id, and the uuid TerminalUuids derived for the terminal at its old place, so that a wire naming either @@ -1021,10 +1028,10 @@ void Element::matchMovedTerminals(const QList &records, } } - for (auto it = candidates.constBegin(); it != candidates.constEnd(); ++it) { - Terminal *t = it.value().value(0); - if (it.value().size() != 1 || claimants.value(t).size() != 1) continue; - const QDomElement &record = records.at(it.key()); + //Pair a record with a live terminal: register its legacy id, and the + //uuid derived for the terminal at its old place + const auto pair_up = [&](int index, Terminal *t) { + const QDomElement &record = records.at(index); const QPointF dock = dock_of(record); //The old place of the terminal: its docking point less the @@ -1039,7 +1046,7 @@ void Element::matchMovedTerminals(const QList &records, //The rank among the records at the same place, as the project //counted it when it gave the terminals their uuids int rank = 0; - for (int j = 0; j < it.key(); ++j) { + for (int j = 0; j < index; ++j) { const QPointF other = dock_of(records.at(j)); if (orientation_of(records.at(j)) == orientation_of(record) && qAbs(other.x() - dock.x()) < 1e-6 @@ -1051,6 +1058,74 @@ void Element::matchMovedTerminals(const QList &records, m_old_place_terminals.insert(TerminalUuids::derived(old_pos.x(), old_pos.y(), orientation_of(record), rank), t); m_moved_terminals << t; + }; + const auto same_place = [](const QPointF &a, const QPointF &b) { + return qAbs(a.x() - b.x()) < 1e-6 && qAbs(a.y() - b.y()) < 1e-6; + }; + + //Rigid offset: when a whole drawing was moved, every terminal moved + //by the same amount. Try each offset seen between a record and a + //live terminal; if exactly one maps every unmatched record onto a + //distinct live terminal, the pairs are settled by it. + QList unmatched; + QList offsets; + for (int i = 0; i < records.size(); ++i) { + if (matched_records.contains(i)) continue; + unmatched << i; + for (Terminal *t : candidates.value(i)) { + const QPointF offset = t->dock_elmt_ - dock_of(records.at(i)); + const bool known = std::any_of(offsets.cbegin(), offsets.cend(), + [&](const QPointF &o) { return same_place(o, offset); }); + if (!known) offsets << offset; + } + } + QList> rigid; + int rigid_offsets = 0; + for (const QPointF &offset : std::as_const(offsets)) { + QList> mapping; + QSet used; + for (int i : std::as_const(unmatched)) { + Terminal *found = nullptr; // records at one place take the live ones in order + for (Terminal *t : candidates.value(i)) { + if (!used.contains(t) + && same_place(t->dock_elmt_, dock_of(records.at(i)) + offset)) { + found = t; + break; + } + } + if (!found) break; + used << found; + mapping.append({i, found}); + } + if (mapping.size() != unmatched.size()) continue; + ++rigid_offsets; + rigid = mapping; + } + if (rigid_offsets == 1) { + for (const auto &pair : std::as_const(rigid)) pair_up(pair.first, pair.second); + return; + } + + //Per terminal: a record and a live terminal that are each the only + //candidate of the other. Several records at one place with as many + //live terminals at one place pair in order. + for (int i : std::as_const(unmatched)) { + const QList live = candidates.value(i); + if (live.isEmpty()) continue; + QList group; + for (int j : std::as_const(unmatched)) { + if (orientation_of(records.at(j)) == orientation_of(records.at(i)) + && same_place(dock_of(records.at(j)), dock_of(records.at(i)))) + group << j; + } + if (group.first() != i || live.size() != group.size()) continue; + bool clear = true; + for (Terminal *t : live) { + if (!same_place(t->dock_elmt_, live.first()->dock_elmt_) + || claimants.value(t) != group) clear = false; + } + if (!clear) continue; + for (int k = 0; k < group.size(); ++k) pair_up(group.at(k), live.at(k)); } } diff --git a/tests/qttest/tst_movedterminals.cpp b/tests/qttest/tst_movedterminals.cpp index 77f5cff71..1f20a304e 100644 --- a/tests/qttest/tst_movedterminals.cpp +++ b/tests/qttest/tst_movedterminals.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -86,6 +87,65 @@ QString byUuid(int old_south_y) .arg(uuid_a, derived(old_south_y, 2), uuid_b, derived(-10, 0)); } + +// A symbol with any terminals, for the cases of several terminals moved at +// once. A terminal is its orientation letter and its position. +struct Term { QChar o; int x; int y; }; + +QString multiDefinition(const QList &terms) +{ + QString out; + for (const Term &t : terms) + out += QStringLiteral("").arg(t.o).arg(t.x).arg(t.y); + QString d = definition({}); + // the lone north terminal of definition() is replaced + d.replace(QRegularExpression(QStringLiteral("]*/>")), QString()); + return d.replace(QStringLiteral(""), out + QStringLiteral("")); +} + +// A placed symbol whose terminals were saved at the given positions (ids +// first_id, first_id + 1, ...). The saved docking point is the position less +// the stub of 4: north y+4, south y-4, east x-4, west x+4. +QString multiPlaced(const QString &uuid, int first_id, int x, const QList &saved) +{ + QString out; + int id = first_id; + for (const Term &t : saved) { + int dx = t.x, dy = t.y, o = 0; + switch (t.o.toLatin1()) { + case 'n': dy += 4; o = 0; break; + case 'e': dx -= 4; o = 1; break; + case 's': dy -= 4; o = 2; break; + default : dx += 4; o = 3; break; + } + out += QStringLiteral("") + .arg(o).arg(dx).arg(dy).arg(id++); + } + return QStringLiteral( + "%3") + .arg(uuid).arg(x).arg(out); +} + +// Two copies of the symbol saved with terminals `saved`, now defined with +// `now`; one wire joins each terminal of the first copy to the same terminal +// of the second (by legacy id). +QString multiProject(const QList &saved, const QList &now) +{ + QString wires; + for (int i = 0; i < saved.size(); ++i) + wires += QStringLiteral("") + .arg(i).arg(saved.size() + i); + return QStringLiteral( + "" + "" + "%1%2%3%4") + .arg(multiPlaced(uuid_a, 0, 100, saved), + multiPlaced(uuid_b, saved.size(), 300, saved), + wires, multiDefinition(now)); +} + } // namespace class tst_movedterminals : public QObject @@ -95,7 +155,7 @@ class tst_movedterminals : public QObject QTemporaryDir m_dir; int m_run = 0; - struct Result { int wires = -1; QString log; }; + struct Result { int wires = -1; QString log; QStringList pairs; }; // Open and save the project in a sandbox of its own (so a running // QElectroTech cannot answer instead): the wires that survive. @@ -125,6 +185,10 @@ class tst_movedterminals : public QObject QFile saved(out); if (!saved.open(QIODevice::ReadOnly) || !doc.setContent(&saved)) return {}; r.wires = doc.elementsByTagName(QStringLiteral("conductor")).size(); + for (int i = 0; i < r.wires; ++i) { + const QDomElement c = doc.elementsByTagName(QStringLiteral("conductor")).at(i).toElement(); + r.pairs << c.attribute(QStringLiteral("terminal1")) + QLatin1Char('>') + c.attribute(QStringLiteral("terminal2")); + } return r; } @@ -184,6 +248,57 @@ private slots: QVERIFY(!said(legacy)); QCOMPARE(resave(project({13, 14}, byUuid(10))).wires, 0); } + + // A whole drawing moved by 5: two terminals 10 apart, so each saved + // one has two live terminals at 5 units. Only one shift fits both. + void wholeDrawingMovedPairsBothTerminals() + { + const Result r = resave(multiProject({{'n', -5, -10}, {'n', 5, -10}}, + {{'n', -10, -10}, {'n', 0, -10}})); + QCOMPARE(r.wires, 2); + QVERIFY(said(r)); + } + + // Two terminals at one place, moved by 1: paired in order. + void terminalsAtOnePlaceKeepTheirOrder() + { + const QList saved = {{'n', 1, -40}, {'n', 1, -40}}; + const Result r = resave(multiProject(saved, {{'n', 0, -40}, {'n', 0, -40}})); + QCOMPARE(r.wires, 2); + QVERIFY(said(r)); + // each wire runs between the same terminal of both copies, and the + // first saved one took the first live one (the rank-0 uuid) + QCOMPARE(r.pairs.size(), 2); + QCOMPARE(r.pairs.at(0), derived(-40, 0) + QLatin1Char('>') + derived(-40, 0)); + QVERIFY(r.pairs.at(1) != r.pairs.at(0)); + const QStringList second = r.pairs.at(1).split(QLatin1Char('>')); + QCOMPARE(second.at(0), second.at(1)); + } + + // Terminals that moved by different amounts have no common shift: + // each is still paired by itself. + void differentShiftsStillPairPerTerminal() + { + const Result r = resave(multiProject({{'n', -20, -10}, {'n', 20, -10}}, + {{'n', -18, -10}, {'n', 23, -10}})); + QCOMPARE(r.wires, 2); + } + + // A terminal that moved while another did not leaves a lone saved + // terminal: no rigid shift is needed, the per-terminal rule pairs it. + void oneMovedAmongStaysStillPairs() + { + QCOMPARE(resave(multiProject({{'n', -20, -10}, {'n', 20, -10}}, + {{'n', -20, -10}, {'n', 23, -10}})).wires, 2); + } + + // Still ambiguous: the two saved terminals could each be either live + // one, and no single shift maps both. + void ambiguousStillRefuses() + { + QCOMPARE(resave(multiProject({{'n', 0, -10}, {'n', 4, -10}}, + {{'n', 2, -10}, {'n', 6, -10}, {'n', -2, -10}})).wires, 0); + } }; QTEST_APPLESS_MAIN(tst_movedterminals)