From adc9044da2169c0c6b21035725c4e612aae85a21 Mon Sep 17 00:00:00 2001 From: Barend Gehrels Date: Tue, 30 Jul 2024 19:30:16 +0200 Subject: [PATCH 1/2] test: add test cases for issues fixed in next commit --- test/algorithms/buffer/buffer_polygon.cpp | 15 +++++++++++++++ test/algorithms/overlay/overlay_cases.hpp | 12 ++++++++++++ .../set_operations/difference/difference.cpp | 9 +++++++++ .../set_operations/intersection/intersection.cpp | 3 +++ test/algorithms/set_operations/union/union.cpp | 6 ++++++ 5 files changed, 45 insertions(+) diff --git a/test/algorithms/buffer/buffer_polygon.cpp b/test/algorithms/buffer/buffer_polygon.cpp index 25660be91a..6d2d6e09e3 100644 --- a/test/algorithms/buffer/buffer_polygon.cpp +++ b/test/algorithms/buffer/buffer_polygon.cpp @@ -157,6 +157,15 @@ static std::string const issue_1019 static std::string const issue_1262 = "POLYGON((-2.447356204196278639528828 57.21240623671037894837355,34.00960378453005006349485 54.01542955431686721112783,-0.000789642333984375 18.712947845458984375,-41.480987548828125 60.193248748779296875,-3.12519073486328125 57.271846771240234375,-2.447356204196278639528828 57.21240623671037894837355),(-36.24821876005196230607908 57.78889760314127244100746,-0.000785932392148191993896944 21.54137477179954629491476,30.75139677038663066355184 52.2934724874262641947098,-36.24821876005196230607908 57.78889760314127244100746))"; +static std::string const issue_1294_original + = "POLYGON((730.35 750,730.35 740,726.02 740,726.02 735,730.35 735,730.35 0,0 0,0 750,730.35 750))"; +// With 600 subtracted (with 700, the problem does not reproduce) +static std::string const issue_1294 + = "POLYGON((130.35 150,130.35 140,126.02 140,126.02 135,130.35 135,130.35 0,0 0,0 150,130.35 150))"; +// The dent is 10 lower and then it does not reproduce +static std::string const issue_1294_shifted + = "POLYGON((130.35 150,130.35 130,126.02 130,126.02 125,130.35 125,130.35 0,0 0,0 150,130.35 150))"; + // CCW Polygons not working in 1.56 static std::string const mysql_report_2014_10_24 = "POLYGON((0 0, 0 8, 8 8, 8 10, -10 10, -10 0, 0 0))"; @@ -620,6 +629,12 @@ void test_all() test_one("issue_1262_3", issue_1262, join_round4, end_round4, 193.47288, -0.4); } + { + test_one("issue_1294", issue_1294, join_miter, end_flat, 22456.0, 5.0); + test_one("issue_1294_shifted", issue_1294_shifted, join_miter, end_flat, 22456.0, 5.0); + test_one("issue_1294_original", issue_1294_original, join_miter, end_flat, 562666.0, 5.0); + } + { bg::strategy::buffer::join_round join_round32(32); bg::strategy::buffer::end_round end_round32(32); diff --git a/test/algorithms/overlay/overlay_cases.hpp b/test/algorithms/overlay/overlay_cases.hpp index ef477631e2..3ded5aa154 100644 --- a/test/algorithms/overlay/overlay_cases.hpp +++ b/test/algorithms/overlay/overlay_cases.hpp @@ -1195,6 +1195,18 @@ static std::string issue_1244[2] = "POLYGON((2 -1,5 2,2 5,2 -1))" }; +static std::string issue_1293[2] = +{ + "POLYGON((13.90250403054817 -108.7493905903629, 18.1717071533203 -109.6161492466927, 20.38385009765624 -109.6161492466927, 13.74227594050434 -109.9162293522026, 13.80791011645497 -109.3242516085326, 13.90250403054817 -108.7493905903629))", + "POLYGON((18.17170715332031 -109.6161492466927, 19.60749526774088 -109.5220427274138, 11.19696044921875 -108.1424287761621, 12.48744269320276 -108.7788241542253, 13.84995095309101 -109.2413333582132, 15.26117233479817 -109.5220427274138, 16.69696044921874 -109.6161492466927, 18.17170715332031 -109.6161492466927))" +}; + +static std::string issue_1295[2] = +{ + "POLYGON((6.006524919981425 -1.8204168107748433, -1.7115898840974395 -1.8204168107748442, -2.1474675179419931 1.8204168107748429, -0.21793881692227707 1.8204168107748435, 0.0 0.0, 5.7885861030591492 4.6030847803960675e-16, 6.006524919981425 -1.8204168107748433))", + "POLYGON((5.7885861030591501 1.2258588412299519e-16, 4.9808473496052885 -0.91020845951432006, 3.0646473238074286e-16 -0.91020843804041685, 0.0 0.0, 5.7885861030591501 1.2258588412299519e-16))" +}; + static std::string ggl_list_20120229_volker[3] = { "POLYGON((1716 1554,2076 2250,2436 2352,2796 1248,3156 2484,3516 2688,3516 2688,3156 2484,2796 1248,2436 2352,2076 2250, 1716 1554))", diff --git a/test/algorithms/set_operations/difference/difference.cpp b/test/algorithms/set_operations/difference/difference.cpp index 6867b6c78d..819c8c8523 100644 --- a/test/algorithms/set_operations/difference/difference.cpp +++ b/test/algorithms/set_operations/difference/difference.cpp @@ -617,6 +617,15 @@ void test_all() TEST_DIFFERENCE(issue_1244, 3, 8, 3, 2, 6); + TEST_DIFFERENCE(issue_1293, 1, 1.40999, 1, 2.318951, 2); + +#if defined(BOOST_GEOMETRY_TEST_FAILURES) + // Difference fails for this case. This was not reported for this case. + // Reported as a failing intersection, which is fixed. + // The failing difference should be investigated more thoroughly. + TEST_DIFFERENCE(issue_1295, 1, 9.999, 1, 9.999, 1); +#endif + TEST_DIFFERENCE(mysql_21977775, 2, 160.856568913, 2, 92.3565689126, 4); TEST_DIFFERENCE(mysql_21965285, 1, 92.0, 1, 14.0, 1); TEST_DIFFERENCE(mysql_23023665_1, 1, 92.0, 1, 142.5, 2); diff --git a/test/algorithms/set_operations/intersection/intersection.cpp b/test/algorithms/set_operations/intersection/intersection.cpp index 585c35dd17..29e4da6144 100644 --- a/test/algorithms/set_operations/intersection/intersection.cpp +++ b/test/algorithms/set_operations/intersection/intersection.cpp @@ -307,6 +307,9 @@ void test_areal() TEST_INTERSECTION(issue_1244, 1, -1, 7); + TEST_INTERSECTION(issue_1293, 1, -1, 1.49123); + TEST_INTERSECTION(issue_1295, 1, -1, 4.90121); + test_one("buffer_mp1", buffer_mp1[0], buffer_mp1[1], 1, 31, 2.271707796); test_one("buffer_mp2", buffer_mp2[0], buffer_mp2[1], diff --git a/test/algorithms/set_operations/union/union.cpp b/test/algorithms/set_operations/union/union.cpp index 9aacb0c320..e4ea757b82 100644 --- a/test/algorithms/set_operations/union/union.cpp +++ b/test/algorithms/set_operations/union/union.cpp @@ -475,6 +475,12 @@ void test_areal() TEST_UNION(issue_1244, 1, 1, -1, 17); TEST_UNION_REV(issue_1244, 1, 1, -1, 17); + TEST_UNION(issue_1293, 1, 0, -1, 5.22017); + TEST_UNION_REV(issue_1293, 1, 0, -1, 5.22017); + + TEST_UNION(issue_1295, 1, 0, -1, 17.56273); + TEST_UNION_REV(issue_1295, 1, 0, -1, 17.56273); + TEST_UNION(geos_1, 1, 0, -1, expectation_limits(3458.0, 3461.3203125)); TEST_UNION(geos_2, 1, 0, -1, expectation_limits(349.0625, 350.55102539)); TEST_UNION(geos_3, 1, 0, -1, 29391548.4998779); From cace2c73fba68d32ffd6e236731853694943daa4 Mon Sep 17 00:00:00 2001 From: Barend Gehrels Date: Tue, 30 Jul 2024 19:55:46 +0200 Subject: [PATCH 2/2] fix: traverse first through non clustered turns, remove cluster exits, make priority consistent fixes #1293 #1294 and #1295 --- .../detail/overlay/cluster_exits.hpp | 233 ------------------ .../algorithms/detail/overlay/traversal.hpp | 78 ++---- .../detail/overlay/traversal_ring_creator.hpp | 45 ++-- 3 files changed, 47 insertions(+), 309 deletions(-) delete mode 100644 include/boost/geometry/algorithms/detail/overlay/cluster_exits.hpp diff --git a/include/boost/geometry/algorithms/detail/overlay/cluster_exits.hpp b/include/boost/geometry/algorithms/detail/overlay/cluster_exits.hpp deleted file mode 100644 index 94fe235d4e..0000000000 --- a/include/boost/geometry/algorithms/detail/overlay/cluster_exits.hpp +++ /dev/null @@ -1,233 +0,0 @@ -// Boost.Geometry (aka GGL, Generic Geometry Library) - -// Copyright (c) 2020 Barend Gehrels, Amsterdam, the Netherlands. -// Copyright (c) 2023 Adam Wulkiewicz, Lodz, Poland. - -// This file was modified by Oracle on 2020-2023. -// Modifications copyright (c) 2020-2023 Oracle and/or its affiliates. -// Contributed and/or modified by Vissarion Fysikopoulos, on behalf of Oracle -// Contributed and/or modified by Adam Wulkiewicz, on behalf of Oracle - -// Use, modification and distribution is subject to the Boost Software License, -// Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at -// http://www.boost.org/LICENSE_1_0.txt) - -#ifndef BOOST_GEOMETRY_ALGORITHMS_DETAIL_OVERLAY_CLUSTER_EXITS_HPP -#define BOOST_GEOMETRY_ALGORITHMS_DETAIL_OVERLAY_CLUSTER_EXITS_HPP - -#include -#include -#include - -#include - -#include -#include -#include -#include -#include - -#if defined(BOOST_GEOMETRY_DEBUG_INTERSECTION) \ - || defined(BOOST_GEOMETRY_OVERLAY_REPORT_WKT) \ - || defined(BOOST_GEOMETRY_DEBUG_TRAVERSE) -# include -# include -# include -#endif - -namespace boost { namespace geometry -{ - -#ifndef DOXYGEN_NO_DETAIL -namespace detail { namespace overlay -{ - -// Structure to check relatively simple cluster cases -template -struct cluster_exits -{ -private : - static const operation_type target_operation = operation_from_overlay::value; - typedef typename boost::range_value::type turn_type; - typedef typename turn_type::turn_operation_type turn_operation_type; - - struct linked_turn_op_info - { - explicit linked_turn_op_info(signed_size_type ti = -1, int oi = -1, - signed_size_type nti = -1) - : turn_index(ti) - , op_index(oi) - , next_turn_index(nti) - , rank_index(-1) - {} - - signed_size_type turn_index; - int op_index; - signed_size_type next_turn_index; - signed_size_type rank_index; - }; - - inline signed_size_type get_rank(Sbs const& sbs, - linked_turn_op_info const& info) const - { - for (auto const& rp : sbs.m_ranked_points) - { - if (rp.turn_index == info.turn_index - && rp.operation_index == info.op_index - && rp.direction == sort_by_side::dir_to) - { - return rp.rank; - } - } - return -1; - } - - std::set const& m_ids; - std::vector possibilities; - std::vector blocked; - - bool m_valid; - - bool collect(Turns const& turns) - { - for (auto cluster_turn_index : m_ids) - { - turn_type const& cluster_turn = turns[cluster_turn_index]; - if (cluster_turn.discarded) - { - continue; - } - if (cluster_turn.both(target_operation)) - { - // Not (yet) supported, can be cluster of u/u turns - return false; - } - for (int i = 0; i < 2; i++) - { - turn_operation_type const& op = cluster_turn.operations[i]; - turn_operation_type const& other_op = cluster_turn.operations[1 - i]; - signed_size_type const ni = op.enriched.get_next_turn_index(); - - if (op.operation == target_operation - || op.operation == operation_continue) - { - if (ni == cluster_turn_index) - { - // Not (yet) supported, traveling to itself, can be - // hole - return false; - } - possibilities.push_back( - linked_turn_op_info(cluster_turn_index, i, ni)); - } - else if (op.operation == operation_blocked - && ! (ni == other_op.enriched.get_next_turn_index()) - && m_ids.count(ni) == 0) - { - // Points to turn, not part of this cluster, - // and that way is blocked. But if the other operation - // points at the same turn, it is still fine. - blocked.push_back( - linked_turn_op_info(cluster_turn_index, i, ni)); - } - } - } - return true; - } - - bool check_blocked(Sbs const& sbs) - { - if (blocked.empty()) - { - return true; - } - - for (auto& info : possibilities) - { - info.rank_index = get_rank(sbs, info); - } - for (auto& info : blocked) - { - info.rank_index = get_rank(sbs, info); - } - - for (auto const& lti : possibilities) - { - for (auto const& blti : blocked) - { - if (blti.next_turn_index == lti.next_turn_index - && blti.rank_index == lti.rank_index) - { - return false; - } - } - } - return true; - } - -public : - cluster_exits(Turns const& turns, - std::set const& ids, - Sbs const& sbs) - : m_ids(ids) - , m_valid(collect(turns) && check_blocked(sbs)) - { - } - - inline bool apply(signed_size_type& turn_index, - int& op_index, - bool first_run = true) const - { - if (! m_valid) - { - return false; - } - - // Traversal can either enter the cluster in the first turn, - // or it can start halfway. - // If there is one (and only one) possibility pointing outside - // the cluster, take that one. - linked_turn_op_info target; - for (auto const& lti : possibilities) - { - if (m_ids.count(lti.next_turn_index) == 0) - { - if (target.turn_index >= 0 - && target.next_turn_index != lti.next_turn_index) - { - // Points to different target - return false; - } - if BOOST_GEOMETRY_CONSTEXPR (OverlayType == overlay_buffer) - { - if (first_run && target.turn_index >= 0) - { - // Target already assigned, so there are more targets - // or more ways to the same target - return false; - } - } - - target = lti; - } - } - if (target.turn_index < 0) - { - return false; - } - - turn_index = target.turn_index; - op_index = target.op_index; - - return true; - } -}; - -}} // namespace detail::overlay -#endif // DOXYGEN_NO_DETAIL - -}} // namespace boost::geometry - -#endif // BOOST_GEOMETRY_ALGORITHMS_DETAIL_OVERLAY_CLUSTER_EXITS_HPP diff --git a/include/boost/geometry/algorithms/detail/overlay/traversal.hpp b/include/boost/geometry/algorithms/detail/overlay/traversal.hpp index 50af9aff79..e306688827 100644 --- a/include/boost/geometry/algorithms/detail/overlay/traversal.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/traversal.hpp @@ -1,6 +1,6 @@ // Boost.Geometry (aka GGL, Generic Geometry Library) -// Copyright (c) 2007-2012 Barend Gehrels, Amsterdam, the Netherlands. +// Copyright (c) 2007-2024 Barend Gehrels, Amsterdam, the Netherlands. // Copyright (c) 2023-2024 Adam Wulkiewicz, Lodz, Poland. // This file was modified by Oracle on 2017-2024. @@ -23,7 +23,6 @@ #include #include -#include #include #include #include @@ -231,26 +230,12 @@ public : segment_identifier const& previous_seg_id) const { // For uu/ii, only switch sources if indicated - - if BOOST_GEOMETRY_CONSTEXPR (OverlayType == overlay_buffer) - { - // Buffer does not use source_index (always 0). - return select_source_generic<&segment_identifier::multi_index>( + // Buffer and self-turns do not use source_index (always 0). + return OverlayType == overlay_buffer || is_self_turn(turn) + ? select_source_generic<&segment_identifier::multi_index>( + turn, candidate_seg_id, previous_seg_id) + : select_source_generic<&segment_identifier::source_index>( turn, candidate_seg_id, previous_seg_id); - } - else // else prevents unreachable code warning - { - if (is_self_turn(turn)) - { - // Also, if it is a self-turn, stay on same ring (multi/ring) - return select_source_generic<&segment_identifier::multi_index>( - turn, candidate_seg_id, previous_seg_id); - } - - // Use source_index - return select_source_generic<&segment_identifier::source_index>( - turn, candidate_seg_id, previous_seg_id); - } } inline bool traverse_possible(signed_size_type turn_index) const @@ -569,7 +554,7 @@ public : // 3: OK // 4: OK and start turn matches // 5: OK and start turn and start operation both match, this is the best - inline int priority_of_turn_in_cluster_union(sort_by_side::rank_type selected_rank, + inline int priority_of_turn_in_cluster(sort_by_side::rank_type selected_rank, typename sbs_type::rp const& ranked_point, cluster_info const& cinfo, signed_size_type start_turn_index, int start_op_index) const @@ -591,14 +576,14 @@ public : if BOOST_GEOMETRY_CONSTEXPR (OverlayType != overlay_dissolve) { - if (op.enriched.count_left != 0 || op.enriched.count_right == 0) + if ((op.enriched.count_left != 0 || op.enriched.count_right == 0) && cinfo.spike_count > 0) { // Check counts: in some cases interior rings might be generated with // polygons on both sides. For dissolve it can be anything. // If this forms a spike, going to/from the cluster point in the same // (opposite) direction, it can still be used. - return cinfo.spike_count > 0 ? 1 : 0; + return 1; } } @@ -657,9 +642,8 @@ public : return -1; } - inline bool select_from_cluster_union(signed_size_type& turn_index, - cluster_info const& cinfo, - int& op_index, sbs_type const& sbs, + inline bool select_from_cluster(signed_size_type& turn_index, int& op_index, + cluster_info const& cinfo, sbs_type const& sbs, signed_size_type start_turn_index, int start_op_index) const { sort_by_side::rank_type const selected_rank = select_rank(sbs); @@ -674,7 +658,7 @@ public : break; } - int const priority = priority_of_turn_in_cluster_union(selected_rank, + int const priority = priority_of_turn_in_cluster(selected_rank, ranked_point, cinfo, start_turn_index, start_op_index); if (priority > current_priority) @@ -687,7 +671,9 @@ public : return current_priority > 0; } - inline bool analyze_cluster_intersection(signed_size_type& turn_index, + // Analyzes a clustered intersection, as if it is clustered. + // This is used for II intersections + inline bool analyze_ii_cluster(signed_size_type& turn_index, int& op_index, sbs_type const& sbs) const { // Select the rank based on regions and isolation @@ -823,34 +809,13 @@ public : } } - cluster_exits exits(m_turns, cinfo.turn_indices, sbs); - - if (exits.apply(turn_index, op_index)) - { - return true; - } - - bool result = false; - - if BOOST_GEOMETRY_CONSTEXPR (is_union) - { - result = select_from_cluster_union(turn_index, cinfo, - op_index, sbs, - start_turn_index, start_op_index); - if (! result) - { - // There no way out found, try second pass in collected cluster exits - result = exits.apply(turn_index, op_index, false); - } - } - else - { - result = analyze_cluster_intersection(turn_index, op_index, sbs); - } - return result; + return select_from_cluster(turn_index, op_index, cinfo, sbs, start_turn_index, start_op_index); } // Analyzes a non-clustered "ii" intersection, as if it is clustered. + // TODO: it, since select_from_cluster is generalized (July 2024), + // uses a specific function used only for "ii" intersections. + // Therefore the sort-by-side solution should not be necessary and can be refactored. inline bool analyze_ii_intersection(signed_size_type& turn_index, int& op_index, turn_type const& current_turn, segment_identifier const& previous_seg_id) @@ -874,9 +839,7 @@ public : sbs.apply(current_turn.point); - bool result = analyze_cluster_intersection(turn_index, op_index, sbs); - - return result; + return analyze_ii_cluster(turn_index, op_index, sbs); } inline void change_index_for_self_turn(signed_size_type& to_vertex_index, @@ -1075,7 +1038,6 @@ private : }; - }} // namespace detail::overlay #endif // DOXYGEN_NO_DETAIL diff --git a/include/boost/geometry/algorithms/detail/overlay/traversal_ring_creator.hpp b/include/boost/geometry/algorithms/detail/overlay/traversal_ring_creator.hpp index 71c44d14a8..89d44b5b29 100644 --- a/include/boost/geometry/algorithms/detail/overlay/traversal_ring_creator.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/traversal_ring_creator.hpp @@ -322,31 +322,40 @@ struct traversal_ring_creator void iterate(Rings& rings, std::size_t& finalized_ring_size, typename Backtrack::state_type& state) { - for (std::size_t turn_index = 0; turn_index < m_turns.size(); ++turn_index) + auto do_iterate = [&](int phase) { - turn_type const& turn = m_turns[turn_index]; - - if (turn.discarded || turn.blocked()) + for (std::size_t turn_index = 0; turn_index < m_turns.size(); ++turn_index) { - // Skip discarded and blocked turns - continue; - } + turn_type const& turn = m_turns[turn_index]; - if (turn.both(operation_continue)) - { - traverse_with_operation(turn, turn_index, - get_operation_index(turn), - rings, finalized_ring_size, state); - } - else - { - for (int op_index = 0; op_index < 2; op_index++) + if (turn.discarded || turn.blocked() || (phase == 0 && turn.is_clustered())) { - traverse_with_operation(turn, turn_index, op_index, + // Skip discarded and blocked turns + continue; + } + + if (turn.both(operation_continue)) + { + traverse_with_operation(turn, turn_index, + get_operation_index(turn), rings, finalized_ring_size, state); } + else + { + for (int op_index = 0; op_index < 2; op_index++) + { + traverse_with_operation(turn, turn_index, op_index, + rings, finalized_ring_size, state); + } + } } - } + }; + + // Traverse all turns, first starting with the non-clustered ones. + do_iterate(0); + + // Traverse remaining clustered turns, if any. + do_iterate(1); } template