From 319207673f9a1afb3dc8f0cb0b5f4e071e1f91c9 Mon Sep 17 00:00:00 2001 From: Florian Fontan Date: Wed, 23 Sep 2026 15:44:22 +0200 Subject: [PATCH] Fix orientation of the inner circular arc extras '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. --- .../approximate_by_line_segments/4.json | 317 ++++++++++++++++++ src/approximation.cpp | 15 +- test/approximation_test.cpp | 7 + 3 files changed, 333 insertions(+), 6 deletions(-) create mode 100644 data/tests/approximation/approximate_by_line_segments/4.json diff --git a/data/tests/approximation/approximate_by_line_segments/4.json b/data/tests/approximation/approximate_by_line_segments/4.json new file mode 100644 index 0000000..cadc4bd --- /dev/null +++ b/data/tests/approximation/approximate_by_line_segments/4.json @@ -0,0 +1,317 @@ +{ + "expected_output": { + "elements": [ + { + "end": { + "x": 0.0, + "y": 100.0 + }, + "start": { + "x": 100.0, + "y": 100.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 0.0, + "y": 0.0 + }, + "start": { + "x": 0.0, + "y": 100.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 100.0, + "y": 0.0 + }, + "start": { + "x": 0.0, + "y": 0.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 100.0, + "y": 100.0 + }, + "start": { + "x": 100.0, + "y": 0.0 + }, + "type": "LineSegment" + } + ], + "holes": [ + { + "elements": [ + { + "end": { + "x": 25.0, + "y": 50.0 + }, + "start": { + "x": 20.0, + "y": 50.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 25.0, + "y": 62.071067811865476 + }, + "start": { + "x": 25.0, + "y": 50.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 20.0, + "y": 50.0 + }, + "start": { + "x": 25.0, + "y": 62.071067811865476 + }, + "type": "LineSegment" + } + ], + "is_path": false, + "type": "general" + }, + { + "elements": [ + { + "end": { + "x": 49.999999999999986, + "y": 78.86751345948127 + }, + "start": { + "x": 26.286262797369307, + "y": 65.17638090205041 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 73.71373720263071, + "y": 65.17638090205043 + }, + "start": { + "x": 49.999999999999986, + "y": 78.86751345948127 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 71.21320343559643, + "y": 71.21320343559643 + }, + "start": { + "x": 73.71373720263071, + "y": 65.17638090205043 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 50.0, + "y": 80.0 + }, + "start": { + "x": 71.21320343559643, + "y": 71.21320343559643 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 28.786796564403577, + "y": 71.21320343559643 + }, + "start": { + "x": 50.0, + "y": 80.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 26.286262797369307, + "y": 65.17638090205041 + }, + "start": { + "x": 28.786796564403577, + "y": 71.21320343559643 + }, + "type": "LineSegment" + } + ], + "is_path": false, + "type": "general" + }, + { + "elements": [ + { + "end": { + "x": 75.0, + "y": 50.0 + }, + "start": { + "x": 74.99999999999997, + "y": 62.07106781186556 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 80.0, + "y": 50.0 + }, + "start": { + "x": 75.0, + "y": 50.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 74.99999999999997, + "y": 62.07106781186556 + }, + "start": { + "x": 80.0, + "y": 50.0 + }, + "type": "LineSegment" + } + ], + "is_path": false, + "type": "general" + } + ], + "is_path": false, + "type": "general" + }, + "segment_length": 20.0, + "shape_with_holes": { + "elements": [ + { + "end": { + "x": 100.0, + "y": 0.0 + }, + "start": { + "x": 0.0, + "y": 0.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 100.0, + "y": 100.0 + }, + "start": { + "x": 100.0, + "y": 0.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 0.0, + "y": 100.0 + }, + "start": { + "x": 100.0, + "y": 100.0 + }, + "type": "LineSegment" + }, + { + "end": { + "x": 0.0, + "y": 0.0 + }, + "start": { + "x": 0.0, + "y": 100.0 + }, + "type": "LineSegment" + } + ], + "holes": [ + { + "elements": [ + { + "center": { + "x": 50.0, + "y": 50.0 + }, + "end": { + "x": 20.0, + "y": 50.0 + }, + "orientation": "Anticlockwise", + "start": { + "x": 80.0, + "y": 50.0 + }, + "type": "CircularArc" + }, + { + "end": { + "x": 25.0, + "y": 50.0 + }, + "start": { + "x": 20.0, + "y": 50.0 + }, + "type": "LineSegment" + }, + { + "center": { + "x": 50.0, + "y": 50.0 + }, + "end": { + "x": 75.0, + "y": 50.0 + }, + "orientation": "Clockwise", + "start": { + "x": 25.0, + "y": 50.0 + }, + "type": "CircularArc" + }, + { + "end": { + "x": 80.0, + "y": 50.0 + }, + "start": { + "x": 75.0, + "y": 50.0 + }, + "type": "LineSegment" + } + ], + "is_path": false, + "type": "general" + } + ], + "is_path": false, + "type": "general" + } +} diff --git a/src/approximation.cpp b/src/approximation.cpp index 79b11fa..ebe54fe 100644 --- a/src/approximation.cpp +++ b/src/approximation.cpp @@ -301,21 +301,24 @@ std::vector compute_circular_arc_extras_inner( } //std::cout << "point_circle " << point_circle.to_string() << std::endl; + // The circular segment between the chord and the arc, anticlockwise + // like every shape passed to 'compute_union' (the arc is always + // traversed anticlockwise, the chord closes it). Shape shape; if (circular_arc.orientation != shape::ShapeElementOrientation::Clockwise) { - shape.elements.push_back(build_line_segment(point_circle_prev, point_circle)); shape.elements.push_back(build_circular_arc( - point_circle, point_circle_prev, + point_circle, circular_arc.center, - ShapeElementOrientation::Clockwise)); + ShapeElementOrientation::Anticlockwise)); + shape.elements.push_back(build_line_segment(point_circle, point_circle_prev)); } else { + shape.elements.push_back(build_line_segment(point_circle_prev, point_circle)); shape.elements.push_back(build_circular_arc( - point_circle_prev, point_circle, + point_circle_prev, circular_arc.center, - ShapeElementOrientation::Clockwise)); - shape.elements.push_back(build_line_segment(point_circle, point_circle_prev)); + ShapeElementOrientation::Anticlockwise)); } output.push_back(shape); point_circle_prev = point_circle; diff --git a/test/approximation_test.cpp b/test/approximation_test.cpp index c699a27..2b1c0a0 100644 --- a/test/approximation_test.cpp +++ b/test/approximation_test.cpp @@ -286,6 +286,13 @@ INSTANTIATE_TEST_SUITE_P( // is not positive" (fontanf/packingsolver#595). ApproximateByLineSegmentsTestParams::read_json( (fs::path("data") / "tests" / "approximation" / "approximate_by_line_segments" / "3.json").string()), + // Thin half-annulus hole with a large segment length: the tangent + // approximation of its clockwise inner arc crosses the chords of + // its anticlockwise outer arc, so the fallback union is used. The + // circular segments between the outer arc's chords and the arc + // used to be clockwise, and the union filled the hole entirely. + ApproximateByLineSegmentsTestParams::read_json( + (fs::path("data") / "tests" / "approximation" / "approximate_by_line_segments" / "4.json").string()), }), [](const testing::TestParamInfo& info) { return fs::path(info.param.name).stem().string();