[overlay] move segment_count_on_ring and segment_distance to get_ring and add unit test for them

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

@@ -117,15 +117,48 @@ struct get_ring<multi_polygon_tag>
     }
 };
 
-
+// Returns the number of segments on a ring (regardless whether the ring is open or closed)
 template <typename Geometry>
-inline std::size_t segment_count_on_ring(Geometry const& geometry, segment_identifier const& seg_id)
+inline signed_size_type segment_count_on_ring(Geometry const& geometry,
+                                              ring_identifier const& ring_id)
 {
-    typedef typename geometry::tag<Geometry>::type tag;
+    using tag = typename geometry::tag<Geometry>::type;
-    ring_identifier const rid(0, seg_id.multi_index, seg_id.ring_index);
+
     // A closed polygon, a triangle of 4 points, including starting point,
     // contains 3 segments. So handle as if it is closed, and subtract one.
-    return geometry::num_points(detail::overlay::get_ring<tag>::apply(rid, geometry), true) - 1;
+    return geometry::num_points(detail::overlay::get_ring<tag>::apply(ring_id, geometry), true) - 1;
+}
+
+// Returns the number of segments on a ring (regardless whether the ring is open or closed)
+template <typename Geometry>
+inline signed_size_type segment_count_on_ring(Geometry const& geometry,
+                                              segment_identifier const& seg_id)
+{
+    return segment_count_on_ring(geometry, ring_identifier(0, seg_id.multi_index, seg_id.ring_index));
+}
+
+
+// Returns the distance between the second and the first segment identifier (second-first)
+// It supports circular behavior and for this it is necessary to pass the geometry.
+// It will not report negative values
+template <typename Geometry>
+inline signed_size_type segment_distance(Geometry const& geometry,
+        segment_identifier const& first, segment_identifier const& second)
+{
+    // It is an internal function, make sure the preconditions are met
+    BOOST_ASSERT(second.source_index == first.source_index);
+    BOOST_ASSERT(second.multi_index == first.multi_index);
+    BOOST_ASSERT(second.ring_index == first.ring_index);
+
+    signed_size_type const result = second.segment_index - first.segment_index;
+    if (second.segment_index >= first.segment_index)
+    {
+        return result;
+    }
+    // Take wrap into account, counting segments on the ring (passing any of the ids is fine).
+    // Suppose point_count=10 (10 points, 9 segments), first.seg_id=7, second.seg_id=2,
+    // then distance=9-7+2=4, being segments 7,8,0,1
+    return segment_count_on_ring(geometry, first) + result;
 }
 
 }} // namespace detail::overlay

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

@@ -378,47 +378,21 @@ public :
 
             if (is_origin)
             {
-                signed_size_type const segment_distance = calculate_segment_distance(op, departure_seg_id, geometry1, geometry2);
+                signed_size_type const sd
-                if (m_origin_count == 0 ||
+                        = departure_seg_id.source_index == 0
-                        segment_distance < m_origin_segment_distance)
+                          ? segment_distance(geometry1, departure_seg_id, op.seg_id)
+                          : segment_distance(geometry2, departure_seg_id, op.seg_id);
+
+                if (m_origin_count == 0 || sd < m_origin_segment_distance)
                 {
                     m_origin = potential_origin;
-                    m_origin_segment_distance = segment_distance;
+                    m_origin_segment_distance = sd;
                 }
                 m_origin_count++;
             }
         }
     }
 
