Add same-net trace combining phase#368
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07e441c29e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| !this.isStrictlyBetween(vertical.axis, horizontal.min, horizontal.max) | ||
| ) { | ||
| return null | ||
| } | ||
| if (!this.isStrictlyBetween(horizontal.axis, vertical.min, vertical.max)) { | ||
| return null |
There was a problem hiding this comment.
Treating orthogonal intersections as valid only when both coordinates are strictly inside each segment misses new endpoint contacts between different nets. If a moved segment endpoint lands on another-net segment (a T-junction or endpoint touch), getSegmentIntersectionKey returns null, so canApplyCandidate can accept a geometry that electrically shorts nets. This path is reachable whenever an internal segment is shifted onto a nearby axis and one end aligns with an existing different-net segment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a5cc264: endpoint contacts now count as intersections during candidate validation, including orthogonal endpoint touches and collinear endpoint touches. That makes canApplyCandidate reject snaps that would create a new different-net touch. Added a regression for the exact endpoint-touch case.
Verification:
bun test tests/solvers/SameNetTraceCombiningSolver/SameNetTraceCombiningSolver.test.tsbun run format:checkbunx tsc --noEmitbun testbun run buildgit diff --check
| const oldIntersections = this.getDifferentNetIntersectionKeys(sourceTrace) | ||
| const newIntersections = this.getDifferentNetIntersectionKeys(proposedTrace) | ||
|
|
||
| for (const key of newIntersections) { | ||
| if (!oldIntersections.has(key)) return false | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
Candidate acceptance only compares different-net trace intersections and never checks chip obstacle crossings, so a snap can move a legal segment into a chip/body obstacle and still be accepted. Because this phase runs after cleanup in the pipeline and no later phase globally revalidates trace-vs-chip collisions, the final routed traces can become physically invalid for some layouts with nearby parallel same-net segments and tight chip clearances.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a5cc264: same-net snap validation now compares chip-obstacle collision keys before and after the proposed move and rejects any newly introduced chip/body collision. Pin endpoint chips are only excluded when the segment leaves the chip rather than entering it. Added a regression where the moved connector would enter a nearby chip obstacle.
Verification:
bun test tests/solvers/SameNetTraceCombiningSolver/SameNetTraceCombiningSolver.test.tsbun run format:checkbunx tsc --noEmitbun testbun run buildgit diff --check
1 participant
/claim #29
Summary
SameNetTraceCombiningSolverpost-cleanup pipeline phaseglobalConnNetIdand snap close overlapping same-net internal horizontal/vertical segments onto a shared axisVerification
bun test tests/solvers/SameNetTraceCombiningSolver/SameNetTraceCombiningSolver.test.tsbun testbunx tsc --noEmitbun run format:checkbun run build