-
Notifications
You must be signed in to change notification settings - Fork 1
fix board outline #49
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?
fix board outline #49
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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 (srj.outline && srj.outline.length > 2) { | ||
| boardVoidRects = computeInverseRects(bounds, srj.outline as any) | ||
| for (const voidRect of boardVoidRects) { | ||
| for (const voidRect of boardVoidRects ?? []) { | ||
| for (let z = 0; z < layerCount; z++) insertObstacle(voidRect, z) |
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.
Recompute void rects when caller omits boardVoidRects
The new buildObstacleIndexesByLayer only inserts board-outline voids from boardVoidRects ?? [], but boardVoidRects is optional in the input type. If a caller constructs RectDiffGridSolverPipeline directly (as was previously sufficient) and passes only srj with an outline, the outline will now be ignored and no void obstacles will be added, allowing expansions/routing into off‑board space. This is a regression from the prior behavior where buildObstacleIndexes always derived void rects from srj.outline.
Useful? React with 👍 / 👎.
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.
@codex It's fine, we only want to build it once at a top-level pipeline, then pass it as an array. We do not rebuild from simpleroutejson.
We should change the function signature to not take simplroute json at all and only take what is needed. Passing the entire SRJ is kind of misleading. Can you do this codex? I am lazy
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.
Summary
- Updated obstacle index construction to accept only obstacles, layer metadata, and board void rects.
- Moved layer metadata computation into the pipeline before building obstacle indexes.
Testing
- Not run (not requested).
The board outline was broken and was not respected by the new GapFillSolverPipeline; it was not caught in the test because the test only used the final visualization of RectDiffGridSolver. It had been updated to test the entire pipeline.