fix: extra round over ring in #1226 and #1326

This can occur in a sequence of touch and then touch_interior

doc/release_notes.qbk

@@ -19,6 +19,20 @@
 
 [section:release_notes Release Notes]
 
+[/=================]
+[heading Boost 1.87]
+[/=================]
+
+[*Improvements]
+
+* Use using instead of typedef in part of the code
+* Modernize meta function with alias in part of the code
+
+[*Solved issues]
+
+* [@https://github.com/boostorg/geometry/issues/1226 1226] [@https://github.com/boostorg/geometry/issues/1326 1326] Fix extra round on ring
+
+
 [/=================]
 [heading Boost 1.85]
 [/=================]

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

@@ -88,72 +88,155 @@ inline bool corresponding_turn(Turn const& turn, Turn const& start_turn,
     return count == 2;
 }
 
+//  Verify turns (other than start, and cross) if they are present in the map, and if so,
+//  if they have the other turns as corresponding, discard the start turn.
+template <typename Turns, typename TurnBySegmentMap, typename Geometry0, typename Geometry1>
+void discard_duplicate_start_turns(Turns& turns,
+                                   TurnBySegmentMap const& start_turns_by_segment,
+                                   Geometry0 const& geometry0,
+                                   Geometry1 const& geometry1)
+{
+    using multi_and_ring_id_type = std::pair<signed_size_type, signed_size_type>;
+    auto adapt_id = [](segment_identifier const& seg_id)
+    {
+        return multi_and_ring_id_type{seg_id.multi_index, seg_id.ring_index};
+    };
+
+    for (auto const& item : util::enumerate(turns))
+    {
+        // Any turn which "crosses" does not have a corresponding turn.
+        // Also avoid comparing "start" with itself, or with touch/touch_interior
+        auto const& turn = item.value;
+        if (turn.method == method_crosses || turn.method == method_start)
+        {
+            continue;
+        }
+        for (auto const& op : turn.operations)
+        {
+            auto it = start_turns_by_segment.find(adapt_id(op.seg_id));
+            if (it != start_turns_by_segment.end())
+            {
+                for (std::size_t const& i : it->second)
+                {
+                    if (turns[i].cluster_id == turn.cluster_id
+                        && corresponding_turn(turn, turns[i], geometry0, geometry1))
+                    {
+                        turns[i].discarded = true;
+                    }
+                }
+            }
+        }
+    }
+}
+
+//  Discard turns for the following (rare) case:
+//  - they are consecutive
+//  - the first has a touch, the second a touch_interior
+// And then one of the segments touches the others next in the middle.
+// This is geometrically not possible, and caused by floating point precision.
+// Discard the second (touch interior)
+template <typename Turns, typename Geometry0, typename Geometry1>
+void discard_touch_touch_interior_turns(Turns& turns,
+                                        Geometry0 const& geometry0,
+                                        Geometry1 const& geometry1)
+{
+    for (auto const& current_item : util::enumerate(turns))
+    {
+        if (current_item.value.method != method_touch_interior)
+        {
+            // Because touch_interior is a rarer case, it is more efficient to start with that
+            continue;
+        }
+        for (auto const& previous_item : util::enumerate(turns))
+        {
+            if (previous_item.value.method != method_touch)
+            {
+                continue;
+            }
+
+            // Compare 0 with 0 and 1 with 1
+            // Note that 0 has always source 0 and 1 has always source 1
+            // (not in buffer). Therefore this comparison is OK.
+            // MAYBE we need to check for buffer.
+            bool const common0 = current_item.value.operations[0].seg_id == previous_item.value.operations[0].seg_id;
+            bool const common1 = current_item.value.operations[1].seg_id == previous_item.value.operations[1].seg_id;
+
+            // If one of the operations is common, and the other is not, then there is one comment segment.
+            bool const has_common_segment = common0 != common1;
+
+            if (! has_common_segment)
+            {
+                continue;
+            }
+
+            // If the second index (1) is common, we need to check consecutivity of the first index (0)
+            // and vice versa.
+            bool const consecutive =  
+                common1 ? is_consecutive(previous_item.value.operations[0].seg_id, current_item.value.operations[0].seg_id, geometry0, geometry1)
+                : is_consecutive(previous_item.value.operations[1].seg_id, current_item.value.operations[1].seg_id, geometry0, geometry1);              
+
+            if (consecutive)
+            {
+                turns[current_item.index].discarded = true;
+            }
+        }
+    }
+}
+
 template <typename Turns, typename Geometry0, typename Geometry1>
-inline void discard_duplicate_start_turns(Turns& turns,
-                                          Geometry0 const& geometry0,
-                                          Geometry1 const& geometry1)
+void discard_duplicate_turns(Turns& turns,
+                             Geometry0 const& geometry0,
+                             Geometry1 const& geometry1)
 {
     // Start turns are generated, in case the previous turn is missed.
     // But often it is not missed, and then it should be deleted.
     // This is how it can be
     // (in float, collinear, points far apart due to floating point precision)
     //   [m, i s:0, v:6 1/1 (1) // u s:1, v:5  pnt (2.54044, 3.12623)]
     //   [s, i s:0, v:7 0/1 (0) // u s:1, v:5  pnt (2.70711, 3.29289)]
+    //
+    // Also, if two turns are consecutive, and one is touch and the other touch_interior,
+    // the touch_interior is discarded.
 
     using multi_and_ring_id_type = std::pair<signed_size_type, signed_size_type>;
 
-    auto adapt_id = [](segment_identifier const& seg_id)
+    auto add_to_map = [](auto const& turn, auto& map, std::size_t index)
     {
-        return multi_and_ring_id_type{seg_id.multi_index, seg_id.ring_index};
+        auto adapt_id = [](segment_identifier const& seg_id)
+        {
+            return multi_and_ring_id_type{seg_id.multi_index, seg_id.ring_index};
+        };
+        for (auto const& op : turn.operations)
+        {
+            map[adapt_id(op.seg_id)].insert(index);
+        }
     };
 
-    // 1 Build map of start turns (multi/ring-id -> turn indices)
-    std::map<multi_and_ring_id_type, std::vector<std::size_t>> start_turns_per_segment;
-    std::size_t index = 0;
-    for (auto const& turn : turns)
+    // Build map of start turns (multi/ring-id -> turn indices)
+    // and count touch and touch_interior turns (to verify if later checks are needed)
+    std::map<multi_and_ring_id_type, std::set<std::size_t>> start_turns_by_segment;
+    std::size_t touch_count = 0;
+    std::size_t touch_interior_count = 0;
+    for (auto const& item : util::enumerate(turns))
     {
-        if (turn.method == method_start)
+        auto const& turn = item.value;
+        switch(turn.method)
         {
-            for (auto const& op : turn.operations)
-            {
-                start_turns_per_segment[adapt_id(op.seg_id)].push_back(index);
-            }
+            case method_start: add_to_map(turn, start_turns_by_segment, item.index); break;
+            case method_touch: touch_count++; break;
+            case method_touch_interior: touch_interior_count++; break;
+            default: break;
         }
-        index++;
     }
 
-    // 2: Verify all other turns if they are present in the map, and if so,
-    //    if they have the other turns as corresponding
-    for (auto const& turn : turns)
+    if (!start_turns_by_segment.empty())
     {
-        // Any turn which "crosses" does not have a corresponding turn.
-        // Also avoid comparing "start" with itself.
-        if (turn.method != method_crosses && turn.method != method_start)
-        {
-            for (auto const& op : turn.operations)
-            {
-                auto it = start_turns_per_segment.find(adapt_id(op.seg_id));
-                if (it != start_turns_per_segment.end())
-                {
-                    for (std::size_t const& i : it->second)
-                    {
-                        if (turns[i].cluster_id != turn.cluster_id)
-                        {
-                            // The turns are not part of the same cluster,
-                            // or one is clustered and the other is not.
-                            // This is not corresponding.
-                            continue;
-                        }
-                        if (corresponding_turn(turn, turns[i],
-                                               geometry0, geometry1))
-                        {
-                            turns[i].discarded = true;
-                        }
-                    }
-                }
-            }
-        }
-        index++;
+        discard_duplicate_start_turns(turns, start_turns_by_segment, geometry0, geometry1);
+    }
+
+    if (touch_count > 0 && touch_interior_count > 0)
+    {
+       discard_touch_touch_interior_turns(turns, geometry0, geometry1);
     }
 }
 

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

@@ -441,7 +441,7 @@ inline void enrich_intersection_points(Turns& turns,
         has_colocations = ! clusters.empty();
     }
 
-    discard_duplicate_start_turns(turns, geometry1, geometry2);
+    discard_duplicate_turns(turns, geometry1, geometry2);
 
     // Discard turns not part of target overlay
     for (auto& turn : turns)