Support circular arcs in no_fit_polygon - #51
Merged
Conversation
Decomposes a ShapeWithHoles into convex polygons and circular segments (a convex arc closed by its chord, plus the arc's circle for exact tangent-based separation). Convex arcs on the outer boundary and holes are cut off; holes are stored anticlockwise like the outer boundary, so a Clockwise arc there is convex-from-material (material bulging into the void) and gets reversed when stored standalone so the segment shape stays a valid anticlockwise Shape. Arcs subtending >= 180 degrees, or whose chord would touch any other element of the shape (any intersection category, anywhere but the chord's own endpoints), are subdivided at their midpoint and retried.
The convex-convex core previously required a pure polygon (no CircularArc elements). Generalize the rotating-calipers merge to treat an arc as a continuous range of tangent directions instead of a single vector, splitting an arc where the other shape's direction falls inside its span, and summing two overlapping arcs into one (their Minkowski sum over the shared direction range is itself a circular arc: center = sum of centers, radius = sum of radii). Two issues had to be worked around along the way: - strictly_lesser_angle's fixed branch cut can flip a negated arc's start/end relative order, since negation shifts both by a constant 180 degrees independently of where that cut falls. Arc-internal comparisons now use angle_radian-based relative offsets instead, which have no fixed cut. - Splitting an arc reactively, only when the merge loop's current element happens to be that arc, misses interactions whenever the arc isn't the first element processed on its side. Arcs are now pre-split against the other shape's critical directions (including the global tangent-order minimum) before the merge runs, and the starting element is chosen by tangent direction rather than geometric position (bottom/top-most vertex), which stops being a reliable proxy once an arc can span across it. The general (non-convex) overload now decomposes via decompose_into_basic_shapes instead of compute_convex_partition, so arc-containing shapes work end-to-end through it too. Add oracle-verified test cases (segment vs polygon, self-NFP, overlapping/disjoint arc-arc, both orderings of arc vs polygon).
The ray-casting winding count classified an arc endpoint's crossing direction from the tangent's sign at that exact point. When the endpoint coincides with the circle's own topmost/bottommost point, the tangent there is exactly horizontal (neither up nor down), so that classification is undefined and previously defaulted to the wrong answer, miscounting a mere tangent touch as a genuine crossing. The fix only needs to know which extremum (top or bottom) the touched point is: y has a strict local maximum at the top and minimum at the bottom, so moving away from (or approaching) it is always in a fixed direction regardless of the arc's orientation, span, or where its other endpoint ends up -- unlike, e.g., comparing the two endpoints' y-coordinates directly, which would need a separate argument for why that comparison is even valid here. Add a minimal repro (one CircularArc + one LineSegment) to ShapeContainsTest.
Generalizes the convex-convex core from at most one arc per shape to any number, so callers no longer need decompose_into_basic_shapes for shapes like an inflated convex polygon, which rounds every corner into its own arc. Also fixes a latent bug in the merge loop's tie-handling: comparing tangent directions with strictly_lesser_angle (an exact, zero-tolerance cross-product test) spuriously breaks ties between directions that are mathematically identical but differ by ~1e-16 due to sqrt/trig noise -- common once multiple arcs are involved (e.g. any self-NFP), rare with a single arc.
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
1 participant
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.
Summary
decompose_into_basic_shapes, decomposing aShapeWithHolesinto convex polygons and circular segments (a convex arc closed by its chord, plus the arc's circle for exact tangent-based separation).no_fit_polygonconvex-convex core: treat an arc as a continuous range of tangent directions, splitting arcs where the other shape's direction falls inside their span and summing overlapping arcs (Minkowski sum of two overlapping arcs is itself an arc: center = sum of centers, radius = sum of radii). The general (non-convex) overload now usesdecompose_into_basic_shapesinstead ofcompute_convex_partition, so arc-containing shapes work end-to-end.Shape::containsfor arcs tangent to the ray at start/end, where the tangent direction is undefined at the circle's topmost/bottommost point and previously defaulted to miscounting a mere tangent touch as a genuine crossing.strictly_lesser_angle's zero-tolerance cross-product test spuriously broke ties between mathematically identical directions differing by ~1e-16 due to sqrt/trig noise.Test plan
./build_claude/test/Shape_shape_test— full suite passesno_fit_polygontest cases: segment vs polygon, self-NFP, overlapping/disjoint arc-arc, both orderings of arc vs polygonShapeContainsTestrepro for the tangent-at-extremum fix