From 84cda1f0b0141822f1455bb13f5472a1e39c473b Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 3 Oct 2026 08:43:03 +1300 Subject: [PATCH] Routing: no loops between close symbols, and route inside frames Reported on #1178: with "route": "avoid", two contacts one above the other, wired bottom terminal to top terminal 30 px apart or closer, got a five-segment loop (at 20 px: v 80, h -20, v -140, h 20, v 80) that ran down through the lower contact and back up past the upper one. The cause was exitPoint(): it walked out of a terminal until clear of every obstacle, so with another symbol in front it walked through that symbol, and the search then had to come back. The same walk made a wire between two symbols inside a frame (a cabinet drawn as one element) leave through the frame's side, go round, and cross back in. The router now knows each terminal's own symbol (Request::start_symbol, end_symbol; applyRoute() fills them): - two terminals facing each other on one line with nothing between them are joined straight, however close; - the exit walks through the margin around other symbols but never through one; a terminal pointing straight into another symbol gets "no-route" instead of a route through it; - an obstacle drawn around either end's own symbol is left out. Without the symbols (the old Request), routes are as before. Rerouting every wire of four shipped examples (perceuse, affuteuse_250h, Polonez, industrial; 1331 wires): master routes 121 of them through another symbol, this none (10 pass through a second symbol lying exactly on an end symbol's rectangle, which no route can avoid). 99 wires that master routed through a symbol now get "no-route" and keep their path. The 1232 wires both route are 6 % shorter in total (436,144 -> 410,608 units) with 11 % fewer bends (2256 -> 2010). Co-Authored-By: Claude Opus 5.5 --- misc/qet-mcp/README.md | 8 +- sources/conductorrouter.cpp | 119 +++++++++++++++++++++++---- sources/conductorrouter.h | 6 ++ sources/scripting/qetscriptapi.cpp | 9 +- tests/qttest/tst_conductorrouter.cpp | 93 +++++++++++++++++++++ 5 files changed, 218 insertions(+), 17 deletions(-) diff --git a/misc/qet-mcp/README.md b/misc/qet-mcp/README.md index d6b391456..b68cc335b 100644 --- a/misc/qet-mcp/README.md +++ b/misc/qet-mcp/README.md @@ -550,7 +550,13 @@ Python, plus the hang guard on `addConductor` and the database refresh in the run goes on. Like a hand-edited path, it is stretched rather than rerouted when a symbol is moved afterwards; route again after moving things. Needs `qet.routeConductor()` / `qet.routeConductorBetween()` in - the build, and only an edit that routes requires them. + the build, and only an edit that routes requires them. A symbol drawn + around either end's own symbol (a cabinet made as one element) is not + an obstacle, so a wire between two symbols inside one is routed inside + it. Two terminals facing each other on one line, with nothing between + them, are joined by a straight line however close they are. A + terminal pointing straight into another symbol has no route, rather + than one through that symbol. - **A terminal can be named by its uuid**: `terminal`, `from_terminal` and `to_terminal` take the terminal's uuid (as `qet_element_info` lists it) in place of its index, on the op's own element (for `add_conductor`, on diff --git a/sources/conductorrouter.cpp b/sources/conductorrouter.cpp index d3d800453..4bf2474e2 100644 --- a/sources/conductorrouter.cpp +++ b/sources/conductorrouter.cpp @@ -74,24 +74,54 @@ bool insideAny(const QPointF &p, const QList &rects) ///other axis struct Span { qreal at, from, to; }; - ///The first point of a route after a terminal: one grid step out in the - ///terminal's direction, snapped to the grid the way - ///Conductor::extendTerminal() snaps it, then on until it is clear of - ///every obstacle -- of the terminal's own symbol above all. -bool exitPoint(const QPointF &dock, Direction d, const ConductorRouter::Request &r, - const QList &obstacles, QPointF &out) + ///One grid step out of a terminal in its direction, snapped to the + ///grid the way Conductor::extendTerminal() snaps it +QPointF firstStep(const QPointF &dock, Direction d, qreal grid) { const QPointF s = step(d); QPointF p = dock; if (s.x() != 0) - p.setX(std::round((dock.x() + s.x() * r.grid) / r.grid) * r.grid); + p.setX(std::round((dock.x() + s.x() * grid) / grid) * grid); else - p.setY(std::round((dock.y() + s.y() * r.grid) / r.grid) * r.grid); + p.setY(std::round((dock.y() + s.y() * grid) / grid) * grid); + return p; +} + + ///Whether the horizontal or vertical segment from @p a to @p b runs + ///through the inside of @p rect; along its edge does not count. +bool crossesInside(const QPointF &a, const QPointF &b, const QRectF &rect) +{ + const QRectF seg = QRectF(a, b).normalized(); + if (seg.width() < eps) + return seg.left() > rect.left() + eps && seg.left() < rect.right() - eps + && std::min(seg.bottom(), rect.bottom()) - std::max(seg.top(), rect.top()) > eps; + return seg.top() > rect.top() + eps && seg.top() < rect.bottom() - eps + && std::min(seg.right(), rect.right()) - std::max(seg.left(), rect.left()) > eps; +} + + ///The first point of a route after a terminal: firstStep(), then on + ///until it is clear of every obstacle -- of the terminal's own symbol + ///above all. + ///When the terminal's own symbol is known, never through another + ///symbol, only through the margin around it: a terminal pointing + ///straight into one has no exit, where walking on through it would + ///give a route that crosses that symbol and loops back. @p others are + ///the other symbols, without their margin. +bool exitPoint(const QPointF &dock, Direction d, const ConductorRouter::Request &r, + const QList &obstacles, const QList &others, + bool own_known, QPointF &out) +{ + const QPointF s = step(d); + QPointF from = dock, p = firstStep(dock, d, r.grid); for (int i = 0; i < 200; ++i) { + if (own_known) + for (const QRectF &o : others) + if (crossesInside(from, p, o)) return false; if (!insideAny(p, obstacles)) { out = p; return true; } + from = p; p += s * r.grid; } return false; @@ -204,14 +234,75 @@ ConductorRouter::Result ConductorRouter::route(const Request &r) return result; } - QList obstacles; - for (const QRectF &o : r.obstacles) - obstacles << o.normalized().adjusted(-r.margin, -r.margin, r.margin, r.margin); + const QRectF own1 = r.start_symbol.normalized(), own2 = r.end_symbol.normalized(); + QList obstacles, others, other_symbols; + for (const QRectF &o : r.obstacles) { + const QRectF n = o.normalized(); + const bool own = (own1.isValid() && n == own1) || (own2.isValid() && n == own2); + // A symbol drawn around a terminal's own one is a frame the + // wire starts or ends inside: crossing its edge is the way in + // or out, and its inside is the place to route. + if (!own && ((own1.isValid() && n.contains(own1)) + || (own2.isValid() && n.contains(own2)))) + continue; + const QRectF padded = n.adjusted(-r.margin, -r.margin, r.margin, r.margin); + obstacles << padded; + if (!own) { + others << padded; + other_symbols << n; + } + } + + // Two terminals facing each other on one line, with nothing + // between them: the straight line, even when they are too close + // for each to step out a grid square first. With room, it keeps + // the step out of each terminal, as the search does (see the + // corners below); without, the point between them gives the path + // the three points a conductor's path needs. + const QPointF s = step(r.start_direction); + const QPointF ahead = r.end - r.start; + if (r.end_direction == opposite(r.start_direction) + && std::abs(s.x() != 0 ? ahead.y() : ahead.x()) < eps + && ahead.x() * s.x() + ahead.y() * s.y() > eps) { + bool clear = true; + for (const QRectF &o : others) + if (crossesInside(r.start, r.end, o)) { clear = false; break; } + // nor along another wire, which the search would avoid + const bool vertical = s.x() == 0; + const qreal line = vertical ? r.start.x() : r.start.y(); + const qreal lo = vertical ? std::min(r.start.y(), r.end.y()) : std::min(r.start.x(), r.end.x()); + const qreal hi = vertical ? std::max(r.start.y(), r.end.y()) : std::max(r.start.x(), r.end.x()); + for (const QVector &w : r.wires) { + for (int i = 0; clear && i + 1 < w.size(); ++i) { + const QPointF a = w.at(i), b = w.at(i + 1); + const qreal a_at = vertical ? a.x() : a.y(), b_at = vertical ? b.x() : b.y(); + if (std::abs(a_at - line) > 0.5 || std::abs(b_at - line) > 0.5) continue; + const qreal a_on = vertical ? a.y() : a.x(), b_on = vertical ? b.y() : b.x(); + if (std::min(hi, std::max(a_on, b_on)) - std::max(lo, std::min(a_on, b_on)) > eps) + clear = false; + } + } + if (clear) { + const QPointF e1 = firstStep(r.start, r.start_direction, r.grid); + const QPointF e2 = firstStep(r.end, r.end_direction, r.grid); + const QPointF gap = e2 - e1; + result.points << r.start; + if (gap.x() * s.x() + gap.y() * s.y() > eps) + result.points << e1 << e2; + else if (std::abs(gap.x()) < eps && std::abs(gap.y()) < eps) + result.points << e1; + else + result.points << (r.start + r.end) / 2; + result.points << r.end; + return result; + } + } QPointF s1, s2; - if (!exitPoint(r.start, r.start_direction, r, obstacles, s1) - || !exitPoint(r.end, r.end_direction, r, obstacles, s2)) { - result.error = QStringLiteral("a terminal has no way out of the symbols around it"); + if (!exitPoint(r.start, r.start_direction, r, obstacles, other_symbols, own1.isValid(), s1) + || !exitPoint(r.end, r.end_direction, r, obstacles, other_symbols, own2.isValid(), s2)) { + result.error = QStringLiteral("a terminal points straight into another symbol, " + "or has no way out of the symbols around it"); return result; } diff --git a/sources/conductorrouter.h b/sources/conductorrouter.h index 1fa6b65fe..d8bc1ac42 100644 --- a/sources/conductorrouter.h +++ b/sources/conductorrouter.h @@ -53,6 +53,12 @@ namespace ConductorRouter Direction end_direction = Direction::North; ///Areas no segment may cross. A margin is added to each. QList obstacles; + ///Each terminal's own symbol, when known, as it appears in + ///obstacles. A route steps out of it first and never walks + ///through another symbol to get out; an obstacle drawn around + ///it (a cabinet made as one element) is left out. + QRectF start_symbol; + QRectF end_symbol; ///The other wires on the folio, each as its list of points. ///Running along one or crossing one costs extra. QList> wires; diff --git a/sources/scripting/qetscriptapi.cpp b/sources/scripting/qetscriptapi.cpp index 0c5e83f9c..6b5462ffb 100644 --- a/sources/scripting/qetscriptapi.cpp +++ b/sources/scripting/qetscriptapi.cpp @@ -1226,8 +1226,9 @@ ConductorRouter::Direction routerDirection(Qet::Orientation o) as one undo step through Conductor::setPathPoints() -- the same ChangeConductorCommand a handle drag pushes, so the path is saved and survives a reload. Obstacles are every element's own rectangle, its - texts left out; the other conductors are not obstacles but cost extra - to run along or cross. + texts left out, except one drawn around either end's own symbol (a + frame); the other conductors are not obstacles but cost extra to run + along or cross. @return "routed", or "no-route" with the reason logged when there is no such path -- the conductor then keeps the path it had. Not a failure: the wire exists and joins the right terminals either way. @@ -1246,6 +1247,10 @@ QString QetScriptApi::applyRoute(Conductor *conductor, const QString &caller) request.bounds = diagram->border_and_titleblock.insideBorderRect(); for (Element *e : diagram->elements()) request.obstacles << e->mapRectToScene(e->boundingRect()); + if (Element *e = conductor->terminal1->parentElement()) + request.start_symbol = e->mapRectToScene(e->boundingRect()); + if (Element *e = conductor->terminal2->parentElement()) + request.end_symbol = e->mapRectToScene(e->boundingRect()); for (Conductor *other : diagram->conductors()) { if (other == conductor) continue; QVector wire; diff --git a/tests/qttest/tst_conductorrouter.cpp b/tests/qttest/tst_conductorrouter.cpp index 6807d4149..dab3b2de4 100644 --- a/tests/qttest/tst_conductorrouter.cpp +++ b/tests/qttest/tst_conductorrouter.cpp @@ -206,6 +206,99 @@ private slots: QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); checkShape(result.points, r); } + + // #1178: two contacts one above the other, the upper one's bottom + // terminal wired to the lower one's top terminal. Each docking + // point is 10 px inside its symbol, as in con_simple.elmt. Close + // together, the route went down through the lower contact, back + // up past the upper one and down again. + void closeFacingTerminalsGoStraight_data() + { + QTest::addColumn("gap"); + for (int gap : {2, 8, 10, 18, 20, 28, 30, 38, 40, 60}) + QTest::addRow("%d px", gap) << gap; + } + + void closeFacingTerminalsGoStraight() + { + QFETCH(int, gap); + ConductorRouter::Request r; + r.start = {100, 120}; + r.start_direction = Direction::South; + r.end = {100, 120. + gap}; + r.end_direction = Direction::North; + r.start_symbol = QRectF(90, 70, 20, 60); + r.end_symbol = QRectF(90, r.end.y() - 10, 20, 60); + r.obstacles << r.start_symbol << r.end_symbol; + const auto result = ConductorRouter::route(r); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + checkShape(result.points, r); + for (int i = 0; i + 1 < result.points.size(); ++i) { + QCOMPARE(result.points.at(i).x(), 100.); + QVERIFY2(result.points.at(i + 1).y() > result.points.at(i).y(), + "the route turns back on itself"); + } + } + + // The straight line is taken only when nothing is in its way. + void facingTerminalsGoAroundWhatIsBetweenThem() + { + ConductorRouter::Request r; + r.start = {100, 120}; + r.start_direction = Direction::South; + r.end = {100, 250}; + r.end_direction = Direction::North; + r.start_symbol = QRectF(90, 70, 20, 60); + r.end_symbol = QRectF(90, 240, 20, 60); + const QRectF between(80, 170, 40, 30); + r.obstacles << r.start_symbol << r.end_symbol << between; + const auto result = ConductorRouter::route(r); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + checkShape(result.points, r); + QVERIFY(!crosses(result.points, between.adjusted(-r.margin, -r.margin, r.margin, r.margin))); + } + + // A terminal pointing straight into a symbol that is not the other + // end: no route, rather than one that runs through that symbol. + void noRouteThroughASymbolInFrontOfATerminal() + { + ConductorRouter::Request r; + r.start = {100, 120}; + r.start_direction = Direction::South; + r.end = {300, 300}; + r.end_direction = Direction::West; + r.start_symbol = QRectF(90, 70, 20, 60); + r.end_symbol = QRectF(300, 280, 40, 40); + const QRectF in_front(60, 132, 80, 40); + r.obstacles << r.start_symbol << r.end_symbol << in_front; + const auto result = ConductorRouter::route(r); + QVERIFY(result.points.isEmpty()); + QVERIFY(!result.error.isEmpty()); + } + + // A symbol drawn around others (a cabinet made as one element): + // the wire between two symbols inside it stays inside it, going + // round what lies between them. + void routesInsideAnEnclosingSymbol() + { + ConductorRouter::Request r; + r.start = {100, 100}; + r.start_direction = Direction::East; + r.end = {300, 100}; + r.end_direction = Direction::West; + r.start_symbol = QRectF(60, 80, 40, 40); + r.end_symbol = QRectF(300, 80, 40, 40); + const QRectF frame(20, 20, 360, 200); + const QRectF between(170, 60, 60, 80); + r.obstacles << frame << r.start_symbol << r.end_symbol << between; + r.bounds = QRectF(0, 0, 500, 400); + const auto result = ConductorRouter::route(r); + QVERIFY2(result.error.isEmpty(), qPrintable(result.error)); + checkShape(result.points, r); + QVERIFY(!crosses(result.points, between.adjusted(-r.margin, -r.margin, r.margin, r.margin))); + for (const QPointF &p : result.points) + QVERIFY2(frame.contains(p), "the route leaves the frame"); + } }; QTEST_MAIN(TestConductorRouter)