fix: consider clusters in turn_in_piece_visitor

Author: barendgehrels

include/boost/geometry/algorithms/detail/buffer/buffered_piece_collection.hpp

@@ -466,8 +466,8 @@ struct buffered_piece_collection
         turn_in_piece_visitor
             <
                 geometry::cs_tag_t<point_type>,
-                turn_vector_type, piece_vector_type, DistanceStrategy, Strategy
-            > visitor(m_turns, m_pieces, m_distance_strategy, m_strategy);
+                turn_vector_type, cluster_type, piece_vector_type, DistanceStrategy, Strategy
+            > visitor(m_turns, m_clusters, m_pieces, m_distance_strategy, m_strategy);
 
         geometry::partition
             <

include/boost/geometry/algorithms/detail/buffer/turn_in_piece_visitor.hpp

@@ -38,6 +38,7 @@ template
 <
     typename CsTag,
     typename Turns,
+    typename Clusters,
     typename Pieces,
     typename DistanceStrategy,
     typename UmbrellaStrategy
@@ -46,6 +47,7 @@ template
 class turn_in_piece_visitor
 {
     Turns& m_turns; // because partition is currently operating on const input only
+    Clusters const& m_clusters;
     Pieces const& m_pieces; // to check for piece-type
     DistanceStrategy const& m_distance_strategy; // to check if point is on original or one_sided
     UmbrellaStrategy const& m_umbrella_strategy;
@@ -77,6 +79,39 @@ class turn_in_piece_visitor
         return false;
     }
 
