Skip to content

fix: maintain a 4:3 aspect ratio when packing distant schematic groups - #211

Open
anil08607 wants to merge 3 commits into
tscircuit:mainfrom
anil08607:fix/issue-14-distant-group-aspect-ratio-1
Open

fix: maintain a 4:3 aspect ratio when packing distant schematic groups#211
anil08607 wants to merge 3 commits into
tscircuit:mainfrom
anil08607:fix/issue-14-distant-group-aspect-ratio-1

Conversation

@anil08607

Copy link
Copy Markdown
Contributor

Fixes #14.

The partition packer could stack distantly related schematic groups vertically when they shared only a global rail such as GND. This caused schematics to grow vertically and produce an unbalanced portrait layout.

Changes

  • Prefer horizontal placement for two distant movable partitions sharing at most one network.
  • Preserve the configured partition gap between repositioned groups.
  • Keep fixed-position partitions unchanged.
  • Preserve existing behavior for partitions connected through multiple networks.
  • Update the regression test to enforce a minimum 4:3 layout aspect ratio.
  • Regenerate repro-tall-growing-schematic.snap.svg.
  • Fix the RP2040 repro input typing so the CI type-check passes.
  • Apply the required formatting changes.

Testing

  • bun run format:check — passed
  • bunx tsc --noEmit — passed
  • Relevant tests — 5 passed, 0 failed
  • git diff --check — passed

@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
matchpack Ready Ready Preview Aug 8, 2026 4:22am

Request Review

if (deltaX >= deltaY) return packedComponents

const anchorIndex = Number(anchor.componentId.replace("partition_", ""))
const movingIndex = Number(moving.componentId.replace("partition_", ""))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!

@seveibar seveibar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using strings isn't allowed, i suspect this implementation is more of a hack than a proper aspect ratio aware solution

@seveibar seveibar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry meant to reject

@seveibar

seveibar commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

there is already string replacement being used but we shouldn't make the problem worse

I don't know what the correct solution for this is, but i think this is more of a hack, i'd need to see evidence of other solutions being trialed and actually a selection process

@mohan-bee mohan-bee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how helpful is this .. becuase this may regress the trace solver to fallback to netlabel instead of routing

@MustafaMulla29

Copy link
Copy Markdown
Contributor

@anil08607 mohan-bee has a point, whenever you are making contributions to matchpack or schematic-trace-solver you should always bun link it in core and see the snapshot changes are actually improvements or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintain 4:3 ratio when laying out distant groups (prevent "tall growing schematic")

4 participants