-
Notifications
You must be signed in to change notification settings - Fork 42
feat(solver): Add TraceMergerSolver to merge same-net trace lines #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements a new solver that merges trace segments belonging to the same electrical net when they are positioned close together (within threshold) and axially aligned (same X or Y coordinate). This improves schematic readability by consolidating parallel traces that should logically be a single path. Fixes tscircuit#34
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rushabhcodes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to recreate an example of the problem, then show that you fixed the problem in a snapshot
Added side-by-side visualization that demonstrates: - Before: Traces as generated by SchematicTraceLinesSolver (red) - After: Traces after TraceMergerSolver processing (green) This shows the problem being fixed - close parallel traces on the same net are merged into cleaner paths.
|
Added a before/after comparison snapshot as requested. The new test (
This demonstrates the problem being solved - when traces on the same net have parallel segments that are close together, they get merged into cleaner paths. See: |
|
Friendly ping for review! I've added the before/after comparison snapshot as requested. Let me know if any other changes are needed. |
rushabhcodes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dont add thses type of test tests/solvers/__snapshots__/before_after_comparison.snap.svg
Just run the test twice with and without the changes and share the results in comments
Keep tests/solvers/__snapshots__/TraceMergerSolver.snap.svg
Removed before/after comparison snapshot test as requested. Kept simple pipeline solver snapshot test.
|
I've simplified the test file as requested - removed the before/after comparison snapshot. Test Results: All tests pass: The TraceMergerSolver is integrated into the pipeline and works by:
Ready for another review! |
/claim #34
Summary
TraceMergerSolverthat merges trace segments belonging to the same electrical net when they are positioned close together and axially alignedTraceOverlapShiftSolverandNetLabelPlacementSolverImplementation Details
The solver:
globalConnNetIdTest plan
TraceMergerSolver.test.tswith a multi-pin GND net scenariobun test- 29 pass, 1 skip, 0 fail)Debug Data
Fixes #34