From 6f5f657a27ca917d00b7562995dc3af3d2a00454 Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sat, 25 Jul 2026 20:09:54 +0900 Subject: [PATCH 1/3] test: pin different-net trace crossings per bug report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a helper that counts places where traces on different nets properly cross, and a test that pins the current count for every imported bug report. These are current values, not targets — several of these boards are open bugs. The point is that a routing or cleanup change which makes a board worse now fails a named assertion instead of disappearing into a regenerated SVG snapshot. Verified the guard bites: changing the expected count for bug-report-20260706T213649Z from 11 to 10 fails with 'Expected: 10, Received: 11'. --- tests/different-net-trace-crossings.test.ts | 74 +++++++++++++++ tests/fixtures/traceCrossings.ts | 100 ++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/different-net-trace-crossings.test.ts create mode 100644 tests/fixtures/traceCrossings.ts diff --git a/tests/different-net-trace-crossings.test.ts b/tests/different-net-trace-crossings.test.ts new file mode 100644 index 000000000..2f61a4a08 --- /dev/null +++ b/tests/different-net-trace-crossings.test.ts @@ -0,0 +1,74 @@ +import { expect, test } from "bun:test" +import fs from "node:fs" +import path from "node:path" +import { SchematicTracePipelineSolver } from "lib/solvers/SchematicTracePipelineSolver/SchematicTracePipelineSolver" +import { getTraceCrossings } from "tests/fixtures/traceCrossings" + +/** + * Different-net trace crossings, measured across every imported bug report. + * + * These are the current values, not a target — several of these boards are + * open bugs. The point is that the numbers are pinned: a routing or cleanup + * change that makes any board worse fails here instead of sliding into a + * regenerated SVG snapshot where a crossing is easy to miss. + * + * If a change legitimately improves a board, lower the number in the same PR. + */ +const EXPECTED_CROSSINGS: Record = { + "bug-report-20260706T213649Z": 11, + "bug-report-20260706T220324Z": 6, + "bug-report-20260721T221026Z": 5, + "bug-report-20260716T144856Z": 4, + "bug-report-20260717T022934Z": 3, + "bug-report-20260707T134549Z": 3, + "bug-report-20260708T055430Z": 2, + "bug-report-20260707T092615Z": 2, + "bug-report-20260707T230831Z": 2, + "bug-report-20260707T140410Z": 1, + "bug-report-20260707T020342Z": 1, + "bug-report-20260717T031704Z": 0, + "bug-report-20260708T053736Z": 0, + "bug-report-20260707T134722Z": 0, + "bug-report-20260707T141025Z": 0, + "bug-report-20260708T095725Z": 0, + "bug-report-20260707T141421Z": 0, + "bug-report-20260717T042845Z": 0, + "bug-report-20260724T175257Z": 0, +} + +const BUG_REPORTS_DIR = path.join(import.meta.dir, "bug-reports") + +const countCrossings = (board: string) => { + const inputPath = path.join(BUG_REPORTS_DIR, board, `${board}.json`) + const inputProblem = JSON.parse(fs.readFileSync(inputPath, "utf8")) + + const solver = new SchematicTracePipelineSolver(inputProblem as any) + solver.solve() + + // `netLabelNetLabelCollisionSolver` is the last pipeline step but only + // adjusts labels, so the final trace geometry comes from the cleanup pass + // feeding it. + const traces = + solver.traceCleanupSolver2?.getOutput().traces ?? + solver.traceLabelOverlapAvoidanceSolver?.getOutput().traces ?? + [] + + return getTraceCrossings(traces).length +} + +test("every pinned board still exists", () => { + const onDisk = fs + .readdirSync(BUG_REPORTS_DIR) + .filter((d) => fs.existsSync(path.join(BUG_REPORTS_DIR, d, `${d}.json`))) + .sort() + + // A newly imported bug report should be added above rather than silently + // going unmeasured. + expect(onDisk).toEqual(Object.keys(EXPECTED_CROSSINGS).sort()) +}) + +for (const [board, expected] of Object.entries(EXPECTED_CROSSINGS)) { + test(`${board} has ${expected} different-net trace crossings`, () => { + expect(countCrossings(board)).toBe(expected) + }) +} diff --git a/tests/fixtures/traceCrossings.ts b/tests/fixtures/traceCrossings.ts new file mode 100644 index 000000000..2a8042a73 --- /dev/null +++ b/tests/fixtures/traceCrossings.ts @@ -0,0 +1,100 @@ +import type { SolvedTracePath } from "lib/solvers/SchematicTraceLinesSolver/SchematicTraceLinesSolver" + +const EPS = 1e-9 + +const isHorizontal = ( + a: { x: number; y: number }, + b: { x: number; y: number }, +) => Math.abs(a.y - b.y) < EPS + +const isVertical = (a: { x: number; y: number }, b: { x: number; y: number }) => + Math.abs(a.x - b.x) < EPS + +type Segment = { + a: { x: number; y: number } + b: { x: number; y: number } + traceId: string + netId: string + segmentIndex: number +} + +/** + * True when a horizontal and a vertical segment properly cross — they share an + * interior point. Touching at an endpoint is excluded, since traces on the same + * net legitimately meet there. + */ +const properlyCrosses = (h: Segment, v: Segment) => { + const y = h.a.y + const x = v.a.x + + const withinH = + x > Math.min(h.a.x, h.b.x) + EPS && x < Math.max(h.a.x, h.b.x) - EPS + const withinV = + y > Math.min(v.a.y, v.b.y) + EPS && y < Math.max(v.a.y, v.b.y) - EPS + + return withinH && withinV +} + +/** + * Counts places where traces belonging to *different* nets cross each other. + * + * A schematic can't always avoid these, but each one is a readability cost, so + * this is useful as a regression guard: a change to the routing or cleanup + * solvers should not silently increase the count on a known board. + */ +export const getTraceCrossings = (traces: SolvedTracePath[]) => { + const segments: Segment[] = [] + + for (const trace of traces) { + for (let i = 0; i < trace.tracePath.length - 1; i++) { + segments.push({ + a: trace.tracePath[i]!, + b: trace.tracePath[i + 1]!, + traceId: trace.mspPairId, + netId: trace.globalConnNetId, + segmentIndex: i, + }) + } + } + + const crossings: Array<{ + aTraceId: string + aNetId: string + aSegmentIndex: number + bTraceId: string + bNetId: string + bSegmentIndex: number + at: { x: number; y: number } + }> = [] + + for (let i = 0; i < segments.length; i++) { + for (let k = i + 1; k < segments.length; k++) { + const s1 = segments[i]! + const s2 = segments[k]! + if (s1.netId === s2.netId) continue + + let h: Segment | null = null + let v: Segment | null = null + if (isHorizontal(s1.a, s1.b) && isVertical(s2.a, s2.b)) { + h = s1 + v = s2 + } else if (isVertical(s1.a, s1.b) && isHorizontal(s2.a, s2.b)) { + h = s2 + v = s1 + } + if (!h || !v || !properlyCrosses(h, v)) continue + + crossings.push({ + aTraceId: s1.traceId, + aNetId: s1.netId, + aSegmentIndex: s1.segmentIndex, + bTraceId: s2.traceId, + bNetId: s2.netId, + bSegmentIndex: s2.segmentIndex, + at: { x: v.a.x, y: h.a.y }, + }) + } + } + + return crossings +} From c56a75ecf4a71da01353ce8836f1e64b3b19198a Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sun, 26 Jul 2026 12:47:49 +0900 Subject: [PATCH 2/3] test: update the 092615Z baseline after #727 #727 (TraceOverlapShiftSolver) fixed the RP2040 USB-C overlap and moved bug-report-20260707T092615Z from 2 different-net crossings to 3. Measured either side of that merge: ca7e80c (before) TOTAL 40, this board 2 be20aa7 (#727) TOTAL 41, this board 3 Pinned at the current value with a comment recording the change, and reported it on #727 so the trade is visible rather than absorbed. --- tests/different-net-trace-crossings.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/different-net-trace-crossings.test.ts b/tests/different-net-trace-crossings.test.ts index 2f61a4a08..ca2150374 100644 --- a/tests/different-net-trace-crossings.test.ts +++ b/tests/different-net-trace-crossings.test.ts @@ -22,7 +22,10 @@ const EXPECTED_CROSSINGS: Record = { "bug-report-20260717T022934Z": 3, "bug-report-20260707T134549Z": 3, "bug-report-20260708T055430Z": 2, - "bug-report-20260707T092615Z": 2, + // Was 2 before #727 (TraceOverlapShiftSolver); that PR fixed the RP2040 + // USB-C overlap and moved this board from 2 to 3. Pinned at the current + // value with the regression reported on #727 rather than silently accepted. + "bug-report-20260707T092615Z": 3, "bug-report-20260707T230831Z": 2, "bug-report-20260707T140410Z": 1, "bug-report-20260707T020342Z": 1, From f788a17eab416378b17c3a069d181529302318c3 Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sat, 25 Jul 2026 20:16:34 +0900 Subject: [PATCH 3/3] fix: reject net-label candidates whose connector crosses another net getCandidateStatus already checks the candidate's connector against chips and other net labels, but never against existing traces. A candidate could therefore be accepted while its connector cut straight across a different net, which reads as a short in the rendered schematic. Reuse the existing tracePathCrossesAnyTrace helper, filtered to traces on other nets, and reject those candidates so the search moves to the next one. Measured across every imported bug report: 40 -> 19 different-net crossings, 7 boards improved, none regressed. Worst board goes 11 -> 4, and two boards reach zero. --- .../AvailableNetOrientationSolver.ts | 28 ++++++ .../bug-report-20260706T213649Z.snap.svg | 26 +++--- .../bug-report-20260706T220324Z.snap.svg | 10 +- .../bug-report-20260707T134549Z.snap.svg | 6 +- .../bug-report-20260708T055430Z.snap.svg | 92 +++++++++---------- .../bug-report-20260716T144856Z.snap.svg | 14 +-- .../bug-report-20260717T022934Z.snap.svg | 14 +-- .../bug-report-20260721T221026Z.snap.svg | 10 +- tests/different-net-trace-crossings.test.ts | 20 ++-- .../examples/__snapshots__/example43.snap.svg | 6 +- .../examples/__snapshots__/example44.snap.svg | 14 +-- .../examples/__snapshots__/example45.snap.svg | 6 +- .../examples/__snapshots__/example46.snap.svg | 6 +- .../examples/__snapshots__/example49.snap.svg | 14 +-- ...repro51-overlap-junction-crossing.snap.svg | 6 +- 15 files changed, 150 insertions(+), 122 deletions(-) diff --git a/lib/solvers/AvailableNetOrientationSolver/AvailableNetOrientationSolver.ts b/lib/solvers/AvailableNetOrientationSolver/AvailableNetOrientationSolver.ts index 5a92ffcca..5ac7c9f08 100644 --- a/lib/solvers/AvailableNetOrientationSolver/AvailableNetOrientationSolver.ts +++ b/lib/solvers/AvailableNetOrientationSolver/AvailableNetOrientationSolver.ts @@ -24,6 +24,7 @@ import { simplifyOrthogonalPath, traceCrossesBoundsInterior, tracePathCrossesAnyBounds, + tracePathCrossesAnyTrace, tracePathIntersectsBounds, } from "./geometry" import { getPinMap, getTracePins, toNetLabelPlacementPatch } from "./traces" @@ -815,9 +816,36 @@ export class AvailableNetOrientationSolver extends BaseSolver { } } + // The connector is checked against chips and other labels above, but not + // against existing traces — so a candidate could be accepted while its + // connector cut straight across another net, which reads as a short in the + // rendered schematic. Reject those and let the search try the next + // candidate. + if (this.connectorCrossesOtherNetTrace(connectorTrace, label)) { + return "trace-collision" + } + return "valid" } + /** + * True when the candidate's connector properly crosses a trace on a + * different net. Same-net traces are excluded: a connector legitimately + * meets the trace it is attaching to. + */ + private connectorCrossesOtherNetTrace( + connectorTrace: Point[], + label: NetLabelPlacement, + ) { + const otherNetTraces: Record = {} + for (const [id, trace] of Object.entries(this.traceMap)) { + if (trace.globalConnNetId === label.globalConnNetId) continue + otherNetTraces[id] = trace + } + + return tracePathCrossesAnyTrace(connectorTrace, otherNetTraces) + } + private isAcceptableTraceAnchorChipCollision( candidate: CandidateLabel, label: NetLabelPlacement, diff --git a/tests/bug-reports/bug-report-20260706T213649Z/__snapshots__/bug-report-20260706T213649Z.snap.svg b/tests/bug-reports/bug-report-20260706T213649Z/__snapshots__/bug-report-20260706T213649Z.snap.svg index b98797851..78f608ec2 100644 --- a/tests/bug-reports/bug-report-20260706T213649Z/__snapshots__/bug-report-20260706T213649Z.snap.svg +++ b/tests/bug-reports/bug-report-20260706T213649Z/__snapshots__/bug-report-20260706T213649Z.snap.svg @@ -1,6 +1,6 @@ -