Skip to content

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

@spe-ciellt

Description

@spe-ciellt

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions