[traverse] reverse meaning of isolation in difference

Author: barendgehrels

include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp

@@ -26,8 +26,7 @@ struct is_self_turn_check
     template <typename Turn>
     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();
     }
 };
 

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)

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

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

test/algorithms/set_operations/difference/difference_multi.cpp

@@ -209,10 +209,8 @@ 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);
 
     TEST_DIFFERENCE(issue_888_34, 22, 0.2506824, 6, 0.0253798, 28);
     TEST_DIFFERENCE(issue_888_37, 15, 0.0451408, 65, 0.3014843, 80);

test/algorithms/set_operations/difference/test_difference.hpp

@@ -322,18 +322,25 @@ std::string test_one(std::string const& caseid,
     bg::correct(g1);
     bg::correct(g2);
 
-    std::string result = test_difference<OutputType>(caseid + "_a", g1, g2,
+    std::string result;
+
+#if ! defined(BOOST_GEOMETRY_TEST_DIFFERENCE_ONLY_B)
+    result = test_difference<OutputType>(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
 
     test_difference<OutputType>(caseid + "_b", g2, g1,
         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