diff --git a/lib/solvers/TraceCleanupSolver/simplifyPath.ts b/lib/solvers/TraceCleanupSolver/simplifyPath.ts index e17bfb52..16349355 100644 --- a/lib/solvers/TraceCleanupSolver/simplifyPath.ts +++ b/lib/solvers/TraceCleanupSolver/simplifyPath.ts @@ -4,13 +4,38 @@ import { isVertical, } from "lib/solvers/SchematicTraceLinesSolver/SchematicTraceSingleLineSolver2/collisions" +const EPS = 1e-9 + +/** + * Remove consecutive duplicate points (within epsilon) from a path. + * Zero-length segments caused by duplicate points render as spurious extra + * trace lines, so this cleanup should be applied whenever a new trace path + * is assembled from spliced segments. + */ +export const removeDuplicateConsecutivePoints = (path: Point[]): Point[] => { + if (path.length < 2) return path + const result: Point[] = [path[0]] + for (let i = 1; i < path.length; i++) { + const prev = result[result.length - 1] + const curr = path[i] + if (Math.abs(prev.x - curr.x) > EPS || Math.abs(prev.y - curr.y) > EPS) { + result.push(curr) + } + } + return result +} + export const simplifyPath = (path: Point[]): Point[] => { - if (path.length < 3) return path - const newPath: Point[] = [path[0]] - for (let i = 1; i < path.length - 1; i++) { + // First remove exact duplicate consecutive points so the collinear check + // below does not produce degenerate zero-length segments. + const deduped = removeDuplicateConsecutivePoints(path) + + if (deduped.length < 3) return deduped + const newPath: Point[] = [deduped[0]] + for (let i = 1; i < deduped.length - 1; i++) { const p1 = newPath[newPath.length - 1] - const p2 = path[i] - const p3 = path[i + 1] + const p2 = deduped[i] + const p3 = deduped[i + 1] if ( (isVertical(p1, p2) && isVertical(p2, p3)) || (isHorizontal(p1, p2) && isHorizontal(p2, p3)) @@ -19,7 +44,7 @@ export const simplifyPath = (path: Point[]): Point[] => { } newPath.push(p2) } - newPath.push(path[path.length - 1]) + newPath.push(deduped[deduped.length - 1]) if (newPath.length < 3) return newPath const finalPath: Point[] = [newPath[0]] diff --git a/lib/solvers/TraceCleanupSolver/sub-solver/UntangleTraceSubsolver.ts b/lib/solvers/TraceCleanupSolver/sub-solver/UntangleTraceSubsolver.ts index 3519aa90..367cd825 100644 --- a/lib/solvers/TraceCleanupSolver/sub-solver/UntangleTraceSubsolver.ts +++ b/lib/solvers/TraceCleanupSolver/sub-solver/UntangleTraceSubsolver.ts @@ -8,6 +8,7 @@ import { getTraceObstacles } from "./getTraceObstacles" import { findIntersectionsWithObstacles } from "./findIntersectionsWithObstacles" import { generateLShapeRerouteCandidates } from "./generateLShapeRerouteCandidates" import { isPathColliding, type CollisionInfo } from "./isPathColliding" +import { removeDuplicateConsecutivePoints } from "../simplifyPath" import { generateRectangleCandidates, type Rectangle, @@ -258,11 +259,15 @@ export class UntangleTraceSubsolver extends BaseSolver { p.x === this.currentLShape!.p2.x && p.y === this.currentLShape!.p2.y, ) if (p2Index !== -1) { - const newTracePath = [ + // Splice the rerouted segment into the original path. When the + // slice boundaries coincide with points in bestRoute the concatenation + // produces consecutive duplicate points that render as spurious extra + // trace lines (issue #78). Remove them before storing. + const newTracePath = removeDuplicateConsecutivePoints([ ...originalTrace.tracePath.slice(0, p2Index), ...bestRoute, ...originalTrace.tracePath.slice(p2Index + 1), - ] + ]) this.input.allTraces[traceIndex] = { ...originalTrace, tracePath: newTracePath, diff --git a/tests/examples/__snapshots__/example21.snap.svg b/tests/examples/__snapshots__/example21.snap.svg index 5e7e7ceb..1ffbc5a3 100644 --- a/tests/examples/__snapshots__/example21.snap.svg +++ b/tests/examples/__snapshots__/example21.snap.svg @@ -162,10 +162,10 @@ y-" data-x="2.9752723250000006" data-y="-0.4999999999999998" cx="345.12348870636 - + - + diff --git a/tests/functions/removeDuplicateConsecutivePoints.test.ts b/tests/functions/removeDuplicateConsecutivePoints.test.ts new file mode 100644 index 00000000..745b8696 --- /dev/null +++ b/tests/functions/removeDuplicateConsecutivePoints.test.ts @@ -0,0 +1,130 @@ +import { test, expect } from "bun:test" +import { + removeDuplicateConsecutivePoints, + simplifyPath, +} from "lib/solvers/TraceCleanupSolver/simplifyPath" + +/** + * Unit tests for removeDuplicateConsecutivePoints. + * + * This utility is the core fix for issue #78: when UntangleTraceSubsolver + * splices a rerouted segment back into the original trace path the junction + * points appear twice (once at the end of the left slice and again at the + * start of bestRoute, or at the end of bestRoute and the start of the right + * slice). Those zero-length duplicate segments render as extra trace lines + * in the schematic viewer. + */ + +test("removeDuplicateConsecutivePoints: removes exact duplicate consecutive points", () => { + const path = [ + { x: 0, y: 0 }, + { x: 1, y: 0 }, // duplicate of next + { x: 1, y: 0 }, + { x: 2, y: 0 }, + ] + const result = removeDuplicateConsecutivePoints(path) + expect(result).toEqual([ + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 2, y: 0 }, + ]) +}) + +test("removeDuplicateConsecutivePoints: removes points within epsilon (1e-9)", () => { + const eps = 5e-10 // less than 1e-9 + const path = [ + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 1 + eps, y: eps }, // near-duplicate of previous + { x: 2, y: 0 }, + ] + const result = removeDuplicateConsecutivePoints(path) + expect(result).toEqual([ + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 2, y: 0 }, + ]) +}) + +test("removeDuplicateConsecutivePoints: keeps non-duplicate points unchanged", () => { + const path = [ + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 1, y: 1 }, + { x: 2, y: 1 }, + ] + const result = removeDuplicateConsecutivePoints(path) + expect(result).toEqual(path) +}) + +test("removeDuplicateConsecutivePoints: handles empty path", () => { + expect(removeDuplicateConsecutivePoints([])).toEqual([]) +}) + +test("removeDuplicateConsecutivePoints: handles single-point path", () => { + const path = [{ x: 1, y: 2 }] + expect(removeDuplicateConsecutivePoints(path)).toEqual(path) +}) + +test("removeDuplicateConsecutivePoints: removes all duplicates in a run", () => { + const path = [ + { x: 0, y: 0 }, + { x: 0, y: 0 }, + { x: 0, y: 0 }, + { x: 1, y: 1 }, + ] + expect(removeDuplicateConsecutivePoints(path)).toEqual([ + { x: 0, y: 0 }, + { x: 1, y: 1 }, + ]) +}) + +test("removeDuplicateConsecutivePoints: does NOT remove non-consecutive duplicates", () => { + const path = [ + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 0, y: 0 }, // same as first but NOT consecutive + ] + const result = removeDuplicateConsecutivePoints(path) + expect(result).toEqual(path) +}) + +test("simplifyPath: duplicate boundary points from _applyBestRoute splice are removed", () => { + // Simulate the splice that _applyBestRoute performs (issue #78 root cause). + // Original path: A - B - C - D + // p2 = B (index 1); bestRoute = [A, mid, C] + // Naive splice: slice(0,1)=[A], bestRoute=[A,mid,C], slice(2)=[C,D] + // => [A, A, mid, C, C, D] -- has two pairs of consecutive duplicates + // After fix: [A, mid, C, D] + + const A = { x: 0, y: 0 } + const mid = { x: 0, y: 1 } + const C = { x: 1, y: 1 } + const D = { x: 2, y: 1 } + + const originalPath = [A, { x: 1, y: 0 }, C, D] + const p2Index = 1 + const bestRoute = [A, mid, C] + + const spliced = [ + ...originalPath.slice(0, p2Index), // [A] + ...bestRoute, // [A, mid, C] — A duplicates end of left slice + ...originalPath.slice(p2Index + 1), // [C, D] — C duplicates end of bestRoute + ] + // spliced = [A, A, mid, C, C, D] + + const cleaned = simplifyPath(spliced) + + // No consecutive duplicate points in the result + for (let i = 1; i < cleaned.length; i++) { + const prev = cleaned[i - 1] + const curr = cleaned[i] + expect(prev.x === curr.x && prev.y === curr.y).toBe(false) + } + + // simplifyPath also removes collinear intermediate points so mid→C→D + // (all at y=1, going right) collapses to mid→D. The key invariant is + // no consecutive duplicates and the path starts at A and ends at D. + expect(cleaned[0]).toEqual(A) + expect(cleaned[cleaned.length - 1]).toEqual(D) +}) diff --git a/tests/solvers/TraceCleanupSolver/TraceCleanupSolver_repro78.test.ts b/tests/solvers/TraceCleanupSolver/TraceCleanupSolver_repro78.test.ts new file mode 100644 index 00000000..b4ffa5df --- /dev/null +++ b/tests/solvers/TraceCleanupSolver/TraceCleanupSolver_repro78.test.ts @@ -0,0 +1,91 @@ +import type { InputProblem } from "lib/types/InputProblem" +import { test, expect } from "bun:test" +import { SchematicTracePipelineSolver } from "lib/index" + +/** + * Regression test for issue #78: "Fix extra trace lines in post-processing step" + * + * When UntangleTraceSubsolver reroutes an L-shaped turn it splices a bestRoute + * segment into the original trace path: + * + * newTracePath = [...slice(0, p2Index), ...bestRoute, ...slice(p2Index + 1)] + * + * The last point of the left slice equals the first point of bestRoute (p1), + * and the last point of bestRoute (p3) equals the first point of the right + * slice. Those duplicate points produce zero-length segments that render as + * spurious extra trace lines in the schematic viewer. + * + * The fix calls removeDuplicateConsecutivePoints() on the assembled path so + * that the duplicate boundary points are eliminated before the trace is stored. + */ +const inputProblem: InputProblem = { + chips: [ + { + chipId: "U1", + center: { x: 0, y: 0 }, + width: 1.0, + height: 2.0, + pins: [ + { pinId: "U1.1", x: -0.5, y: 0.5 }, + { pinId: "U1.2", x: -0.5, y: -0.5 }, + ], + }, + { + chipId: "U2", + center: { x: 4, y: 0 }, + width: 1.0, + height: 2.0, + pins: [ + { pinId: "U2.1", x: 3.5, y: 0.5 }, + { pinId: "U2.2", x: 3.5, y: -0.5 }, + ], + }, + { + chipId: "U3", + center: { x: 2, y: 3 }, + width: 1.0, + height: 2.0, + pins: [ + { pinId: "U3.1", x: 1.5, y: 3.5 }, + { pinId: "U3.2", x: 1.5, y: 2.5 }, + ], + }, + ], + directConnections: [ + { netId: "NET_A", pinIds: ["U1.1", "U2.1"] }, + { netId: "NET_B", pinIds: ["U1.2", "U3.2"] }, + ], + netConnections: [], + availableNetLabelOrientations: {}, + maxMspPairDistance: 10, +} + +test("SchematicTracePipelineSolver_repro78: no duplicate consecutive points in solved traces", () => { + const solver = new SchematicTracePipelineSolver(inputProblem) + solver.solve() + expect(solver.solved).toBe(true) + + const EPS = 1e-9 + + // Collect all trace paths from every sub-solver that may produce them + const traceCleanupSolver = solver.traceCleanupSolver + if (!traceCleanupSolver) return + + for (const trace of traceCleanupSolver.getOutput().traces) { + const path = trace.tracePath + for (let i = 1; i < path.length; i++) { + const prev = path[i - 1] + const curr = path[i] + const dx = Math.abs(curr.x - prev.x) + const dy = Math.abs(curr.y - prev.y) + const isDuplicate = dx <= EPS && dy <= EPS + expect(isDuplicate).toBe(false) + } + } +}) + +test("SchematicTracePipelineSolver_repro78: solver completes without error", () => { + const solver = new SchematicTracePipelineSolver(inputProblem) + expect(() => solver.solve()).not.toThrow() + expect(solver.solved).toBe(true) +})