-
-
Notifications
You must be signed in to change notification settings - Fork 54
Cosmetic fix: gerbv_draw_polygon has a now-redundant workaround with a misleading comment #433
Description
Cosmetic
gerbv_draw_polygon has a now-redundant workaround with a misleading comment
/* skip first point, since we've moved there already */
/* include last point, since we may be drawing an aperture hole next
and cairo may not correctly close the path itself */
for (i = 1; i <= (int)numberOfSidesInteger; i++) {
...
cairo_line_to(...); /* deliberately repeats the first vertex */
}This comment (and the loop bound i <= numberOfSidesInteger instead of
i < numberOfSidesInteger) exists precisely because of the bug this PR fixes:
the author previously closed the polygon back to the start vertex so that the
implicit line Cairo would draw to the hole-arc start would be zero-length and
invisible. With cairo_new_sub_path() now in place, the workaround is no
longer needed and the comment is misleading.
The loop bound change would be a separate correctness question (closing vs.
not closing the polygon explicitly), but the comment should at minimum be
removed or updated so future readers are not confused about why the polygon is
drawn the way it is. Cosmetic, but likely to cause confusion in code review
later.
Originally posted by @spe-ciellt in #380 (comment)