diff --git a/sources/positionorder.h b/sources/positionorder.h
new file mode 100644
index 000000000..4d895f419
--- /dev/null
+++ b/sources/positionorder.h
@@ -0,0 +1,69 @@
+/*
+ Copyright 2006-2026 The QElectroTech Team
+ This file is part of QElectroTech.
+
+ QElectroTech is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 2 of the License, or
+ (at your option) any later version.
+
+ QElectroTech is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with QElectroTech. If not, see .
+*/
+#ifndef POSITIONORDER_H
+#define POSITIONORDER_H
+
+#include
+#include
+
+/**
+ The order of items by their position on a folio, for std::sort: left to
+ right then top to bottom, or top to bottom then left to right. Each
+ function is a strict weak ordering, which std::sort requires: two items
+ at the same position come out in either order, but never "both before
+ each other", and the order is transitive. A comparison with "<=", or
+ with a tolerance ("within 1 px counts as aligned"), is neither, and
+ std::sort may then read past the range or assert.
+*/
+namespace PositionOrder
+{
+ /// a before b when a is left of b, or level with it and above it.
+ inline bool xThenY(const QPointF &a, const QPointF &b)
+ {
+ if (a.x() != b.x()) return a.x() < b.x();
+ return a.y() < b.y();
+ }
+
+ /// a before b when a is above b, or level with it and left of it.
+ inline bool yThenX(const QPointF &a, const QPointF &b)
+ {
+ if (a.y() != b.y()) return a.y() < b.y();
+ return a.x() < b.x();
+ }
+
+ /// The position rounded to whole pixels, so that items placed a
+ /// fraction of a pixel apart count as aligned.
+ inline QPointF rounded(const QPointF &p)
+ {
+ return QPointF(qRound(p.x()), qRound(p.y()));
+ }
+
+ /// xThenY() on the positions rounded to whole pixels.
+ inline bool roundedXThenY(const QPointF &a, const QPointF &b)
+ {
+ return xThenY(rounded(a), rounded(b));
+ }
+
+ /// yThenX() on the positions rounded to whole pixels.
+ inline bool roundedYThenX(const QPointF &a, const QPointF &b)
+ {
+ return yThenX(rounded(a), rounded(b));
+ }
+}
+
+#endif // POSITIONORDER_H
diff --git a/sources/qetgraphicsitem/element.cpp b/sources/qetgraphicsitem/element.cpp
index 80804145e..54c328ca5 100644
--- a/sources/qetgraphicsitem/element.cpp
+++ b/sources/qetgraphicsitem/element.cpp
@@ -53,6 +53,7 @@ static const QString plcTerminalKeys[] = {
QETInformation::ELMT_PLC_T4
};
#include "../qetxml.h"
+#include "../positionorder.h"
#include "../qetversion.h"
#include "qgraphicsitemutility.h"
#include
@@ -1934,9 +1935,7 @@ bool comparPos(const Element *elmt1, const Element *elmt2)
if (a != b)
return apos().x() == elmt2->pos().x())
- return elmt1->y() <= elmt2->pos().y();
- return elmt1->pos().x() <= elmt2->pos().x();
+ return PositionOrder::xThenY(elmt1->pos(), elmt2->pos());
}
/**
diff --git a/sources/ui/terminalnumberingdialog.cpp b/sources/ui/terminalnumberingdialog.cpp
index fde1df6da..6e30d32cc 100644
--- a/sources/ui/terminalnumberingdialog.cpp
+++ b/sources/ui/terminalnumberingdialog.cpp
@@ -4,6 +4,7 @@
#include "../qetproject.h"
#include "../diagram.h"
#include "../qetgraphicsitem/element.h"
+#include "../positionorder.h"
#include "../undocommand/changeelementinformationcommand.h"
#include "../qet.h"
#include
@@ -210,14 +211,12 @@ QUndoCommand* TerminalNumberingDialog::getUndoCommand(QETProject *project) const
// Then sort by folio (page) index
if (a.folioIndex != b.folioIndex) return a.folioIndex < b.folioIndex;
- // Finally sort by coordinates (with a 1.0px tolerance to handle slight misalignments)
- if (axisX) {
- if (qAbs(a.x - b.x) > 1.0) return a.x < b.x;
- return a.y < b.y;
- } else {
- if (qAbs(a.y - b.y) > 1.0) return a.y < b.y;
- return a.x < b.x;
- }
+ // Finally sort by coordinates, rounded to whole pixels so that slight
+ // misalignments count as aligned (a tolerance test is not transitive,
+ // which std::sort requires)
+ const QPointF pa(a.x, a.y), pb(b.x, b.y);
+ return axisX ? PositionOrder::roundedXThenY(pa, pb)
+ : PositionOrder::roundedYThenX(pa, pb);
});
// 4. Generate new numbering and create the undo command macro
diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt
index 7e10b46fa..53a6ab620 100644
--- a/tests/qttest/CMakeLists.txt
+++ b/tests/qttest/CMakeLists.txt
@@ -108,6 +108,13 @@ add_test(NAME tst_diagramsortkeys COMMAND tst_diagramsortkeys)
target_include_directories(tst_diagramsortkeys PRIVATE ${QET_DIR}/sources)
target_link_libraries(tst_diagramsortkeys PRIVATE Qt::Test)
+# positionorder.h is header-only: the position comparisons behind element
+# renumbering and terminal numbering, as strict weak orderings for std::sort.
+add_executable(tst_positionorder tst_positionorder.cpp)
+target_include_directories(tst_positionorder PRIVATE ${QET_DIR}/sources)
+target_link_libraries(tst_positionorder PRIVATE Qt::Test Qt::Core)
+add_test(NAME tst_positionorder COMMAND tst_positionorder)
+
# bordercelllabels.h is header-only too.
add_executable(tst_bordercelllabels tst_bordercelllabels.cpp)
add_test(NAME tst_bordercelllabels COMMAND tst_bordercelllabels)
diff --git a/tests/qttest/tst_positionorder.cpp b/tests/qttest/tst_positionorder.cpp
new file mode 100644
index 000000000..1ee9de8b1
--- /dev/null
+++ b/tests/qttest/tst_positionorder.cpp
@@ -0,0 +1,155 @@
+/*
+ Copyright 2006-2026 The QElectroTech Team
+ This file is part of QElectroTech.
+
+ QElectroTech is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 2 of the License, or
+ (at your option) any later version.
+
+ QElectroTech is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with QElectroTech. If not, see .
+*/
+#include "positionorder.h"
+
+#include
+#include
+#include
+
+/**
+ PositionOrder -- the position comparisons behind renumbering elements
+ (comparPos) and numbering terminals. std::sort needs a strict weak
+ ordering; the old comparisons used "<=" and a 1 px tolerance, which
+ are not one, so two elements at the same position (or three terminals
+ a fraction apart) were undefined behaviour.
+*/
+class tst_positionorder : public QObject
+{
+ Q_OBJECT
+
+ using Less = std::function;
+
+ static QList> orders()
+ {
+ return {{QStringLiteral("xThenY"), PositionOrder::xThenY},
+ {QStringLiteral("yThenX"), PositionOrder::yThenX},
+ {QStringLiteral("roundedXThenY"), PositionOrder::roundedXThenY},
+ {QStringLiteral("roundedYThenX"), PositionOrder::roundedYThenX}};
+ }
+
+private slots:
+ // Nothing is before itself, and equal points are before each other
+ // in neither direction: what "<=" broke.
+ void equalPointsAreNotOrdered()
+ {
+ const QPointF p(100, 200), q(100, 200);
+ for (const auto &order : orders()) {
+ QVERIFY2(!order.second(p, p), qPrintable(order.first));
+ QVERIFY2(!order.second(p, q), qPrintable(order.first));
+ QVERIFY2(!order.second(q, p), qPrintable(order.first));
+ }
+ }
+
+ void leftToRightThenTopToBottom()
+ {
+ QVERIFY(PositionOrder::xThenY(QPointF(10, 500), QPointF(20, 0)));
+ QVERIFY(!PositionOrder::xThenY(QPointF(20, 0), QPointF(10, 500)));
+ QVERIFY(PositionOrder::xThenY(QPointF(10, 0), QPointF(10, 5)));
+ QVERIFY(!PositionOrder::xThenY(QPointF(10, 5), QPointF(10, 0)));
+ }
+
+ void topToBottomThenLeftToRight()
+ {
+ QVERIFY(PositionOrder::yThenX(QPointF(500, 10), QPointF(0, 20)));
+ QVERIFY(!PositionOrder::yThenX(QPointF(0, 20), QPointF(500, 10)));
+ QVERIFY(PositionOrder::yThenX(QPointF(0, 10), QPointF(5, 10)));
+ QVERIFY(!PositionOrder::yThenX(QPointF(5, 10), QPointF(0, 10)));
+ }
+
+ // Items a fraction of a pixel apart (placed with Ctrl, or after a
+ // rotation) count as aligned, and are then ordered on the other axis.
+ void roundedTreatsFractionsAsAligned()
+ {
+ QVERIFY(PositionOrder::roundedXThenY(QPointF(10.2, 0), QPointF(9.8, 5)));
+ QVERIFY(!PositionOrder::roundedXThenY(QPointF(9.8, 5), QPointF(10.2, 0)));
+ QVERIFY(PositionOrder::roundedYThenX(QPointF(0, 10.2), QPointF(5, 9.8)));
+ QVERIFY(!PositionOrder::roundedYThenX(QPointF(5, 9.8), QPointF(0, 10.2)));
+ }
+
+ // The three terminals that broke the tolerance comparison: with
+ // "within 1 px counts as aligned", a < b (by y), b < c (by y) and
+ // c < a (by x), a cycle. Rounded, they are simply ordered by x.
+ void roundedIsTransitiveWhereToleranceWasNot()
+ {
+ const QPointF a(2, 0), b(1.1, 1), c(0.2, 2);
+ QVERIFY(PositionOrder::roundedXThenY(c, b));
+ QVERIFY(PositionOrder::roundedXThenY(b, a));
+ QVERIFY(PositionOrder::roundedXThenY(c, a));
+ QVERIFY(!PositionOrder::roundedXThenY(a, c));
+ }
+
+ // Every order is a strict weak ordering on a grid of awkward points:
+ // irreflexive, asymmetric, transitive, and with transitive
+ // equivalence. Checked by brute force on the whole set.
+ void strictWeakOrdering_data()
+ {
+ QTest::addColumn("name");
+ for (const auto &order : orders())
+ QTest::newRow(qPrintable(order.first)) << order.first;
+ }
+
+ void strictWeakOrdering()
+ {
+ QFETCH(QString, name);
+ Less less;
+ for (const auto &order : orders())
+ if (order.first == name) less = order.second;
+ QVERIFY(less);
+
+ QList points;
+ const QList values{-1, -0.6, -0.4, 0, 0.2, 0.5, 0.9, 1, 1.1, 2, 10.5, 61.3, 61.7};
+ for (qreal x : values)
+ for (qreal y : values)
+ points << QPointF(x, y);
+ points << points.first() << QPointF(2, 0) << QPointF(1.1, 1) << QPointF(0.2, 2);
+
+ auto equiv = [&](const QPointF &p, const QPointF &q) { return !less(p, q) && !less(q, p); };
+ for (const QPointF &p : points) {
+ QVERIFY(!less(p, p));
+ for (const QPointF &q : points) {
+ if (less(p, q)) QVERIFY(!less(q, p));
+ for (const QPointF &r : points) {
+ if (less(p, q) && less(q, r)) QVERIFY2(less(p, r), "transitive");
+ if (equiv(p, q) && equiv(q, r)) QVERIFY2(equiv(p, r), "equivalence transitive");
+ }
+ }
+ }
+
+ // And std::sort on it ends sorted, with the list intact.
+ QList sorted = points;
+ std::sort(sorted.begin(), sorted.end(), less);
+ QCOMPARE(sorted.size(), points.size());
+ for (int i = 1; i < sorted.size(); ++i)
+ QVERIFY(!less(sorted.at(i), sorted.at(i - 1)));
+ }
+
+ // Twenty items at one position, as pasting at the origin leaves
+ // them: a list std::sort could scan past the end of with "<=".
+ void manyEqualPositionsSort()
+ {
+ QList points(20, QPointF(0, 0));
+ points << QPointF(-10, 0) << QPointF(10, 0);
+ std::sort(points.begin(), points.end(), PositionOrder::xThenY);
+ QCOMPARE(points.size(), 22);
+ QCOMPARE(points.first(), QPointF(-10, 0));
+ QCOMPARE(points.last(), QPointF(10, 0));
+ }
+};
+
+QTEST_GUILESS_MAIN(tst_positionorder)
+#include "tst_positionorder.moc"