Skip to content

Fix circle primitives in aperture macros ignoring rotation#286

Merged
spe-ciellt merged 2 commits intogerbv:developfrom
SourceParts:fix/amacro-circle-rotation
Feb 27, 2026
Merged

Fix circle primitives in aperture macros ignoring rotation#286
spe-ciellt merged 2 commits intogerbv:developfrom
SourceParts:fix/amacro-circle-rotation

Conversation

@rampageservices
Copy link
Copy Markdown
Contributor

@rampageservices rampageservices commented Feb 26, 2026 •
edited
Loading

Summary

  • Adds CIRCLE_ROTATION to the circle parameter enum and stores the 5th parameter (nuf_parameters = 5)
  • Applies cairo_rotate() before cairo_translate() when rendering circles, so off-center circles orbit around the origin — matching the existing pattern used by LINE20/LINE21
  • Rotates center coordinates in the bounding box calculation so auto-fit viewport includes all rotated positions
  • Caps memcpy to actual stack depth (s->sp) so macros with only 4 circle parameters (rotation omitted) don't read stale stack data; omitted trailing parameters default to 0 from the pre-existing memset
  • Accumulates external rotation in image merge (step-and-repeat)
  • Exports the 5th parameter in RS-274X output

The rotation is in degrees, not a spatial dimension, so it is intentionally excluded from the mm-to-inches unit conversion block.

Files changed

File Change
src/gerbv.h Add CIRCLE_ROTATION to enum
src/gerber.c nuf_parameters = 5, cap to s->sp, rotate center in bbox
src/draw.c Apply rotation before translation
src/gerb_image.c Accumulate rotation on image merge
src/export-rs274x.c Export circle rotation parameter

Test plan

  • New test file test-amacro-circle-rotation.gbx — single macro with 4 off-center circles at 0°/90°/180°/270° renders as a cross pattern (without fix, all 4 overlap at the 0° position)
  • RS-274X round-trip export preserves rotation values
  • Full test suite: 90 passed (up from 89), 0 new failures
  • example_am_test (uses 1,1,$1,0,0* with no rotation param) still renders correctly — the s->sp guard ensures omitted rotation defaults to 0

Fixes #254

@rampageservices
fix: apply rotation parameter for circle primitives in aperture macros
e7b36bb 
The Gerber spec defines circle primitive (code 1) with 5 parameters:
exposure, diameter, center_x, center_y, and rotation. gerbv only
stored 4 parameters (nuf_parameters = 4), silently discarding the
rotation. When a macro uses off-center circles with rotation, the
centers must orbit around the origin, but gerbv rendered them at
their unrotated positions.

Changes:
- Add CIRCLE_ROTATION to gerbv_aptype_macro_circle_index_t enum
- Set nuf_parameters = 5 for circle primitives so rotation is stored
- Cap memcpy to s->sp (actual stack depth) so macros with only 4
  circle parameters don't read stale stack data as rotation; omitted
  trailing parameters default to 0 from the memset
- Apply cairo_rotate before cairo_translate when rendering circles,
  matching the existing pattern used by LINE20/LINE21 primitives
- Rotate center coordinates in bounding box calculation so the
  viewport auto-fit includes all rotated circle positions
- Accumulate external rotation in image merge (step-and-repeat)
- Export the 5th parameter (rotation) in RS-274X output

The rotation is in degrees and is not a spatial dimension, so it is
intentionally excluded from the mm-to-inches unit conversion block.

Fixes gerbv#254
@rampageservices rampageservices force-pushed the fix/amacro-circle-rotation branch from d62decd to e7b36bb Compare February 26, 2026 14:58
@spe-ciellt spe-ciellt self-assigned this Feb 27, 2026
@spe-ciellt
Copy link
Copy Markdown
Contributor

spe-ciellt commented Feb 27, 2026

Thank you for this patch. If this is good enough I apply it else I will leave comments.

@spe-ciellt
Copy link
Copy Markdown
Contributor

spe-ciellt commented Feb 27, 2026

Code: The implementation is comprehensive and internally consistent:

  • cairo_rotate() before cairo_translate() in draw.c exactly matches the existing LINE20/LINE21 pattern; CIRCLE_ROTATION as the final enum value follows the same convention
  • The s->sp cap in gerber.c is the critical safety guard — without it, a 4-parameter circle (rotation omitted) would memcpy one element beyond what was pushed onto the stack; the cap limits the copy to s->sp elements and the pre-zeroed sam->parameter correctly defaults rotation to 0.0
  • The MM-to-inches conversion correctly excludes CIRCLE_ROTATION (rotation is dimensionless degrees, consistent with LINE20/LINE21)
    • The gerbv_rotate_coord() call in the bounding box path is essential — without it the auto-fit viewport would be wrong for rotated off-center circles
    • The old TODO: test circle macro center rotation comment in gerb_image.c is cleanly removed and replaced with correct accumulation

Before fix: Four off-center circles at 0°/90°/180°/270° all render at the 0° position (overlapping).
After fix: Correct cross pattern. 141,845 pixel difference vs develop confirms the rendering genuinely changes.

Tests: New test test-amacro-circle-rotation passes. example_am_test (4-parameter circles, no rotation) is a pre-existing Cairo-baseline failure unchanged by this PR — confirmed by checking develop directly. 94/104 overall; the 10 failures are all pre-existing Cairo-baseline mismatches — no regressions.

@spe-ciellt spe-ciellt merged commit 787dbc9 into gerbv:develop Feb 27, 2026
1 check passed
@rampageservices rampageservices deleted the fix/amacro-circle-rotation branch February 27, 2026 17:20
@rampageservices rampageservices mentioned this pull request Feb 27, 2026
Open
@spe-ciellt spe-ciellt added the fix Solution for a potential problem or omission. label Feb 28, 2026
@BrewTestBot BrewTestBot mentioned this pull request Feb 28, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@spe-ciellt spe-ciellt

Labels

fix Solution for a potential problem or omission.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Gerbv does not Acknowledge Rotation of Circle Primitives in Aperture Macros

2 participants

@rampageservices @spe-ciellt