Repository navigation
fix: rotate LED silkscreen rectangles with their footprints - #4816
Merged
ShiboSoftwareDev merged 7 commits intoOct 21, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| }, | ||
| width: finalWidth, | ||
| height: finalHeight, | ||
| ccw_rotation: isRotated90Degrees ? 0 : rotation, |
Contributor
There was a problem hiding this comment.
Could you also update getPcbSize() to account for arbitrary rotation? The rectangle now rotates, but its reported bounds still use the unrotated width/height. Please add a bounds regression test too.
| // orientation (+X right, +Y top, CCW degrees), including bottom-layer flips. | ||
| // Use the same placement matrix as SilkscreenPath's transformed corners. | ||
| const transform = this._computePcbGlobalTransformBeforeLayout() | ||
| const rotation = (Math.atan2(transform.b, transform.a) * 180) / Math.PI |
Contributor
There was a problem hiding this comment.
handbook violation
ShiboSoftwareDev
approved these changes
Oct 21, 2026
Contributor
|
Thank you for your contribution! 🎉 PR Rating: ⭐⭐ Track your contributions and see the leaderboard at: tscircuit Contribution Tracker |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the LED outline rotation reproduced in #4622. Rotating C540985 or C540986 by 45° now rotates the silkscreen rectangle together with its pads.
Use the footprint's full placement transform for both the rectangle orientation and its axis-aligned size, including bottom-layer mirroring.
getPcbSize()now reuses the existing rotated-rectangle bounds helper, so arbitrary angles report the space the rectangle occupies. Keep the existing width/height swap for 90° and 270° placements.The regression compares the rectangle corners and reported dimensions with an equivalent emitted path on both PCB layers and inside rotated groups. At 45°, the previous code reported a 6 mm width for a 6 × 2 mm rectangle; its actual bounds are approximately 5.657 × 5.657 mm. Transform names specify their coordinate spaces, and rotation names specify direction and units.
Before
After
Validation: 32 silkscreen-related tests passed, TypeScript passed, build passed, and changed TypeScript files pass formatting. The geometry regression fails against the original implementation; the added bounds assertions also fail against the previous PR revision. The silkscreen snapshots remain valid after correcting the bounds.