fix(ecad): Gerber apertures must turn with the pad#687
Merged
Conversation
`resolve_pads_on_layer` rotated each pad's POSITION by the footprint rotation and then emitted the aperture from the raw width/height. `ApertureShape` had nowhere to put an angle. So every non-square pad on a rotated footprint was flashed axis-aligned: U4's 0.875 x 0.25mm TQFN-48 pads on a 90°-turned part came out 0.875mm wide ALONG the 0.5mm pitch, and all 48 pads on a side merged into one continuous copper bar. A board built from these Gerbers is shorted, not merely misassembled. Zoomed out in GerbView it looks like a plausible package outline, which is how it survived. Apertures now carry a rotation, effective angle `fp.rotation + pad.rotation` (vcad's IR stores the pad angle relative to its footprint). Multiples of 90° fold into a width/height swap — exact, and it keeps the aperture table standard: 828 affected pads on CM5, 763 of them at 90°, and F_Cu gains only 3 macros. Off-quadrant angles emit an aperture macro per distinct (shape, angle) with literal numbers, using primitive 21 (centre line) for the body and primitive 1 for an obround's pre-rotated end caps. That is what KiCad itself emits: its `%AMRotRect` body is `21,1,$1,$2,0,0,$3`, and for the 0.3 x 0.94mm 45° pad KiCad writes `RotRect,0.300000X0.940000X45.000000` against our `21,1,0.300000,0.940000,0,0,45.000000` — the same primitive with the same numbers. `ApertureKey` grew a third slot holding the angle in micro-degrees, so two pads differing only in rotation cannot collapse into one wrongly turned aperture. RoundRect still degrades to a rect, now a rotated one. Circles are rotation-invariant and untouched. Cross-checked against KiCad's own export of the same board (`kicad-cli pcb export gerbers`), comparing every position-matched pad aperture across 8 layers as (long dim, short dim, angle mod 180): before 12085 / 13792 agree (1707 wrong) after 13527 / 13792 agree ( 0 wrong) The 265 that still differ are all F_Mask circles (0.24 vs 0.39mm) — a solder-mask expansion difference, rotation-invariant, present before this change and out of scope here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Choji has used up today's reviewsYour team has reached the free plan's limit of 5 reviews per day, so Choji stepped aside on this pull request. That's a plan limit, not a reflection of the change itself. Your free reviews reset tomorrow. To have Choji review every push without waiting, upgrade to Pro for unlimited reviews. You're on the free plan · 5 reviews/day. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
ecto
added a commit
that referenced
this pull request
Jul 25, 2026
Main moved during this work: #685 (Eagle pad rotation), #687 (Gerber apertures turn with the pad), and #691 (per-layer controlled-impedance geometry). #691 rewrites router/pair.rs and adds a fifth SI claim, so the corrected numbers were re-run against it rather than assumed still valid. Nothing moved. The stripped floor (311 / 23), our board's DRC (Clearance 173), and the 40-net route (byte-identical board, receipt 1.353 / 0.582 / 2.474) all reproduce exactly — #691's per-layer widths are opt-in on target_impedance, which the CM5 fixture's classes do not declare. The new pair_impedance_correct_fraction claim is vacuous on this fixture: it reports 0.000 against a bound of 0.000 with the reason "no differential net class declares a target impedance — nothing to verify". The receipt is now five claims wide, so the raw fraction reads 2 of 5 on the full board and 4 of 5 on the subset. Those are not improvements, and the docs say so — the honest counts against claims that actually verify something remain 1 of 4 and 3 of 4. Also resolves the merge conflict with main in router/pair.rs by taking main's version; this branch carries no crate changes, only docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ecto
added a commit
that referenced
this pull request
Jul 25, 2026
…lated baseline (#690) * fix(router): couple short pairs with a direct centerline A pair whose pads sit a few mm apart has no room for the lead-inset phantom centerline: the lead eats both ends and a ~1.8mm span keeps ~0.9mm to search, so both ladders fail and the pair falls back to two independent singles. Those singles land on DIFFERENT layers, and coupled_fraction only counts twin copper on the same layer — so the pair scores exactly 0.0 and pins the SI receipt's min_pair_coupled_fraction at zero however well the rest of the board routes (M7 named this: the /HS.* group). Such a pair does not need a search. Route it directly: one straight, lead-free centerline on the best layer all four pads share, offset into the two legs by the same realize_legs every other centerline uses. Four point probes choose the layer; the authoritative check is still validate_and_commit, so this adds no way to commit copper the oracle has not seen. CM5 pair census: 44 of 49 coupled, up from 39, and the centerline-search bail class — M7's dominant mode at 5-6 pairs — is now empty. The 5 that remain are long pairs failing at their connectors, a different mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ecad): KiCad pad angles are absolute — stop double-counting footprint rotation KiCad stores a pad's angle ABSOLUTELY: it already includes the footprint's orientation. Measured on the CM5 fixture, the pad angle tracks the footprint exactly — fp_rot -90 -> pad_rot 270 (798 pads), 45 -> 45, 90 -> 90 (285), 135 -> 135, 180 -> 180. vcad's IR stores it RELATIVE, because all eleven geometry consumers (DRC, the router's spatial index, DFM, pour synthesis, render, the 3D mesh, eval) compose fp.rotation + pad.rotation. The importer stored the absolute angle, so every rotated footprint had its footprint rotation counted twice. A TQFN's 0.25 x 0.875mm pads at 0.5mm pitch came out turned 90 degrees — long axis ALONG the pitch — so neighbouring pads overlapped. That is not a subtle error: it invented copper. Fixed at the import boundary (reader subtracts, writer re-composes) so all eleven consumers become correct without touching them, pinned by a round-trip test. Measured on the CM5 fixture, copper stripped: short/clearance 980 -> 311 Clearance 74 -> 23 So 648 of the violations this project has been attributing to the reverse-engineered source were ours. Every DRC-delta claim measured against that baseline needs re-measuring. Pair coupling census: 39 -> 47 of 49. The four /ETH.* pairs that pinned both broken SI claims couple now that their pads no longer overlap. Also here, both found while chasing those bails and each worth a pair on its own (46 -> 47 together): - short pairs get a direct, lead-free centerline (a ~1.8mm pad-to-pad span has no room for the lead inset, so it fell back to singles on different layers and scored exactly 0.0 coupling); - a pad connector retreats along the centerline and escalates its search window, because a pad inside a fine-pitch field must escape laterally before it can drop a via, and a window sized from a 0.8mm hop cannot contain that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(router): correct every CM5 DRC/SI claim measured against the inflated baseline vcad's KiCad importer stored pad angles as read, but KiCad's pad angle is absolute — it already includes the footprint's orientation — while eleven consumers compose `fp.rotation + pad.rotation`. Rotated fine-pitch packages therefore had overlapping pads, manufacturing phantom shorts and clearance violations (PR #684). Those phantoms landed in the stripped-fixture *baseline*, which is the denominator of every route-attributable claim we publish. A bigger floor makes our delta look smaller, so the error flattered us in every comparison — the direction that most needs correcting. Re-measured from scratch on merged main with #684 in. DRC and SI are deterministic given a fixed board file; every figure was run twice with identical results, and the stochastic route stage is reported with wall-clock. Numbers that moved: - stripped-fixture floor: 980 -> 311 short/clearance, 74 -> 23 Clearance (648 violations blamed on the reverse-engineered source were ours) - human production board: 16,485 -> 14,104 violations - our full board: "route-attributable ZERO" was FALSE (74 - 74 = 0 against the inflated floor); corrected, 173 vs floor 23 = 150 attributable - full-board receipt: 2/4 -> 1/4 (vias_per_si_net 2.766 -> 3.128, crossing the 3.0 bound as routability rose 0.983 -> 0.994) - 40-net "receipt Pass (ALL HOLD)" WITHDRAWN -> 3/4 (intra-pair skew 1.353 vs 1.100); it already failed to reproduce pre-fix at 1.347, so the Pass was a one-branch artifact rather than a result - pad-level shorts in the import: ~906 -> 258; 40-net route adds 27, not ~409 (the conclusion stands: Short is not route-attributable, use Clearance) - route wall-clock 113 min -> 958.9 s (16.0 min); full chain 27.2 min, so the M6 "<30 min chain" target is met - pair-first couples 47 pairs in round 0, up from 43 No receipt bound was adjusted. The human board's anchor re-measures bit-identical (9.756 / 1.074 / 0.857 / 2.265, ALL HOLD) because pad angles feed pad geometry, not trace lengths — so the envelope argument survives intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(router): add route-stage spread from a second run Second full-board route: routability 0.994 again, byte-identical board (3,791 segments / 895 vias) — the route is deterministic given the tree. Wall-clock 895.5 s vs run 1's 958.9 s. The chain figure stays at one complete run: run 2's si_finish stage was truncated when the machine's disk filled, so its 1,288 s is not a valid chain sample and is not averaged in. Said so in the doc rather than presenting it as a second measurement — a prior version of this document published a single lucky run as typical, which is the habit this correction exists to break. Both runs shared the machine with another session's full-board route, so the timings are pessimistic; the <30 min conclusion holds regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(router): re-verify the corrected CM5 figures against current main Main moved during this work: #685 (Eagle pad rotation), #687 (Gerber apertures turn with the pad), and #691 (per-layer controlled-impedance geometry). #691 rewrites router/pair.rs and adds a fifth SI claim, so the corrected numbers were re-run against it rather than assumed still valid. Nothing moved. The stripped floor (311 / 23), our board's DRC (Clearance 173), and the 40-net route (byte-identical board, receipt 1.353 / 0.582 / 2.474) all reproduce exactly — #691's per-layer widths are opt-in on target_impedance, which the CM5 fixture's classes do not declare. The new pair_impedance_correct_fraction claim is vacuous on this fixture: it reports 0.000 against a bound of 0.000 with the reason "no differential net class declares a target impedance — nothing to verify". The receipt is now five claims wide, so the raw fraction reads 2 of 5 on the full board and 4 of 5 on the subset. Those are not improvements, and the docs say so — the honest counts against claims that actually verify something remain 1 of 4 and 3 of 4. Also resolves the merge conflict with main in router/pair.rs by taking main's version; this branch carries no crate changes, only docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ecto
added a commit
that referenced
this pull request
Jul 25, 2026
The Gerber exporter had no representation for `PadShape::Custom`, so it flashed a 0.1mm circle placeholder and moved on. Every other consumer — DRC, the router, the copper-pour engine, the renderer — treats the full polygon as copper, so the board that came out of fabrication was missing copper the whole toolchain believed was there. A thermal tab, a shielding paddle or a stub antenna shrank to a dot with nothing reporting it. Custom pads are now painted as G36/G37 regions. `AbsolutePad` carries an `AbsolutePadGeom`: `Flash(ApertureShape)` for the standard pads, which is what it always did, and `Region(Vec<Vec2>)` for polygons. No standard aperture describes an arbitrary polygon, and a region is the portable way to say it. The vertices are pad-local, so they are turned by the same board-frame angle the flashed shapes use (`footprint.rotation + pad.rotation`, the convention #687 established) and then translated to the pad centre. That convention is taken from the existing consumers rather than invented: vcad-render's pcb.rs and vcad-ecad-pcb's copper_pour.rs both read these vertices as local and centred. The test pins what the placeholder hid — a 2.0 x 0.5mm polygon bar keeps its size, lands on the pad, and stands up under either a 90 degree footprint rotation or a 90 degree pad rotation — by measuring the emitted region's board-frame extents and centre, and asserting the 0.1mm aperture is gone. This bug survived because nothing asserted on it. Found while preparing the CM5 fab package; that fixture has one custom pad out of 3037. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Reopens #686 against
main. That PR was stacked on #684 and got merged into the now-stalefix/short-pair-couplingbranch instead of reachingmain, so the fix is not in the tree.Same single commit, rebased onto
main(which has #684 as e1a5d90) and re-verified there —numbers below are from the rebased build.
The bug
resolve_pads_on_layerrotated each pad's POSITION by the footprint rotation and then emitted the aperture from the rawwidth/height.ApertureShapehad nowhere to put an angle at all.So every non-square pad on a rotated footprint was flashed axis-aligned. On
.scratch/CM5RevEng.kicad_pcb, 828 pads — 763 at an effective 90°, 51 at 45°, 14 at 135°. U4's 0.875 x 0.25mm TQFN-48 pads on a 90°-turned part came out 0.875mm wide along the 0.5mm pitch, so all 48 pads on a side merged into one continuous bar. That board is shorted, not merely misassembled.Rendering the actual F_Cu flashes around U4 (
/tmp/u4-before.svg,/tmp/u4-after.svg, reproducible fromthe Gerbers): before, each side of the package is one unbroken bar of copper — 48 pads fused.
After, 48 discrete pads, long-axis outward.
The fix
Apertures carry a rotation. Effective angle is
fp.rotation + pad.rotation(vcad's IR stores the pad angle relative to its footprint).ApertureKeygrew a third slot holding the angle in micro-degrees, so two pads differing only in rotation cannot collapse into one wrongly-turned aperture.This is what KiCad itself emits. Its
%AMRotRectbody is21,1,$1,$2,0,0,$3; for the 0.3 x 0.94mm 45° pad on this board KiCad writesRotRect,0.300000X0.940000X45.000000against our21,1,0.300000,0.940000,0,0,45.000000— the same primitive with the same numbers.Acceptance evidence: cross-check against KiCad's own export
kicad-cli pcb export gerberson the same.kicad_pcb, then every position-matched pad aperture compared as (long dim, short dim, angle mod 180), tolerance 2µm / 0.01°:Per-footprint: U4 52/52 agree, and the 0402s sampled at 90°/270° (C50–C64) 2/2 each.
The 265 that still differ are all F_Mask circles (0.24 vs 0.39mm) — a solder-mask expansion difference. Circles are rotation-invariant, this was present before the change (413 F_Mask disagreements before = 148 rotation + 265 mask-expansion), and it is out of scope here.
Tests
New in
gerber.rs:rect_pad_on_rotated_footprint_swaps_axes— 0.25 x 0.875 pad at fp 90 / pad 90 / fp 45+pad 45 / fp -90 all put the long axis on the right board axisrect_pad_at_45_degrees_emits_rotated_macro— 45° emits%AM…21,1,…,45and the%AMprecedes its%ADD; asserts it does not degrade to an axis-aligned rectaperture_dedup_separates_rotations— 0/90/45/135 are four apertures, but 180≡0, -90≡90, 225≡45 still dedupoval_pad_rotation— quadrant folding, square-obround-is-a-circle, and pre-rotated cap centres at 45°roundrect_pad_rotatescargo test -p vcad-ecad-export32 passed;-p vcad-ecad-pcb288;-p vcad-ecad-symbols115.cargo clippy -p vcad-ecad-export --all-targets -- -D warningsclean.🤖 Generated with Claude Code