refactor(pipeline): Decompose overlap solver #76
Conversation
|
@0hmX is attempting to deploy a commit to the tscircuit Team on Vercel. A member of the Team first needs to authorize it. |
0hmX
changed the title
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Deployed |
|
@0hmX you need to add jsdoc strings to each new solver, and honestly you should probably make them subsolvers because this increases the complexity of the pipeline. Where possible visually demonstrate the input/output to these solvers using stackGraphicsHorizontally |
|
might exceed the iteration limit of all is merged together! i will give it a try |
|
It worked out! |
…nal label placement The monolithic TraceLabelOverlapAvoidanceSolver has been decomposed into a more modular, multi-step pipeline to improve separation of concerns: - LabelMergingSolver: Merges adjacent net labels into single targets. - TraceLabelOverlapAvoidanceSolver: Reroutes traces around merged labels. - TraceCleanupSolver: Optimizes the geometry of modified traces. This refactoring revealed a bug where the final label placement was skipped if no traces were rerouted. A new, unconditional `finalNetLabelPlacementSolver` step has been added to the pipeline to fix this, ensuring labels are always drawn correctly.
|
We already have snapshots to test the NetLabelMerging behavior! |
|
Hope this PR makes it easier to understand what is going on under the hood! and visualized because that is the entire point of the refactor! |
| longDistancePairSolver?: LongDistancePairSolver | ||
| traceOverlapShiftSolver?: TraceOverlapShiftSolver | ||
| netLabelPlacementSolver?: NetLabelPlacementSolver | ||
| labelMergingSolver?: MergedNetLabelObstacles |
There was a problem hiding this comment.
| labelMergingSolver?: MergedNetLabelObstacles | |
| labelMergingSolver?: MergedNetLabelObstacleSolver |
| return [ | ||
| { | ||
| inputProblem: instance.inputProblem, | ||
| problem: instance.inputProblem, |
There was a problem hiding this comment.
| problem: instance.inputProblem, | |
| inputProblem: instance.inputProblem, |
There was a problem hiding this comment.
one of our principles is variable transparency https://github.com/tscircuit/handbook/blob/main/guides/code.md
|
@seveibar I hope I am not pushing change that you feel are not needed at the moment. I don't want to be a jerk and push changes and refractors that just lead to wastage of maintainer time and resources. If this is not a real priority, we can close this. Maybe make some sub issues? That can be consumed progressively. I can still work on this , but I hope I am not being a jerk and sending changes just not needed 🙂 |
|
@0hmX you're not doing anything wrong, I gave your review too much grief in the video because we get a lot of spammy PRs The team is stressed working towards tscircuit/tscircuit#926 and this is why we're busy, this PR is good, and you're doing good work, just the timing is causing more brevity. |
2 participants
The TraceLabelOverlapAvoidanceSolver was doing too many things. And it was not using the iterative approach when solving! We fix that also add visualization for each step! a much more consistent system