From 46b7c9eaeb9cfbecb8a88c00f9c8e11f0e7df9e0 Mon Sep 17 00:00:00 2001 From: Barend Gehrels Date: Sun, 24 Nov 2024 12:54:42 +0100 Subject: [PATCH] fix: avoid blocking rings for some non union conditions Fixes #893 Fixes #1299 --- .../algorithms/detail/overlay/overlay.hpp | 33 +++++++++++-------- .../overlay/multi_overlay_cases.hpp | 6 ++++ test/algorithms/overlay/overlay_cases.hpp | 6 ++++ .../set_operations/difference/difference.cpp | 8 +++++ .../difference/difference_multi.cpp | 16 ++------- .../intersection/intersection.cpp | 2 ++ .../intersection/intersection_multi.cpp | 2 ++ .../algorithms/set_operations/union/union.cpp | 3 ++ .../set_operations/union/union_multi.cpp | 2 ++ 9 files changed, 51 insertions(+), 27 deletions(-) diff --git a/include/boost/geometry/algorithms/detail/overlay/overlay.hpp b/include/boost/geometry/algorithms/detail/overlay/overlay.hpp index 972f79f036..27e9142e7a 100644 --- a/include/boost/geometry/algorithms/detail/overlay/overlay.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/overlay.hpp @@ -102,38 +102,42 @@ inline void get_ring_turn_info(TurnInfoMap& turn_info_map, Turns const& turns, C = target_operation == operation_union ? operation_intersection : operation_union; + static const bool is_union = target_operation == operation_union; for (auto const& turn : turns) { bool cluster_checked = false; bool has_blocked = false; - if (is_self_turn(turn) && turn.discarded) + if (turn.discarded && (turn.method == method_start || is_self_turn(turn))) { - // Discarded self-turns don't count as traversed + // Discarded self-turns or start turns don't need to block the ring continue; } - for (auto const& op : turn.operations) + for (int i = 0; i < 2; i++) { + auto const& op = turn.operations[i]; + auto const& other_op = turn.operations[1 - i]; ring_identifier const ring_id = ring_id_by_seg_id(op.seg_id); - if (! is_self_turn(turn) - && ( - (BOOST_GEOMETRY_CONDITION(target_operation == operation_union) - && op.enriched.count_left > 0) - || (BOOST_GEOMETRY_CONDITION(target_operation == operation_intersection) - && op.enriched.count_right <= 2))) + // If the turn (one of its operations) is used during traversal, + // and it is an intersection or difference, it cannot be set to blocked. + // This is a rare case, related to floating point precision, + // and can happen if there is, for example, only one start turn which is + // used to traverse through one of the rings (the other should be marked + // as not traversed, but neither blocked). + bool const can_block + = is_union + || ! (op.visited.finalized() || other_op.visited.finalized()); + + if (! is_self_turn(turn) && can_block) { - // Avoid including untraversed rings which have polygons on - // their left side (union) or not two on their right side (int) - // This can only be done for non-self-turns because of count - // information turn_info_map[ring_id].has_blocked_turn = true; continue; } - if (turn.any_blocked()) + if (is_union && turn.any_blocked()) { turn_info_map[ring_id].has_blocked_turn = true; } @@ -157,6 +161,7 @@ inline void get_ring_turn_info(TurnInfoMap& turn_info_map, Turns const& turns, C // don't block (for union) i/u if there is an self-ii too if (has_blocked || (op.operation == opposite_operation + && can_block && ! turn.has_colocated_both && ! (turn.both(opposite_operation) && is_self_turn(turn)))) diff --git a/test/algorithms/overlay/multi_overlay_cases.hpp b/test/algorithms/overlay/multi_overlay_cases.hpp index d0a53ab9cd..48e0e12b84 100644 --- a/test/algorithms/overlay/multi_overlay_cases.hpp +++ b/test/algorithms/overlay/multi_overlay_cases.hpp @@ -1580,6 +1580,12 @@ static std::string issue_1109[2] = "MULTIPOLYGON(((0 -88,0 -115.40000152587890625,-10 -88,0 -88)))" }; +static std::string issue_1299[2] = +{ + "MULTIPOLYGON(((1.2549999979079400 0.85000000411847698, -1.2550000020920500 0.84999999897038103, -1.2549999999999999 -0.85000000102961903, 1.2549999999999999 -0.84999999999999998)))", + "MULTIPOLYGON(((-0.87500000000000000 -0.84999999999999998, -0.87500000000000000 -0.070000000000000201, -1.2549999999999999 -0.070000000000000201, -1.2549999999999999 -0.84999999999999998)))" +}; + static std::string bug_21155501[2] = { "MULTIPOLYGON(((-8.3935546875 27.449790329784214,4.9658203125 18.729501999072138,11.8212890625 23.563987128451217,9.7119140625 25.48295117535531,9.8876953125 31.728167146023935,8.3056640625 32.99023555965106,8.5693359375 37.16031654673677,-1.8896484375 35.60371874069731,-0.5712890625 32.02670629333614,-8.9208984375 29.458731185355344,-8.3935546875 27.449790329784214)))", diff --git a/test/algorithms/overlay/overlay_cases.hpp b/test/algorithms/overlay/overlay_cases.hpp index 3fcabd6c03..b03b83890b 100644 --- a/test/algorithms/overlay/overlay_cases.hpp +++ b/test/algorithms/overlay/overlay_cases.hpp @@ -1077,6 +1077,12 @@ static std::string issue_876b[2] = "POLYGON((-71.6230763305201634 -132.587678014412745,-106.959839171856814 -102.936613347248112,-40.4477408440520776 -23.6705812141075285,-5.11097800271543878 -53.3216458812721612,-71.6230763305201634 -132.587678014412745))" }; +static std::string issue_893[2] = +{ + "POLYGON((-9133.3885344331684 3976.3162451998137, -6748.2449169873034 -5735.0734557728138, 12359.886942916415 -1042.0645095456412, 5126.3084924076147 2226.9708710750697, -1604.5619839035633 573.85084904357439, -9133.3885344331684 3976.3162451998137))", + "POLYGON((-3228.4265340177531 1307.7159344890201, -4500.2645033380131 1882.4913860267370, -4294.7752070657516 1045.8178117890784, -3228.4265340177531 1307.7159344890201))" +}; + static std::string issue_1076[2] = { "POLYGON((981.792858339935151 98, 927.152135631899114 0, 970 98, 981.792858339935151 98))", diff --git a/test/algorithms/set_operations/difference/difference.cpp b/test/algorithms/set_operations/difference/difference.cpp index b2c2dc6ea7..75c0f917f6 100644 --- a/test/algorithms/set_operations/difference/difference.cpp +++ b/test/algorithms/set_operations/difference/difference.cpp @@ -611,6 +611,14 @@ void test_all() TEST_DIFFERENCE(issue_876a, 1, 4728.89916, 1, 786.29563, 2); TEST_DIFFERENCE(issue_876b, 1, 6114.18234, 1, 4754.29449, count_set(1, 2)); + { + // Results are still invalid + ut_settings settings; + settings.set_test_validity(false); + settings.validity_of_sym = false; + TEST_DIFFERENCE_WITH(issue_893, 1, 97213916.0, 0, 0.0, 1, settings); + } + TEST_DIFFERENCE(issue_1138, 1, 203161.751, 2, 1237551.0171, 1); { diff --git a/test/algorithms/set_operations/difference/difference_multi.cpp b/test/algorithms/set_operations/difference/difference_multi.cpp index 8810f61b4e..07fa3aa4cd 100644 --- a/test/algorithms/set_operations/difference/difference_multi.cpp +++ b/test/algorithms/set_operations/difference/difference_multi.cpp @@ -152,19 +152,7 @@ void test_areal() TEST_DIFFERENCE_WITH(0, 1, ggl_list_20120221_volker, 2, 7962.66, 2, 2775258.93, 4); } - { - // 1: Very small sliver for B (discarded when rescaling) - // 2: sym difference is not considered as valid (without rescaling - // this is a false negative) - // 3: with rescaling A is considered as invalid (robustness problem) - ut_settings settings; - settings.validity_of_sym = true; - settings.validity_false_negative_sym = true; - TEST_DIFFERENCE_WITH(0, 1, bug_21155501, - (count_set(1, 4)), expectation_limits(3.75893, 3.75894), - (count_set(1, 4)), (expectation_limits(1.776357e-15, 7.661281e-15)), - (count_set(2, 5))); - } + TEST_DIFFERENCE(bug_21155501, 1, 3.758937, 1, 1.78e-15, 1); #if defined(BOOST_GEOMETRY_TEST_FAILURES) { @@ -211,6 +199,8 @@ void test_areal() TEST_DIFFERENCE(issue_900, 0, 0.0, 2, 35, 2); + TEST_DIFFERENCE(issue_1299, 1, 3.9706, 0, 0, 1); + // Areas and #clips correspond with POSTGIS (except sym case) test_one("case_101_multi", case_101_multi[0], case_101_multi[1], diff --git a/test/algorithms/set_operations/intersection/intersection.cpp b/test/algorithms/set_operations/intersection/intersection.cpp index fe926a6462..4b9294fd2b 100644 --- a/test/algorithms/set_operations/intersection/intersection.cpp +++ b/test/algorithms/set_operations/intersection/intersection.cpp @@ -301,6 +301,8 @@ void test_areal() TEST_INTERSECTION(issue_861, 1, -1, 1.4715007684573677693e-10); + TEST_INTERSECTION(issue_893, 1, -1, 473001.5082956461); + TEST_INTERSECTION(issue_1226, 1, -1, 0.00036722862); TEST_INTERSECTION(issue_1229, 0, -1, 0); diff --git a/test/algorithms/set_operations/intersection/intersection_multi.cpp b/test/algorithms/set_operations/intersection/intersection_multi.cpp index 37ef6c4a20..7ba5aa4ca8 100644 --- a/test/algorithms/set_operations/intersection/intersection_multi.cpp +++ b/test/algorithms/set_operations/intersection/intersection_multi.cpp @@ -362,6 +362,8 @@ void test_areal() TEST_INTERSECTION(issue_888_34, 7, -1, 0.0256838); TEST_INTERSECTION(issue_888_37, 13, -1, 0.0567043); + TEST_INTERSECTION(issue_1299, 1, -1, 0.2964); + TEST_INTERSECTION(mysql_23023665_7, 2, 11, 9.80505786783); TEST_INTERSECTION(mysql_23023665_12, 2, 0, 11.812440191387557); TEST_INTERSECTION(mysql_regression_1_65_2017_08_31, 2, -1, 29.9022122); diff --git a/test/algorithms/set_operations/union/union.cpp b/test/algorithms/set_operations/union/union.cpp index 0473c5cfa2..5f25906f1b 100644 --- a/test/algorithms/set_operations/union/union.cpp +++ b/test/algorithms/set_operations/union/union.cpp @@ -442,6 +442,9 @@ void test_areal() TEST_UNION(issue_838, 1, 0, -1, expectation_limits(1.3333, 1.33785)); TEST_UNION_REV(issue_838, 1, 0, -1, expectation_limits(1.3333, 1.33785)); + TEST_UNION(issue_893, 1, 0, -1, 97686917.29298662); + TEST_UNION_REV(issue_893, 1, 0, -1, 97686917.29298662); + TEST_UNION(issue_1076, 1, 0, -1, 1225.0); TEST_UNION_REV(issue_1076, 1, 0, -1, 1225.0); diff --git a/test/algorithms/set_operations/union/union_multi.cpp b/test/algorithms/set_operations/union/union_multi.cpp index d325a87401..c8827fe07c 100644 --- a/test/algorithms/set_operations/union/union_multi.cpp +++ b/test/algorithms/set_operations/union/union_multi.cpp @@ -437,6 +437,8 @@ void test_areal() TEST_UNION(issue_1109, 2, 0, -1, 3946.5); + TEST_UNION(issue_1299, 1, 0, -1, 4.267); + // One or two polygons, the ideal case is 1 TEST_UNION(mail_2019_01_21_johan, count_set(1, 2), 0, -1, 0.00058896);