Skip to content

cosmetic: remove redundant vertex workaround from gerbv_draw_polygon#436

Merged
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/polygon-redundant-vertex
Mar 17, 2026
Merged

cosmetic: remove redundant vertex workaround from gerbv_draw_polygon#436
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/polygon-redundant-vertex

Conversation

@rampageservices
Copy link
Copy Markdown
Contributor

Closes #433.

What

gerbv_draw_polygon ran its loop with i <= numberOfSidesInteger (inclusive), revisiting the first vertex as a zero-length closing segment. The comment said:

include last point, since we may be drawing an aperture hole next and cairo may not correctly close the path itself

Why it is now dead code

PR #380 added cairo_new_sub_path() at the top of gerbv_draw_aperture_hole(). That call starts a fresh subpath before any hole geometry is drawn, which severs the implicit Cairo connection from the polygon endpoint to the hole. The workaround is no longer needed.

Change

  • Loop bound: i <= numberOfSidesIntegeri < numberOfSidesInteger
  • Remove the two comment lines that justified the old bound

No functional change. Golden test images are unaffected and do not need regeneration.

The loop in gerbv_draw_polygon ran i <= numberOfSidesInteger (inclusive)
to repeat the first vertex as a zero-length closing segment. The comment
explained this was needed because cairo might draw a spurious line from
the polygon endpoint to the start of an aperture hole subpath.

PR gerbv#380 fixed the root cause by adding cairo_new_sub_path() in
gerbv_draw_aperture_hole(), which starts a fresh subpath before drawing
any hole — severing the implicit connection entirely.

With that fix in place the extra vertex is dead code. Remove it and the
now-inaccurate comment. No functional change; golden images are unaffected.

Closes gerbv#433.
@spe-ciellt spe-ciellt merged commit 77cbd52 into gerbv:develop Mar 17, 2026
8 checks passed
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.

Cosmetic fix: gerbv_draw_polygon has a now-redundant workaround with a misleading comment

2 participants