fix: remove extra trace lines from duplicate consecutive points#144
Open
ayushopchauhan wants to merge 1 commit intotscircuit:mainfrom
Open
fix: remove extra trace lines from duplicate consecutive points#144ayushopchauhan wants to merge 1 commit intotscircuit:mainfrom
ayushopchauhan wants to merge 1 commit intotscircuit:mainfrom
Conversation
…rcuit#78) Root cause: UntangleTraceSubsolver._applyBestRoute() concatenates path segments when rerouting L-shaped corners, producing consecutive duplicate points that render as zero-length extra trace segments. Fix: - Add removeDuplicateConsecutivePoints() to simplifyPath.ts - Apply dedup as preprocessing in simplifyPath() before collinear reduction - Apply dedup in _applyBestRoute() after path splice All 59 existing tests pass with zero snapshot regressions. 8 new unit tests + 1 integration test added.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Author
|
@seveibar Ready for review. This fixes extra trace lines caused by consecutive duplicate points in |
Author
|
Hey @seveibar, friendly nudge - this one's also ready for review. CI green, tests passing. Happy to make changes if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #78\n\n## Root Cause\n\n
UntangleTraceSubsolver._applyBestRoute()concatenates path segments when rerouting L-shaped corners:\n\nts\nconst newTracePath = [\n ...originalTrace.tracePath.slice(0, p2Index),\n ...bestRoute,\n ...originalTrace.tracePath.slice(p2Index + 1),\n]\n\n\nThe last point ofslice(0, p2Index)can be identical to the first point ofbestRoute, and the last point ofbestRoutecan be identical to the first point ofslice(p2Index + 1). This produces consecutive duplicate points that render as zero-length extra trace segments.\n\n## Fix\n\n1. New utility:removeDuplicateConsecutivePoints()insimplifyPath.ts\n- Filters consecutive points that are within1e-9epsilon distance\n- Reusable across the codebase (same pattern already exists inTraceOverlapIssueSolver)\n\n2. Applied in_applyBestRoute()after the splice:\nts\nconst newTracePath = removeDuplicateConsecutivePoints([\n ...originalTrace.tracePath.slice(0, p2Index),\n ...bestRoute,\n ...originalTrace.tracePath.slice(p2Index + 1),\n])\n\n\n3. Applied as preprocessing insimplifyPath()\n- Deduplicates before collinear reduction, preventing edge cases where duplicates confuse theisVertical/isHorizontalchecks\n\n## Files Changed\n\n| File | Change |\n|------|--------|\n|lib/solvers/TraceCleanupSolver/simplifyPath.ts| AddedremoveDuplicateConsecutivePoints(), applied as preprocessing insimplifyPath()|\n|lib/solvers/TraceCleanupSolver/sub-solver/UntangleTraceSubsolver.ts| Imported and applied dedup in_applyBestRoute()|\n|site/examples/example32-fix-extra-trace-lines.page.tsx| New example page with crossing traces that trigger rerouting |\n|tests/examples/example32.test.ts| Integration test for the fix |\n|tests/unit/removeDuplicateConsecutivePoints.test.ts| 8 unit tests for the dedup function + 2 for simplifyPath |\n\n## Test Results\n\n- 59 pass, 0 fail, 5 skip (full suite)\n- Zero snapshot regressions on existing tests\n- 8 new unit tests forremoveDuplicateConsecutivePoints\n- 1 new integration test (example32)\n- TypeScript type-check: clean (tsc --noEmitzero errors)\n\n/claim #78"