-    template <typename Operation, typename Geometry1, typename Geometry2>
-    static signed_size_type segment_count_on_ring(Operation const& op,
-            Geometry1 const& geometry1,
-            Geometry2 const& geometry2)
-    {
-        // Take wrap into account
-        // Suppose point_count=10 (10 points, 9 segments), dep.seg_id=7, op.seg_id=2,
-        // then distance=9-7+2=4, being segments 7,8,0,1
-        return op.seg_id.source_index == 0
-            ? detail::overlay::segment_count_on_ring(geometry1, op.seg_id)
-            : detail::overlay::segment_count_on_ring(geometry2, op.seg_id);
-    }
-
-    template <typename Operation, typename Geometry1, typename Geometry2>
-    static signed_size_type calculate_segment_distance(Operation const& op,
-            segment_identifier const& departure_seg_id,
-            Geometry1 const& geometry1,
-            Geometry2 const& geometry2)
-    {
-        BOOST_ASSERT(op.seg_id.source_index == departure_seg_id.source_index);
-        signed_size_type result = op.seg_id.segment_index - departure_seg_id.segment_index;
-        if (op.seg_id.segment_index >= departure_seg_id.segment_index)
-        {
-            // dep.seg_id=5, op.seg_id=7, distance=2, being segments 5,6
-            return result;
-        }
-        return segment_count_on_ring(op, geometry1, geometry2) + result;
-    }
-
     void apply(Point const& turn_point)
     {
         // We need three compare functors:

include/boost/geometry/multi/algorithms/detail/overlay/get_ring.hpp

@@ -1,16 +0,0 @@
-// Boost.Geometry (aka GGL, Generic Geometry Library)
-
-// Copyright (c) 2007-2012 Barend Gehrels, Amsterdam, the Netherlands.
-
-// 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_MULTI_ALGORITHMS_DETAIL_OVERLAY_GET_RING_HPP
-#define BOOST_GEOMETRY_MULTI_ALGORITHMS_DETAIL_OVERLAY_GET_RING_HPP
-
-
-#include <boost/geometry/algorithms/detail/overlay/get_ring.hpp>
-
-
-#endif // BOOST_GEOMETRY_MULTI_ALGORITHMS_DETAIL_OVERLAY_GET_RING_HPP

test/algorithms/overlay/Jamfile

@@ -17,6 +17,7 @@ test-suite boost-geometry-algorithms-overlay
     : 
     [ run assemble.cpp                     : : : : algorithms_assemble ]
     [ run copy_segment_point.cpp           : : : : algorithms_copy_segment_point ]
+    [ run get_ring.cpp                     : : : : algorithms_get_ring ]
     [ run get_turn_info.cpp                : : : : algorithms_get_turn_info ]
     [ run get_turns.cpp                    : : : : algorithms_get_turns ]
     [ run get_turns_areal_areal.cpp        : : : : algorithms_get_turns_areal_areal ]

test/algorithms/overlay/get_ring.cpp

@@ -0,0 +1,166 @@
+// Boost.Geometry
+// Unit Test
+
+// Copyright (c) 2021 Barend Gehrels, Amsterdam, the Netherlands.
+
+// 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)
+
+#include <geometry_test_common.hpp>
+
+#include <boost/geometry/algorithms/detail/overlay/get_ring.hpp>
+
+#include <boost/geometry/algorithms/area.hpp>
+#include <boost/geometry/algorithms/is_valid.hpp>
+
+#include <boost/geometry/strategies/strategies.hpp>
+#include <boost/geometry/geometries/geometries.hpp>
+#include <boost/geometry/io/wkt/wkt.hpp>
+
+namespace
+{
+
+bool g_debug = false;
+
+std::string const simplex = "POLYGON((0 2,1 2,1 1,0 1,0 2))";
+std::string const case_a = "POLYGON((1 0,0 5,5 2,1 0),(2 1,3 2,1 3,2 1))";
+std::string const multi = "MULTIPOLYGON(((0 0,0 5,5 5,5 0,0 0),(1 1,2 1,2 2,1 2,1 1),(3 3,4 3,4 4,3 4,3 3)),((6 6,6 10,10 10,10 6,6 6),(7 7,8 7,8 8,7 7)))";
+}
+
+template <typename Geometry>
+void test_get_ring(std::string const& case_id, std::string const& wkt,
+                   bg::ring_identifier const& ring_id,
+                   std::string const& expected_wkt)
+{
+    using tag = typename bg::tag<Geometry>::type;
+
+    Geometry geometry;
+    bg::read_wkt(wkt, geometry);
+
+    auto const ring = bg::detail::overlay::get_ring<tag>::apply(ring_id, geometry);
+
+    if (g_debug)
+    {
+        std::cout << case_id
+                  << " valid: " << bg::is_valid(geometry)
+                  << " area: " << bg::area(geometry)
+                  << " area(ring): " << bg::area(ring)
+                  << " wkt(ring): " << bg::wkt(ring)
+                  << std::endl;
+    }
+
+    std::ostringstream out;
+    out << bg::wkt(ring);
+    std::string const detected = out.str();
+    BOOST_CHECK_MESSAGE(detected == expected_wkt, "get_ring: " << case_id
+                        << " expected: " << expected_wkt
+                        << " detected: " << detected);
+}
+
+template <typename Geometry>
+void test_segment_count_on_ring(std::string const& case_id, std::string const& wkt,
+                   bg::ring_identifier const& ring_id, bg::signed_size_type expected_count)
+{
+    Geometry geometry;
+    bg::read_wkt(wkt, geometry);
+
+    auto const detected_count = bg::detail::overlay::segment_count_on_ring(geometry, ring_id);
+
+    BOOST_CHECK_MESSAGE(detected_count == expected_count,
+                        "test_get_ring: " << case_id
+                        << " expected: " << expected_count
+                        << " detected: " << detected_count);
+}
+
+template <typename Geometry>
+void test_segment_distance(std::string const& case_id, int line, std::string const& wkt,
+                           bg::segment_identifier const& id1,
+                           bg::segment_identifier const& id2,
+                           bg::signed_size_type expected_distance)
+{
+    Geometry geometry;
+    bg::read_wkt(wkt, geometry);
+
+    auto const detected_distance = bg::detail::overlay::segment_distance(geometry, id1, id2);
+
+    BOOST_CHECK_MESSAGE(detected_distance == expected_distance,
+                        "segment_distance: " << case_id << " (" << line << ")"
+                        << " expected: " << expected_distance
+                        << " detected: " << detected_distance);
+}
+
+template <typename Point, bool Closed>
+void test_get_ring()
+{
+    using ring = bg::model::ring<Point, Closed>;
+    using polygon = bg::model::polygon<Point, Closed>;
+    using multi_polygon = bg::model::multi_polygon<polygon>;
+
+    test_get_ring<ring>("ring_simplex", simplex, {0, -1, -1}, simplex);
+    test_get_ring<polygon>("polygon_simplex", simplex, {0, -1, -1}, simplex);
+    test_get_ring<polygon>("case_a_outer", case_a, {0, -1, -1}, "POLYGON((1 0,0 5,5 2,1 0))");
+    test_get_ring<polygon>("case_a_0", case_a, {0, -1, 0}, "POLYGON((2 1,3 2,1 3,2 1))");
+    test_get_ring<multi_polygon>("multi_0_outer", multi, {0, 0, -1}, "POLYGON((0 0,0 5,5 5,5 0,0 0))");
+    test_get_ring<multi_polygon>("multi_0_0", multi, {0, 0, 0}, "POLYGON((1 1,2 1,2 2,1 2,1 1))");
+    test_get_ring<multi_polygon>("multi_0_1", multi, {0, 0, 1}, "POLYGON((3 3,4 3,4 4,3 4,3 3))");
+    test_get_ring<multi_polygon>("multi_1_outer", multi, {0, 1, -1}, "POLYGON((6 6,6 10,10 10,10 6,6 6))");
+    test_get_ring<multi_polygon>("multi_1_1", multi, {0, 1, 0}, "POLYGON((7 7,8 7,8 8,7 7))");
+}
+
+template <typename Point, bool Closed>
+void test_segment_count_on_ring()
+{
+    using ring = bg::model::ring<Point, true, Closed>;
+    using polygon = bg::model::polygon<Point, true, Closed>;
+    using multi_polygon = bg::model::multi_polygon<polygon>;
+
+    test_segment_count_on_ring<ring>("ring_simplex", simplex, {0, -1, -1}, 4);
+    test_segment_count_on_ring<polygon>("polygon_simplex", simplex, {0, -1, -1}, 4);
+    test_segment_count_on_ring<polygon>("case_a_outer", case_a, {0, -1, -1}, 3);
+    test_segment_count_on_ring<polygon>("case_a_0", case_a, {0, -1, 0}, 3);
+    test_segment_count_on_ring<multi_polygon>("multi_0_outer", multi, {0, 0, -1}, 4);
+    test_segment_count_on_ring<multi_polygon>("multi_0_0", multi, {0, 0, 0}, 4);
+    test_segment_count_on_ring<multi_polygon>("multi_0_1", multi, {0, 0, 1}, 4);
+    test_segment_count_on_ring<multi_polygon>("multi_1_outer", multi, {0, 1, -1}, 4);
+    test_segment_count_on_ring<multi_polygon>("multi_1_1", multi, {0, 1, 0}, 3);
+}
+
+template <typename Point, bool Closed>
+void test_segment_distance()
+{
+    using ring = bg::model::ring<Point, true, Closed>;
+
+    std::string const case_id = "ring_simplex";
+
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 0}, {0, -1, -1, 0}, 0);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 0}, {0, -1, -1, 1}, 1);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 0}, {0, -1, -1, 2}, 2);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 0}, {0, -1, -1, 3}, 3);
+
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 1}, {0, -1, -1, 0}, 3);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 1}, {0, -1, -1, 1}, 0);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 1}, {0, -1, -1, 2}, 1);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 1}, {0, -1, -1, 3}, 2);
+
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 2}, {0, -1, -1, 0}, 2);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 2}, {0, -1, -1, 1}, 3);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 2}, {0, -1, -1, 2}, 0);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 2}, {0, -1, -1, 3}, 1);
+
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 3}, {0, -1, -1, 0}, 1);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 3}, {0, -1, -1, 1}, 2);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 3}, {0, -1, -1, 2}, 3);
+    test_segment_distance<ring>(case_id, __LINE__, simplex, {0, -1, -1, 3}, {0, -1, -1, 3}, 0);
+}
+
+int test_main(int, char* [])
+{
+    using point = bg::model::point<default_test_type, 2, bg::cs::cartesian>;
+    test_get_ring<point, true>();
+    test_segment_count_on_ring<point, true>();
+    test_segment_count_on_ring<point, false>();
+    test_segment_distance<point, true>();
+    test_segment_distance<point, false>();
+    return 0;
+}

test/count_set.hpp

@@ -10,8 +10,6 @@
 #ifndef GEOMETRY_TEST_COUNT_SET_HPP
 #define GEOMETRY_TEST_COUNT_SET_HPP
 
-#include <boost/foreach.hpp>
-
 #include <set>
 #include <ostream>
 
@@ -54,7 +52,7 @@ struct count_set
     friend std::ostream &operator<<(std::ostream &os, const count_set& s)
     {
        os << "{";
-       BOOST_FOREACH(std::size_t const& value, s.m_values)
+       for (std::size_t const& value : s.m_values)
        {
            os << " " << value;
        }
@@ -84,7 +82,7 @@ private :
         else if (a.size() > 1 && b.size() == 1)
         {
             // One of them is optional, add the second
-            BOOST_FOREACH(std::size_t const& value, a)
+            for (std::size_t const& value : a)
             {
                 result.insert(value + *b.begin());
             }