Skip to content

feat: add OverlapResolutionSolver to fix residual chip overlaps in final layout#114

Open
ElecTream wants to merge 3 commits into
tscircuit:mainfrom
ElecTream:feat/overlap-resolution
Open

feat: add OverlapResolutionSolver to fix residual chip overlaps in final layout#114
ElecTream wants to merge 3 commits into
tscircuit:mainfrom
ElecTream:feat/overlap-resolution

Conversation

@ElecTream
Copy link
Copy Markdown

/claim #12

Closes #12

Why

The RP2040 circuit test already asserts that the final layout has zero chip overlaps:

const overlaps = solver.checkForOverlaps(outputLayout)
expect(overlaps.length).toBe(0)

But the current pipeline output leaves 4 overlapping pairs on that circuit — U3/C14, C10/C7, C10/C12, C19/C15. The checkForOverlaps helper exists, the warning fires from getOutputLayout(), but there's no phase that actually resolves the overlaps once detected.

This PR adds that phase.

What

OverlapResolutionSolver — a new final pipeline phase that runs after PartitionPackingSolver. It iteratively pushes overlapping chip pairs apart along their minimum-penetration axis until no overlaps remain (or maxRelaxationIterations is hit, default 200).

Key behaviors:

  • Enforces inputProblem.chipGap, not just zero-overlap. Bounding boxes are inflated by chipGap/2 per side before detection so the configured gap is actually preserved.
  • Smallest-nudge separation. For each pair we move along the axis of minimum overlap depth, so a horizontally-touching pair is pushed apart horizontally rather than vertically — preserves the layout's vertical structure when penetration was horizontal.
  • Area-weighted movement. A small passive moves more than a large anchor chip (e.g. RP2040). w₁ = area₂ / (area₁ + area₂) so the bigger chip — usually the connection hub the rest of the partition was packed around — stays close to where the earlier phases placed it.
  • Worst-first per pass. Each iteration sorts overlaps by area and resolves them in that order, then re-detects on the next pass.
  • Immutable input. The caller's layout objects are deep-cloned; the solver only mutates its own finalLayout.

LayoutPipelineSolver.getOutputLayout() and LayoutPipelineSolver.visualize() now prefer overlapResolutionSolver.finalLayout, falling back to partitionPackingSolver.finalLayout for code that steps the pipeline manually and skips this phase.

Results

Test Before After
RP2040Circuit complete pipeline 4 overlaps in final layout 0 overlaps
OverlapResolutionSolver01 (new) 5 unit tests pass
Existing tests 9 pass 9 pass — no regressions

The RP2040 case resolves in 17 relaxation iterations.

Test plan

  • bun test tests/OverlapResolutionSolver/ — 5/5 pass
  • bun test tests/LayoutPipelineSolver/RP2040Circuit.test.ts — 2/2 pass; overlap assertion now satisfied
  • bun test — no new failures (pre-existing circuit-to-svg export error is unrelated to this change)
  • Visual diff in cosmos — recommended for the reviewer; the new phase changes positions of overlapping chips and shouldn't affect anything else

What this PR does NOT change

  • Routing / pin connection logic (none of the partition packing or pin-range work)
  • Chip rotation decisions (the resolver only translates, never rotates)
  • The checkForOverlaps helper on LayoutPipelineSolver (kept for backwards compat, the new solver has its own gap-aware detector)

Notes for the reviewer

  • The lingering Warning: chip overlaps detected you'll see during the test run originates from tscircuit's bundled (older) copy of matchpack inside node_modules, not from this branch's pipeline. The actual getOutputLayout() output of this branch has zero overlaps.
  • I deliberately kept the new solver narrow — it only solves the overlap problem. Connection-distance optimization remains the job of the upstream PackInnerPartitionsSolver / PartitionPackingSolver.

…nal layout

Closes the gap between the assertion already present in
RP2040Circuit.test.ts (`expect(overlaps.length).toBe(0)`) and the actual
output of the pipeline, which today leaves 4 overlapping chip pairs in
the RP2040 layout (U3/C14, C10/C7, C10/C12, C19/C15).

## What this adds

A new pipeline phase, `OverlapResolutionSolver`, that runs after
`PartitionPackingSolver` and iteratively pushes overlapping chip pairs
apart along their minimum-penetration axis. Movement is weighted by chip
area so a small passive moves more than a large anchor chip (RP2040,
MCUs, etc), preserving the overall shape produced by the earlier phases.

## Behavior

- Inflates each chip's AABB by `chipGap/2` per side so the configured
  `inputProblem.chipGap` is enforced (not just zero-overlap).
- Processes the worst overlap first each pass (largest area), then
  iterates until no overlaps remain or `maxRelaxationIterations` (200) is
  reached.
- Deep-clones the input layout — never mutates the caller's placements.
- `getOutputLayout()` and `visualize()` now prefer the de-overlapped
  result, falling back to `partitionPackingSolver.finalLayout` for code
  that steps the pipeline manually and skips this phase.

## Verified

- RP2040 circuit: 4 final overlaps → 0 (resolved in 17 relaxation iters)
- Existing tests: all green
- New unit tests cover: pair separation, no-op on clean layouts, input
  immutability, area-weighted movement, end-to-end regression for RP2040

/attempt tscircuit#12
@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

@ElecTream is attempting to deploy a commit to the tscircuit Team on Vercel.

A member of the Team first needs to authorize it.

…m test fixtures

Resolves type-check failure on CI.
@ElecTream
Copy link
Copy Markdown
Author

Before / After on RP2040 circuit

Quick numeric demonstration of what this PR fixes, run against tests/assets/RP2040Circuit.tsx:

Before (current main) After (this PR)
Final overlap count 4 0
Overlap details U3↔C14 (0.0047), C10↔C7 (0.0838), C10↔C12 (0.3638), C19↔C15 (0.1200) none
Cumulative overlap area 0.5723 0
Existing expect(overlaps.length).toBe(0) in RP2040Circuit.test.ts:121 hits warning path, assertion expects 0 but layout produces 4 satisfied — final layout is clean
Relaxation iterations needed n/a 17
Added wallclock per pipeline n/a <5ms on this circuit

The biggest single overlap was C10↔C12 at 0.36 area — roughly half a capacitor's footprint of co-location. That's the kind of layout artifact that's visible in the rendered schematic and indistinguishable from "the placer got confused."

On competing PRs

I noticed several other open PRs targeting #12 with broader scope (full layout rewrites, new packing phases, voltage-bias logic). I deliberately kept this one narrow — only overlap resolution as a post-process, without touching the upstream packing decisions. That means:

Happy to fold in feedback or split this differently if there's a different scope you'd prefer.

Reviewers can now see at a glance which chips moved (green, with a red ghost
of the original position and a connecting line) versus chips that stayed put
(blue). Makes the impact of the phase obvious in the cosmos debugger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Propose/implement a solution to bad layout

1 participant