diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 7bde5c1f9..ab0c504f7 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -1315,10 +1315,19 @@ QDomDocument Diagram::toXml(bool whole_content, bool is_copy_command) { } if (!list_conductors.isEmpty()) { + //Symbols copied in old versions can share one uuid. A wire + //saved by that uuid would reopen on the first of them (#1408). + QSet seen_uuids, shared_uuids; + for (auto elmt : std::as_const(list_elements)) { + if (seen_uuids.contains(elmt->uuid())) + shared_uuids.insert(elmt->uuid()); + seen_uuids.insert(elmt->uuid()); + } auto dom_conductors = document.createElement(QStringLiteral("conductors")); for (auto cond : list_conductors) { dom_conductors.appendChild(cond->toXml(document, - table_adr_id)); + table_adr_id, + shared_uuids)); } dom_root.appendChild(dom_conductors); } @@ -1464,18 +1473,20 @@ bool Diagram::initFromXml(QDomElement &document, } /** - @brief findTerminal - Find terminal to which the conductor should be connected + @brief findTerminals + Find the terminals to which the conductor could be connected @param conductor_index 1 or 2 depending on which terminal is searched @param f Conductor xml element @param table_adr_id Hash table to all terminal id assignement (legacy) @param added_elements Elements found in the xml file - @return + @return the terminal, or one per symbol when several symbols of the + folio carry the uuid the wire names (old copies could share one, #1408); + empty if none is found */ -Terminal* findTerminal(int conductor_index, - QDomElement& f, - QHash& table_adr_id, - QList& added_elements) +QList findTerminals(int conductor_index, + QDomElement& f, + QHash& table_adr_id, + QList& added_elements) { assert(conductor_index == 1 || conductor_index == 2); @@ -1483,39 +1494,45 @@ Terminal* findTerminal(int conductor_index, QString element_index = QStringLiteral("element") + str_index; QString terminal_index = QStringLiteral("terminal") + str_index; + QList found; if (f.hasAttribute(element_index)) { QUuid element_uuid = QUuid(f.attribute(element_index)); // element1 did not exist in the conductor part of the xml until prior 0.7 // It is used as an indicator that uuid's are used to identify terminals bool element_found = false; + QUuid terminal_uuid = QUuid(f.attribute(terminal_index)); for (auto element: added_elements) { if (element->uuid() != element_uuid) continue; element_found = true; - QUuid terminal_uuid = QUuid(f.attribute(terminal_index)); + Terminal *match = nullptr; for (auto terminal: element->terminals()) { - if (terminal->uuid() != terminal_uuid) - continue; - - return terminal; + if (terminal->uuid() == terminal_uuid) { + match = terminal; + break; + } } //The uuid a project gave a terminal on opening is worked out //from where the terminal is in its symbol: if the symbol's //definition has since been replaced by one whose terminals //carry other uuids, the terminal at that place is still it. for (auto terminal: element->terminals()) { + if (match) + break; if (terminal->derivedUuid() == terminal_uuid) - return terminal; + match = terminal; } - qDebug() << "Diagram::fromXml() : " - << terminal_index - << ":" - << terminal_uuid - << "not found in " - << element_index - << ":" - << element_uuid; - break; + if (match) + found << match; + else + qDebug() << "Diagram::fromXml() : " + << terminal_index + << ":" + << terminal_uuid + << "not found in " + << element_index + << ":" + << element_uuid; } if (!element_found) qDebug() << "Diagram::fromXml() : " @@ -1532,9 +1549,83 @@ Terminal* findTerminal(int conductor_index, << id_p1 << " not found"; } else - return table_adr_id.value(id_p1); + found << table_adr_id.value(id_p1); } - return nullptr; + return found; +} + +/** + @brief pickEnds + Choose the two ends of a wire when its uuids name several symbols of + the folio (#1408). The pair whose distance matches the wire's saved + path wins, then the symbols whose labels match the ones saved with the + wire, then two terminals no wire joins yet; on a tie, the first symbols + in the file, as before. + @param f Conductor xml element + @param ends1 candidates for the first end, from findTerminals() + @param ends2 candidates for the second end + @return the two terminals, or nullptr where a list is empty +*/ +QPair pickEnds(const QDomElement &f, + const QList &ends1, + const QList &ends2) +{ + if ((ends1.size() <= 1 && ends2.size() <= 1) + || ends1.isEmpty() || ends2.isEmpty()) + return {ends1.value(0), ends2.value(0)}; + + //The saved path runs from the first end to the second + QPointF path_length; + bool has_path = false; + for (QDomElement segment = f.firstChildElement(QStringLiteral("segment")); + !segment.isNull(); + segment = segment.nextSiblingElement(QStringLiteral("segment"))) { + bool ok = false; + const qreal length = segment.attribute(QStringLiteral("length")).toDouble(&ok); + if (!ok || !qIsFinite(length)) + continue; + has_path = true; + if (segment.attribute(QStringLiteral("orientation")) == QLatin1String("horizontal")) + path_length.rx() += length; + else + path_length.ry() += length; + } + + auto label_matches = [&f](Terminal *terminal, const QString &index) { + const QString label = f.attribute(QStringLiteral("element") + index + + QStringLiteral("_label")); + return !label.isEmpty() + && terminal->parentElement()->actualLabel() == label; + }; + + QPair best; + int best_score = -1; + for (auto t1 : ends1) { + for (auto t2 : ends2) { + if (t1 == t2) + continue; + int score = 0; + if (has_path) { + const QPointF gap = t2->dockConductor() - t1->dockConductor(); + if (qAbs(gap.x() - path_length.x()) <= 1.0 + && qAbs(gap.y() - path_length.y()) <= 1.0) + score += 8; + } + if (label_matches(t1, QStringLiteral("1"))) + score += 2; + if (label_matches(t2, QStringLiteral("2"))) + score += 2; + if (!t1->isLinkedTo(t2)) + score += 1; + if (score > best_score) { + best_score = score; + best = {t1, t2}; + } + } + } + if (!best.first) + return {ends1.first(), ends2.first()}; + return best; } /** @@ -1879,8 +1970,11 @@ bool Diagram::fromXml(QDomElement &document, //Check if terminal that conductor must be linked is know - Terminal* p1 = findTerminal(1, f, table_adr_id, added_elements); - Terminal* p2 = findTerminal(2, f, table_adr_id, added_elements); + const auto ends = pickEnds(f, + findTerminals(1, f, table_adr_id, added_elements), + findTerminals(2, f, table_adr_id, added_elements)); + Terminal* p1 = ends.first; + Terminal* p2 = ends.second; //Keep a trace of the wire, it will be missing from the next save if ((!p1 || !p2) && consider_informations) diff --git a/sources/qetgraphicsitem/conductor.cpp b/sources/qetgraphicsitem/conductor.cpp index afae70051..66fe92dbb 100644 --- a/sources/qetgraphicsitem/conductor.cpp +++ b/sources/qetgraphicsitem/conductor.cpp @@ -1162,11 +1162,16 @@ bool Conductor::fromXml(QDomElement &dom_element) @param table_adr_id : Hash stockant les correspondances entre les ids des bornes dans le document XML et leur adresse en memoire + @param shared_uuids : uuids carried by more than one symbol of the + folio. An end on such a symbol is written by its terminal id, as for a + terminal without uuid: by uuid it would reopen on the first symbol + carrying it (#1408). @return Un element XML representant le conducteur */ QDomElement Conductor::toXml(QDomDocument &dom_document, QHash &table_adr_id) const + int> &table_adr_id, + const QSet &shared_uuids) const { QDomElement dom_element = dom_document.createElement("conductor"); @@ -1176,7 +1181,8 @@ QDomElement Conductor::toXml(QDomDocument &dom_document, dom_element.setAttribute("y", QString::number(pos().y())); // Terminal is uniquely identified by the uuid of the terminal and the element - if (terminal1->uuid().isNull()) { + if (terminal1->uuid().isNull() + || shared_uuids.contains(terminal1->parentElement()->uuid())) { // legacy method to identify the terminal dom_element.setAttribute("terminal1", table_adr_id.value(terminal1)); // for backward compatibility } else { @@ -1192,7 +1198,8 @@ QDomElement Conductor::toXml(QDomDocument &dom_document, dom_element.setAttribute("terminalname1", terminal1->name()); } - if (terminal2->uuid().isNull()) { + if (terminal2->uuid().isNull() + || shared_uuids.contains(terminal2->parentElement()->uuid())) { // legacy method to identify the terminal dom_element.setAttribute("terminal2", table_adr_id.value(terminal2)); // for backward compatibility } else { diff --git a/sources/qetgraphicsitem/conductor.h b/sources/qetgraphicsitem/conductor.h index 387fc9d20..34edfe436 100644 --- a/sources/qetgraphicsitem/conductor.h +++ b/sources/qetgraphicsitem/conductor.h @@ -21,6 +21,7 @@ #include "../conductorproperties.h" #include +#include #include class ConductorProfile; @@ -108,7 +109,8 @@ class Conductor : public QGraphicsObject QDomElement toXml ( QDomDocument &, QHash &) const; + int> &, + const QSet &shared_uuids = QSet()) const; private: bool pathFromXml(const QDomElement &); diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index b33b94130..de3223a1c 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -920,6 +920,18 @@ target_compile_definitions(tst_derivedwireuuid PRIVATE "QET_TEST_BINARY_PATH=\"$\"" "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") +# Symbols sharing one uuid on a folio keep their wires through save and +# reopen (#1408): nets of examples/Habitat-Schemas_developpes.qet. +add_executable( + tst_sharedsymboluuid + tst_sharedsymboluuid.cpp) +add_test(NAME tst_sharedsymboluuid COMMAND tst_sharedsymboluuid) +add_dependencies(tst_sharedsymboluuid qelectrotech) +target_link_libraries(tst_sharedsymboluuid PRIVATE Qt::Test Qt::Xml) +target_compile_definitions(tst_sharedsymboluuid PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # Element numbering schemes carry an id and elements follow them by it: # a legacy file gets stable ids on --resave; through --run, rename, edit, # refused removal and undo/redo keep the elements following the scheme. diff --git a/tests/qttest/fixtures/shared_symbol_uuid_0200.qet b/tests/qttest/fixtures/shared_symbol_uuid_0200.qet new file mode 100644 index 000000000..652c2f0a9 --- /dev/null +++ b/tests/qttest/fixtures/shared_symbol_uuid_0200.qet @@ -0,0 +1,685 @@ + + + + 17/04/2021 + 17-04-2021 + 2021-04-17 + Habitat-Schemas_developpes + C:/Users/VD5385/Documents/Github/qelectrotech-source-mirror/qelectrotech-source-mirror/examples/Habitat-Schemas_developpes.qet + 14:45 + + + + + + + + + + + + + + + + + + + + + + + + + + + + L + + + + L + label + + + + + + + + + + + + + label + + + + + + + + + + + + 16A + + + + 16A + label + + + + + + + + + + + + 20A + + + + 20A + label + + + + + + + + + + + + S1 + + + + S1 + label + + + + + + + + + + + + S2 + + + + S2 + label + + + + + + + + + + + + + PC1 + + + + PC1 + label + + + + + + + + + + + + + L1 + + + + L1 + label + + + + + + + + + + + + + L2 + + + + L2 + label + + + + + + + + + + + + + L3 + + + + L3 + label + + + + + + + + + + + + 10A + + + + 10A + label + + + + + + + + + + + + 20A + + + + 20A + label + + + + + + + + + + + N + + + + N + label + + + + + + + + + + + + + label + + + + + + + + + + + PE + + + + PE + label + + + + + + + + + + + + + label + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Импортированные элементы + Imported elements + Elementi importati + Éléments importés + Elementy importowane + Elementos importados + Zavedené prvky + + + + مصادر + Quellen + Источники + Fontes + Sources + Sorgenti + Sources + Źródła zasilania + Fuentes + Zdroje + + + + متعددة الأسلاك + Mehradrig + Многолинейные + Multifilar + Multiline + Multifilari + Multifilaire + Schemat wielokreskowyowy + Multihilo + Několik vedení + + + + + Нейтраль + Protection Electrique + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + مصدر وجه + Phase + Фаза + Fonte de fase + Phase source + Sorgente fase + Source phase + Przewód liniowy + Fuente fase + Fázový zdroj + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + مصدر محايد + Neutralleiter + Нейтраль + Fonte de neutro + Neutral source + Sorgente neutro + Source neutre + Przewód neutralny + Fuente neutro + Nulový zdroj + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + حمايات + Sicherheit + Защитные уст-ва + Protecções + Protections + Protezioni + Protections + Zabezpieczenia + Protecciones + Ochrany + + + + قواطع + Lastschalter + Выключатели + Disjuntores + Circuit-breakers + Interruttori + Disjoncteurs + Wyłączniki + Disyuntores + Jističe + + + + + قاطع أحدي القطب + Circuit-breaker + Disjoncteur unipolaire + Wyłącznik + Jednopólový jistič + Int. Aut. Magneto-termico 1P + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + + + + + + + + + + + ملامسات + Kontakte + Контакты + Contactos + Contacts + Contatti + Contacts + Zestyki + Terminales + Kontakty + + + + مفاتيح + Schalter + переключателей + Interruptores + Switches + Interruttori + Interrupteurs + Przełącznik + Interruptores + Spínače + + + + + مفتاح + Schalter + Переключатель + Interruptor + Switch + Interruttore + Interrupteur + Łącznik + Interruptor + Spínač + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + + + + + Point Lumineux + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + + + + + + + قاطع + Lastschalter + Выключатель + Disjuntor + Circuit-breaker + Sezionatore 1P + Disjoncteur + Wyłącznik + Disyuntor + Jistič + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + + + + + Нейтраль + Protection Electrique + + Author: The QElectroTech team +License: see http://qelectrotech.org/wiki/doc/elements_license + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/qttest/tst_sharedsymboluuid.cpp b/tests/qttest/tst_sharedsymboluuid.cpp new file mode 100644 index 000000000..85ba9bf95 --- /dev/null +++ b/tests/qttest/tst_sharedsymboluuid.cpp @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// Symbols copied in old versions can share one uuid on a folio. Since every +// terminal got a uuid (#1118), the wires on them were saved by symbol uuid +// and reopened on the first symbol carrying it (#1408). +// +// Runs the real binary on examples/Habitat-Schemas_developpes.qet, whose +// folio 1 has three lamps L1, L2, L3, and compares the nets QElectroTech +// exports with those of the unedited example: +// - the lamps given one uuid, saved twice; +// - fixtures/shared_symbol_uuid_0200.qet: that folio as a build with #1118 +// saved it, its wires naming the lamps by the shared uuid. +namespace { + +const QString example = QStringLiteral(QET_EXAMPLES_DIR "/Habitat-Schemas_developpes.qet"); + +QDomDocument load(const QString &path) +{ + QDomDocument doc; + QFile file(path); + if (file.open(QIODevice::ReadOnly)) + doc.setContent(&file); + return doc; +} + +QDomElement firstDiagram(const QDomDocument &doc) +{ + return doc.documentElement().firstChildElement(QStringLiteral("diagram")); +} + +QList childElements(const QDomElement &diagram, const QString &block, + const QString &tag) +{ + QList out; + for (QDomElement e = diagram.firstChildElement(block).firstChildElement(tag); + !e.isNull(); e = e.nextSiblingElement(tag)) + out << e; + return out; +} + +QList lamps(const QDomDocument &doc) +{ + QList out; + for (const QDomElement &e : childElements(firstDiagram(doc), QStringLiteral("elements"), + QStringLiteral("element"))) + if (e.attribute(QStringLiteral("type")).endsWith(QStringLiteral("lampe_pe.elmt"))) + out << e; + return out; +} + +} // namespace + +class tst_sharedsymboluuid : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + int m_run = 0; + + // Run the binary in a sandbox of its own, so a running QElectroTech + // cannot answer instead. + bool run(const QStringList &args) + { + const QString home = m_dir.filePath(QStringLiteral("home%1").arg(m_run++)); + QDir().mkpath(home); + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data")); + QProcess proc; + proc.setProcessEnvironment(env); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), args); + return proc.waitForFinished(120000) && proc.exitCode() == 0; + } + + QString write(const QDomDocument &doc) + { + const QString path = m_dir.filePath(QStringLiteral("in%1.qet").arg(m_run)); + QFile f(path); + if (f.open(QIODevice::WriteOnly)) + f.write(doc.toByteArray()); + return path; + } + + QString resave(const QString &in) + { + const QString out = m_dir.filePath(QStringLiteral("out%1.qet").arg(m_run)); + return run({QStringLiteral("--resave"), in, out}) ? out : QString(); + } + + // Folio 1's nets, each as its sorted symbol labels, sorted: the order + // the export numbers them in follows the file and does not matter. + QStringList nets(const QString &project) + { + const QString out = m_dir.filePath(QStringLiteral("nets%1.json").arg(m_run)); + if (!run({QStringLiteral("--export-nets"), project, out})) + return {}; + QFile f(out); + if (!f.open(QIODevice::ReadOnly)) + return {}; + QStringList result; + const QJsonArray list = QJsonDocument::fromJson(f.readAll()) + .object().value(QStringLiteral("list")).toArray(); + for (const QJsonValue &net : list) { + QStringList ends; + for (const QJsonValue &t : net.toObject().value(QStringLiteral("terminals")).toArray()) { + const QJsonObject end = t.toObject(); + if (end.value(QStringLiteral("folio")).toInt() == 1) + ends << end.value(QStringLiteral("element")).toString(); + } + ends.sort(); + if (!ends.isEmpty()) + result << ends.join(QLatin1Char(',')); + } + result.sort(); + return result; + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + QCOMPARE(lamps(load(example)).size(), 3); + } + + void sharedUuidSurvivesSaves() + { + const QStringList expected = nets(example); + QVERIFY(expected.contains(QStringLiteral("L2,L3,S2"))); + + QDomDocument doc = load(example); + const QList l = lamps(doc); + const QString shared = l.first().attribute(QStringLiteral("uuid")); + for (QDomElement e : l) + e.setAttribute(QStringLiteral("uuid"), shared); + + const QString twice = resave(resave(write(doc))); + QVERIFY(!twice.isEmpty()); + QCOMPARE(nets(twice), expected); + + //No wire names a lamp by the uuid they share + for (const QDomElement &c : childElements(firstDiagram(load(twice)), + QStringLiteral("conductors"), + QStringLiteral("conductor"))) { + QVERIFY(c.attribute(QStringLiteral("element1")) != shared); + QVERIFY(c.attribute(QStringLiteral("element2")) != shared); + } + } + + void savedByUuidReopensRight() + { + const QString fixture = QFINDTESTDATA("fixtures/shared_symbol_uuid_0200.qet"); + QVERIFY(!fixture.isEmpty()); + QCOMPARE(nets(fixture), nets(example)); + } +}; + +QTEST_GUILESS_MAIN(tst_sharedsymboluuid) +#include "tst_sharedsymboluuid.moc"