Fix orientation of the inner circular arc extras - #70
Merged
Merged
Conversation
'compute_circular_arc_extras_inner' built each circular segment between a chord and its arc clockwise (negative area), whatever the arc's own orientation, while 'compute_union' expects anticlockwise shapes, as 'compute_circular_arc_extras_outer' builds them. In the fallback union of 'approximate_by_line_segments', the extras of an anticlockwise hole arc then made the union fill the hole entirely instead of replacing its arcs by chords. Build them anticlockwise: the arc traversed anticlockwise, closed by the chord.
fontanf
force-pushed
the
fix-circular-arc-extras-inner-orientation
branch
from
September 23, 2026 13:55
d048aec to
3192076
Compare
This was referenced Sep 23, 2026
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
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.
Found while investigating fontanf/packingsolver#595.
Problem
When the line-segment approximation of a shape self-intersects,
approximate_by_line_segmentsfalls back to a union of the original shape with "extras": the circular segments between each arc and its chords (or tangents).compute_circular_arc_extras_innerbuilt these circular segments clockwise (negative area), whatever the arc's own orientation.compute_unionexpects anticlockwise shapes, which is howcompute_circular_arc_extras_outerbuilds them. For an anticlockwise hole arc, the union then filled the hole entirely instead of replacing its arcs by chords. This is a silently wrong result: on a 100 × 60 rectangle with a circular hole of radius 20, the output had no hole and area 6000.Fix
Build them anticlockwise: the arc traversed anticlockwise, closed by the chord.
Testing
approximate_by_line_segments/4.jsoncase: a 100 × 100 square with a thin half-annulus hole (anticlockwise outer arc of radius 30, clockwise inner arc of radius 25), with segment length 20. At that length, the tangent approximation of the inner arc crosses the chords of the outer arc, so the fallback union is used.main(with Fix degenerate line segment at the end of an on-circle arc approximation #69), the hole vanishes: the output is the full square, area 10000.