From a7c26dc47c563ff45cec4bd50ae1753f609cba88 Mon Sep 17 00:00:00 2001 From: Barend Gehrels Date: Thu, 8 Jul 2021 12:51:01 +0200 Subject: [PATCH] [traverse] reverse meaning of isolation in difference --- .../detail/overlay/is_self_turn.hpp | 3 +- .../algorithms/detail/overlay/traversal.hpp | 98 +++++++++++-------- .../overlay/traversal_switch_detector.hpp | 49 ++++++++-- .../algorithms/detail/overlay/turn_info.hpp | 5 + .../difference/difference_multi.cpp | 7 +- .../difference/test_difference.hpp | 13 ++- 6 files changed, 113 insertions(+), 62 deletions(-) diff --git a/include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp b/include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp index 448c04404f4..1606c31e903 100644 --- a/include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp @@ -26,8 +26,7 @@ struct is_self_turn_check template static inline bool apply(Turn const& turn) { - return turn.operations[0].seg_id.source_index - == turn.operations[1].seg_id.source_index; + return turn.is_self(); } }; diff --git a/include/boost/geometry/algorithms/detail/overlay/traversal.hpp b/include/boost/geometry/algorithms/detail/overlay/traversal.hpp index a0149789c66..b65f00424f9 100644 --- a/include/boost/geometry/algorithms/detail/overlay/traversal.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/traversal.hpp @@ -621,31 +621,32 @@ public : return m_turns[rp.turn_index].operations[rp.operation_index]; } - inline sort_by_side::rank_type select_rank(sbs_type const& sbs, - bool skip_isolated) const + inline sort_by_side::rank_type select_rank(sbs_type const& sbs) const { + static bool const is_intersection + = target_operation == operation_intersection; + // Take the first outgoing rank corresponding to incoming region, // or take another region if it is not isolated - turn_operation_type const& incoming_op - = operation_from_rank(sbs.m_ranked_points.front()); + auto const& in_op = operation_from_rank(sbs.m_ranked_points.front()); for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++) { - typename sbs_type::rp const& rp = sbs.m_ranked_points[i]; + auto const& rp = sbs.m_ranked_points[i]; if (rp.rank == 0 || rp.direction == sort_by_side::dir_from) { continue; } - turn_operation_type const& op = operation_from_rank(rp); + auto const& out_op = operation_from_rank(rp); - if (op.operation != target_operation - && op.operation != operation_continue) + if (out_op.operation != target_operation + && out_op.operation != operation_continue) { continue; } - if (op.enriched.region_id == incoming_op.enriched.region_id - || (skip_isolated && ! op.enriched.isolated)) + if (in_op.enriched.region_id == out_op.enriched.region_id + || (is_intersection && ! out_op.enriched.isolated)) { // Region corresponds to incoming region, or (for intersection) // there is a non-isolated other region which should be taken @@ -660,7 +661,7 @@ public : int& op_index, 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, false); + sort_by_side::rank_type const selected_rank = select_rank(sbs); int current_priority = 0; for (std::size_t i = 1; i < sbs.m_ranked_points.size(); i++) @@ -688,49 +689,59 @@ public : inline bool analyze_cluster_intersection(signed_size_type& turn_index, int& op_index, sbs_type const& sbs) const { - sort_by_side::rank_type const selected_rank = select_rank(sbs, true); + // Select the rank based on regions and isolation + sort_by_side::rank_type const selected_rank = select_rank(sbs); - if (selected_rank > 0) + if (selected_rank <= 0) { - typename turn_operation_type::comparable_distance_type - min_remaining_distance = 0; + return false; + } - std::size_t selected_index = sbs.m_ranked_points.size(); - for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++) + // From these ranks, select the index: the first, or the one with + // the smallest remaining distance + typename turn_operation_type::comparable_distance_type + min_remaining_distance = 0; + + std::size_t selected_index = sbs.m_ranked_points.size(); + for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++) + { + auto const& ranked_point = sbs.m_ranked_points[i]; + + if (ranked_point.rank > selected_rank) + { + break; + } + else if (ranked_point.rank == selected_rank) { - typename sbs_type::rp const& ranked_point = sbs.m_ranked_points[i]; + auto const& op = operation_from_rank(ranked_point); - if (ranked_point.rank == selected_rank) + if (op.visited.finalized()) { - turn_operation_type const& op = operation_from_rank(ranked_point); - - if (op.visited.finalized()) - { - // This direction is already traveled before, the same - // cannot be traveled again - continue; - } - - // Take turn with the smallest remaining distance - if (selected_index == sbs.m_ranked_points.size() - || op.remaining_distance < min_remaining_distance) - { - selected_index = i; - min_remaining_distance = op.remaining_distance; - } + // This direction is already traveled, + // it cannot be traveled again + continue; } - } - if (selected_index < sbs.m_ranked_points.size()) - { - typename sbs_type::rp const& ranked_point = sbs.m_ranked_points[selected_index]; - turn_index = ranked_point.turn_index; - op_index = ranked_point.operation_index; - return true; + if (selected_index == sbs.m_ranked_points.size() + || op.remaining_distance < min_remaining_distance) + { + // It was unassigned or it is better + selected_index = i; + min_remaining_distance = op.remaining_distance; + } } } - return false; + if (selected_index == sbs.m_ranked_points.size()) + { + // Should not happen, there must be points with the selected rank + return false; + } + + auto const& ranked_point = sbs.m_ranked_points[selected_index]; + turn_index = ranked_point.turn_index; + op_index = ranked_point.operation_index; + return true; } inline bool fill_sbs(sbs_type& sbs, @@ -819,6 +830,7 @@ public : return result; } + // Analyzes a non-clustered "ii" intersection, as if it is clustered. inline bool analyze_ii_intersection(signed_size_type& turn_index, int& op_index, turn_type const& current_turn, segment_identifier const& previous_seg_id) diff --git a/include/boost/geometry/algorithms/detail/overlay/traversal_switch_detector.hpp b/include/boost/geometry/algorithms/detail/overlay/traversal_switch_detector.hpp index ad248826dd3..ae2e7b5c818 100644 --- a/include/boost/geometry/algorithms/detail/overlay/traversal_switch_detector.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/traversal_switch_detector.hpp @@ -89,7 +89,6 @@ struct traversal_switch_detector enum isolation_type { - isolation_unknown = -1, isolation_no = 0, isolation_yes = 1, isolation_multiple = 2 @@ -121,7 +120,7 @@ struct traversal_switch_detector struct region_properties { signed_size_type region_id = -1; - isolation_type isolated = isolation_unknown; + isolation_type isolated = isolation_no; set_type unique_turn_ids; connection_map connected_region_counts; }; @@ -374,7 +373,7 @@ struct traversal_switch_detector { region_properties& properties = key_val.second; - if (properties.isolated == isolation_unknown + if (properties.isolated == isolation_no && has_only_isolated_children(properties)) { properties.isolated = isolation_yes; @@ -388,13 +387,36 @@ struct traversal_switch_detector { for (turn_type& turn : m_turns) { + // For difference, for the input walked through in reverse, + // the meaning is reversed: what is isolated is actually not, + // and vice versa. + bool const reverseMeaningInTurn + = (Reverse1 || Reverse2) + && ! turn.is_self() + && ! turn.is_clustered() + && uu_or_ii(turn) + && turn.operations[0].enriched.region_id + != turn.operations[1].enriched.region_id; + for (auto& op : turn.operations) { auto mit = m_connected_regions.find(op.enriched.region_id); if (mit != m_connected_regions.end()) { + bool const reverseMeaningInOp + = reverseMeaningInTurn + && ((op.seg_id.source_index == 0 && Reverse1) + || (op.seg_id.source_index == 1 && Reverse2)); + + // It is assigned to isolated if it's property is "Yes", + // (one connected interior, or chained). + // "Multiple" doesn't count for isolation, + // neither for intersection, neither for difference. region_properties const& prop = mit->second; - op.enriched.isolated = prop.isolated == isolation_yes; + op.enriched.isolated + = reverseMeaningInOp + ? prop.isolated == isolation_no + : prop.isolated == isolation_yes; } } } @@ -478,8 +500,12 @@ struct traversal_switch_detector // Discarded turns don't connect rings to the same region // Also xx are not relevant // (otherwise discarded colocated uu turn could make a connection) - return ! turn.discarded - && ! turn.both(operation_blocked); + return ! turn.discarded && ! turn.both(operation_blocked); + } + + inline bool uu_or_ii(turn_type const& turn) const + { + return turn.both(operation_union) || turn.both(operation_intersection); } inline bool connects_same_region(turn_type const& turn) const @@ -492,7 +518,7 @@ struct traversal_switch_detector if (! turn.is_clustered()) { // If it is a uu/ii-turn (non clustered), it is never same region - return ! (turn.both(operation_union) || turn.both(operation_intersection)); + return ! uu_or_ii(turn); } if (BOOST_GEOMETRY_CONDITION(target_operation == operation_union)) @@ -568,7 +594,10 @@ struct traversal_switch_detector void iterate() { #if defined(BOOST_GEOMETRY_DEBUG_TRAVERSAL_SWITCH_DETECTOR) - std::cout << "BEGIN SWITCH DETECTOR (region_ids and isolation)" << std::endl; + std::cout << "BEGIN SWITCH DETECTOR (region_ids and isolation)" + << (Reverse1 ? " REVERSE_1" : "") + << (Reverse2 ? " REVERSE_2" : "") + << std::endl; #endif // Collect turns per ring @@ -613,14 +642,14 @@ struct traversal_switch_detector { turn_type const& turn = m_turns[turn_index]; - if ((turn.both(operation_union) || turn.both(operation_intersection)) - && ! turn.is_clustered()) + if (uu_or_ii(turn) && ! turn.is_clustered()) { std::cout << (turn.both(operation_union) ? "UU" : "II") << " " << turn_index << " (" << geometry::get<0>(turn.point) << ", " << geometry::get<1>(turn.point) << ")" << " -> " << std::boolalpha + << " [" << turn.operations[0].seg_id.source_index << "/" << turn.operations[1].seg_id.source_index << "] " << "(" << turn.operations[0].enriched.region_id << " " << turn.operations[0].enriched.isolated << ") / (" << turn.operations[1].enriched.region_id diff --git a/include/boost/geometry/algorithms/detail/overlay/turn_info.hpp b/include/boost/geometry/algorithms/detail/overlay/turn_info.hpp index 7dcbf4c8eff..08cb516cf67 100644 --- a/include/boost/geometry/algorithms/detail/overlay/turn_info.hpp +++ b/include/boost/geometry/algorithms/detail/overlay/turn_info.hpp @@ -138,6 +138,11 @@ struct turn_info { return cluster_id > 0; } + inline bool is_self() const + { + return operations[0].seg_id.source_index + == operations[1].seg_id.source_index; + } private : inline bool has12(operation_type type1, operation_type type2) const diff --git a/test/algorithms/set_operations/difference/difference_multi.cpp b/test/algorithms/set_operations/difference/difference_multi.cpp index bd445437241..cd357501a50 100644 --- a/test/algorithms/set_operations/difference/difference_multi.cpp +++ b/test/algorithms/set_operations/difference/difference_multi.cpp @@ -209,10 +209,9 @@ void test_areal() #endif } -#if defined(BOOST_GEOMETRY_TEST_FAILURES) - // Generates a polygon with two interiors, i/o a multipoly with 3 rings - TEST_DIFFERENCE(issue_869_a, 3, 3600, 0, 0, 1); -#endif + // Requires reveral of isolation in ii turns. There should be 3 rings. + TEST_DIFFERENCE(issue_869_a, 3, 3600, 0, 0, 3); + // 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/difference/test_difference.hpp b/test/algorithms/set_operations/difference/test_difference.hpp index 3c9ce08a546..b36c9fbeed6 100644 --- a/test/algorithms/set_operations/difference/test_difference.hpp +++ b/test/algorithms/set_operations/difference/test_difference.hpp @@ -322,11 +322,14 @@ std::string test_one(std::string const& caseid, bg::correct(g1); bg::correct(g2); - std::string result = test_difference(caseid + "_a", g1, g2, + std::string result; + +#if ! defined(BOOST_GEOMETRY_TEST_DIFFERENCE_ONLY_B) + result = test_difference(caseid + "_a", g1, g2, expected_count1, expected_rings_count1, expected_point_count1, expected_area1, false, settings); - -#ifdef BOOST_GEOMETRY_DEBUG_ASSEMBLE +#endif +#if defined(BOOST_GEOMETRY_TEST_DIFFERENCE_ONLY_A) return result; #endif @@ -334,6 +337,10 @@ std::string test_one(std::string const& caseid, expected_count2, expected_rings_count2, expected_point_count2, expected_area2, false, settings); +#if defined(BOOST_GEOMETRY_TEST_DIFFERENCE_ONLY_B) + return result; +#endif + #if ! defined(BOOST_GEOMETRY_TEST_ALWAYS_CHECK_SYMDIFFERENCE) if (settings.sym_difference) #endif