+    // Returns true if the turn is part of a cluster, and one of the other turns in the same
+    // cluster is involved in the same piece as the operations in the turn.
+    template <typename Turn, typename Piece>
+    inline bool skip_by_same_cluster(Turn const& turn, Piece const& piece) const
+    {
+        auto it = m_clusters.find(turn.cluster_id);
+        if (it == m_clusters.end())
+        {
+            return false;
+        }
+
+        for (auto const& index : it->second.turn_indices)
+        {
+            if (index == static_cast<signed_size_type>(turn.turn_index))
+            {
+                continue;
+            }
+            auto const& other_turn = m_turns[index];
+            auto const& seg_id0 = other_turn.operations[0].seg_id;
+            auto const& seg_id1 = other_turn.operations[1].seg_id;
+
+            if (seg_id0.piece_index == piece.index
+                || seg_id1.piece_index == piece.index)
+            {
+                // One of the other turns in the same cluster is an intersection
+                // with the same piece.
+                // Therefore, the turn under inspection cannot be within that piece.
+                return true;
+            }
+        }
+        return false;
+    }
+
     template <typename NumericType>
     inline bool is_one_sided(NumericType const& left, NumericType const& right) const
     {
@@ -96,10 +131,11 @@ class turn_in_piece_visitor
 
 public:
 
-    inline turn_in_piece_visitor(Turns& turns, Pieces const& pieces,
+    inline turn_in_piece_visitor(Turns& turns, Clusters const& clusters, Pieces const& pieces,
                                  DistanceStrategy const& distance_strategy,
                                  UmbrellaStrategy const& umbrella_strategy)
         : m_turns(turns)
+        , m_clusters(clusters)
         , m_pieces(pieces)
         , m_distance_strategy(distance_strategy)
         , m_umbrella_strategy(umbrella_strategy)
@@ -140,7 +176,12 @@ class turn_in_piece_visitor
 
         if (piece.type == geometry::strategy::buffer::buffered_empty_side)
         {
-            return false;
+            return true;
+        }
+
+        if (skip_by_same_cluster(turn, piece))
+        {
+            return true;
         }
 
         if (piece.type == geometry::strategy::buffer::buffered_point)

test/algorithms/buffer/buffer_cases.hpp

@@ -82,11 +82,10 @@ static std::string const rt_w16
 static std::string const rt_w17
     = "MULTIPOLYGON(((3 1,4 2,4 1,3 1)),((5 3,6 4,6 3,5 3)),((5 0,5 1,6 1,6 0,5 0)),((8 5,9 6,9 5,8 5)),((8 5,7 4,7 5,8 5)))";
 
-// Error in turn_in_piece, see readme
+// Needs checking turns of same cluster in turn_in_piece
 static std::string const rt_w18
     = "MULTIPOLYGON(((4 4,4 5,5 4,4 4)),((5 6,6 7,6 6,5 6)),((5 1,4 1,4 2,5 3,5 1)),((7 6,7 7,8 7,7 6)),((0 6,1 6,1 5,1 4,0 4,0 6)),((1 8,2 7,1 7,1 8)),((1 8,2 9,2 8,1 8)),((1 6,1 7,2 6,1 6)),((7 3,7 2,6 2,7 3)),((7 3,7 4,8 4,8 3,7 3)),((3 2,3 1,2 1,3 2)),((3 2,2 2,2 3,3 3,3 2)),((5 8,5 7,4 7,4 8,5 8)),((5 8,6 9,6 8,5 8)))";
 
-
 // Contains a cc turn (1) located wrongly.
 // Reported at 1/1, but should be 0/1 - or it should have a segment id belonging to previous segment.
 // Fixed by removing specific arriving handling.
@@ -102,6 +101,7 @@ static std::string const rt_w21
     = "MULTIPOLYGON(((2 8,2 9,3 8,2 8)),((4 6,4 7,5 6,4 6)),((4 2,5 2,4 1,4 2)))";
 
 // Needs a spike. Also, it misses a small triangle, so area is not completely correct
+// Also, needs checking turns of same cluster in turn_in_piece
 static std::string const rt_w22
     = "MULTIPOLYGON(((6 3,6 4,7 4,7 3,6 3)),((6 1,6 2,7 2,6 1)),((2 8,2 9,3 9,2 8)),((4 7,5 8,5 7,4 7)),((2 1,2 0,1 0,2 1)),((2 1,2 2,3 2,3 1,2 1)),((3 3,2 2,2 3,3 3)),((3 3,3 4,4 3,3 3)),((0 5,1 4,1 3,0 3,0 5)),((8 6,9 6,9 5,8 5,8 6)),((8 6,7 6,6 6,7 7,8 6)),((1 3,1 2,0 2,1 3)),((4 3,5 3,4 2,4 3)),((4 2,5 1,4 1,4 2)))";
 
@@ -157,4 +157,25 @@ static std::string const rt_w34
 static std::string const rt_w35
     = "MULTIPOLYGON(((6 6,6 7,7 7,7 6,6 6)),((5 4,5 5,6 5,6 4,5 4)),((4 0,4 1,5 0,4 0)),((0 0,1 1,1 0,0 0)),((7 0,7 1,8 1,8 0,7 0)),((0 2,0 3,1 3,1 2,0 2)),((3 3,4 2,3 2,3 3)),((3 3,3 4,4 4,4 3,3 3)))";
 
+// Needs checking turns of same cluster in turn_in_piece
+// Also it has an interesting shape and several overlapping collinear borders in its rings.
+static std::string const rt_w36
+    = "MULTIPOLYGON(((5 0,6 1,6 0,5 0)),((2 0,2 1,3 0,2 0)),((1 4,1 5,2 4,1 4)),((2 7,3 8,3 7,2 7)),((1 8,1 9,2 9,1 8)),((3 4,3 5,4 5,3 4)),((7.5 4.5,8 5,8 4,7 4,7 5,7.5 4.5)))";
+
+// Needs checking turns of same cluster in turn_in_piece
+static std::string const rt_w37
+    = "MULTIPOLYGON(((5 7,6 8,6 7,5 7)),((1 7,1 8,2 8,1 7)),((1 5,2 6,2 5,1 5)),((1 5,2 4,1 4,1 5)))";
+
+// Needs checking turns of same cluster in turn_in_piece
+static std::string const rt_w38
+    = "MULTIPOLYGON(((8 6,8 7,9 7,9 6,8 6)),((1 3,1 4,2 4,1 3)),((2 3,3 4,3 3,2 3)),((8 3,8 4,9 4,9 3,8 3)),((1 8,1 9,2 8,1 8)),((1 0,1 1,2 0,1 0)),((5 1,5 2,6 1,5 1)),((5 5,5 6,6 6,6 5,5 5)))";
+
+// Needs checking turns of same cluster in turn_in_piece
+static std::string const rt_w39
+    = "MULTIPOLYGON(((1 1,1 2,2 1,1 1)),((3 6,4 7,4 6,3 6)),((1 6,2 7,2 6,1 6)),((1 6,2 5,1 5,1 6)),((1 7,1 8,2 8,1 7)),((7 8,8 7,8 6,7 6,7 8)))";
+
+// Needs checking turns of same cluster in turn_in_piece
+static std::string const rt_w40
+    = "MULTIPOLYGON(((6 8,6 9,7 8,6 8)),((7 6,8 6,8 5,7 4,7 6)),((8 8,8 9,9 9,8 8)),((1 6,2 5,1 5,1 6)),((1 6,2 7,2 6,1 6)),((2 5,2 6,3 6,2 5)),((1 5,2 4,1 4,1 5)))";
+
 #endif

test/algorithms/buffer/buffer_linestring_geo.cpp

@@ -116,9 +116,11 @@ void test_linestring_aimes()
     std::set<int> const skip_cases_miter_flat{};
 #else
     // Cases where the algorithm is still failing.
-    std::set<int> const skip_cases_round_round{17, 22, 38, 181, 196};
-    std::set<int> const skip_cases_round_flat{17, 22, 38, 103, 196};
-    std::set<int> const skip_cases_miter_flat{17, 18, 22, 38, 103, 196};
+    // It might differ in debug/release mode.
+    // It might also depend on the compiler and on the operating system.
+    std::set<int> const skip_cases_round_round{17, 22, 38, 67, 75, 109, 163, 181, 143, 196};
+    std::set<int> const skip_cases_round_flat{17, 22, 38, 67, 75, 103, 109, 196};
+    std::set<int> const skip_cases_miter_flat{17, 18, 22, 38, 67, 75, 103, 109, 196};
 #endif
 
     for (auto const& enumerated : bg::util::enumerate(testcases_aimes))

test/algorithms/buffer/buffer_multi_polygon.cpp

@@ -625,10 +625,7 @@ void test_all()
     TEST_BUFFER(rt_w15, join_miter, end_flat, 80.1348, 1.0);
     TEST_BUFFER(rt_w16, join_miter, end_flat, 31.6495, 1.0);
     TEST_BUFFER(rt_w17, join_miter, end_flat, 33.74264, 1.0);
-
-#if defined(BOOST_GEOMETRY_TEST_FAILURES)
-    TEST_BUFFER(rt_w18, join_miter, end_flat, 82.4779, 1.0);
-#endif
+    TEST_BUFFER(rt_w18, join_miter, end_flat, 83.4779, 1.0);
 
 #if defined(BOOST_GEOMETRY_TEST_FAILURES) || defined(BOOST_GEOMETRY_CONCEPT_FIX_ARRIVAL)
     // See comments at issue issue_1262
@@ -637,10 +634,7 @@ void test_all()
 
     TEST_BUFFER(rt_w20, join_miter, end_flat, 63.0269, 1.0);
     TEST_BUFFER(rt_w21, join_miter, end_flat, 26.3137, 1.0);
-
-#if defined(BOOST_GEOMETRY_TEST_FAILURES)
-    TEST_BUFFER(rt_w22, join_miter, end_flat, 86.0416, 1.0);
-#endif    
+    TEST_BUFFER(rt_w22, join_miter, end_flat, 86.1274, 1.0);
 
     TEST_BUFFER(rt_w23, join_miter, end_flat, 59.5711, 1.0);
 
@@ -672,6 +666,11 @@ void test_all()
     TEST_BUFFER_VALIDITY_FALSE_NEGATIVE(rt_w35, join_round32, end_flat, 51.63174, 1.0);
 
     TEST_BUFFER(rt_w35, join_miter, end_flat, 57.6569, 1.0);
+    TEST_BUFFER(rt_w36, join_miter, end_flat, 60.1274, 1.0);
+    TEST_BUFFER(rt_w37, join_miter, end_flat, 30.6569, 1.0);
+    TEST_BUFFER(rt_w38, join_miter, end_flat, 68.2279, 1.0);
+    TEST_BUFFER(rt_w39, join_miter, end_flat, 46.2279, 1.0);
+    TEST_BUFFER(rt_w40, join_miter, end_flat, 49.0490, 1.0);
 
     test_one<multi_polygon_type, polygon_type>("nores_mt_1", nores_mt_1, join_round32, end_flat, 13.4113, 1.0);
     test_one<multi_polygon_type, polygon_type>("nores_mt_2", nores_mt_2, join_round32, end_flat, 17.5265, 1.0);