[intersection] remove closing point correctly

* the function (in namespace detail) is split and two parts are renamed on purpose, because functionality is changed
* tests are added
* intersection.cpp split into new file intersection_integer.cpp
* testing point count now if specified

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

@@ -168,56 +168,64 @@ inline void append_no_collinear(Range& range, Point const& point,
     }
 }
 
-template <typename Range, typename Strategy, typename RobustPolicy>
-inline void clean_closing_dups_and_spikes(Range& range,
-                                          Strategy const& strategy,
-                                          RobustPolicy const& robust_policy)
+// Should only be called internally, from traverse.
+template <typename Ring, typename Strategy, typename RobustPolicy>
+inline void remove_spikes_at_closure(Ring& ring, Strategy const& strategy,
+                                     RobustPolicy const& robust_policy)
 {
-    std::size_t const minsize
-        = core_detail::closure::minimum_ring_size
-            <
-                geometry::closure<Range>::value
-            >::value;
-
-    if (boost::size(range) <= minsize)
+    // It assumes a closed ring (whatever the closure value)
+    constexpr std::size_t min_size
+            = core_detail::closure::minimum_ring_size
+                    <
+                        geometry::closed
+                    >::value;
+
+    if (boost::size(ring) < min_size)
     {
+        // Don't act on too small rings.
         return;
     }
 
-    static bool const closed = geometry::closure<Range>::value == geometry::closed;
-
-// TODO: the following algorithm could be rewritten to first look for spikes
-// and then erase some number of points from the beginning of the Range
-
     bool found = false;
     do
     {
         found = false;
-        auto first = boost::begin(range);
-        auto second = first + 1;
-        auto ultimate = boost::end(range) - 1;
-        if (BOOST_GEOMETRY_CONDITION(closed))
-        {
-            ultimate--;
-        }
+        auto const first = boost::begin(ring);
+        auto const second = first + 1;
+        auto const penultimate = boost::end(ring) - 2;
 
         // Check if closing point is a spike (this is so if the second point is
         // considered as collinear w.r.t. the last segment)
-        if (point_is_collinear(*second, *ultimate, *first,
+        if (point_is_collinear(*second, *penultimate, *first,
                                strategy.side(), // TODO: Pass strategy?
                                robust_policy))
         {
-            range::erase(range, first);
-            if (BOOST_GEOMETRY_CONDITION(closed))
-            {
-                // Remove closing last point
-                range::resize(range, boost::size(range) - 1);
-                // Add new closing point
-                range::push_back(range, range::front(range));
-            }
+            // Remove first point and last point
+            range::erase(ring, first);
+            range::resize(ring, boost::size(ring) - 1);
+            // Close the ring again
+            range::push_back(ring, range::front(ring));
+
             found = true;
         }
-    } while(found && boost::size(range) > minsize);
+    } while (found && boost::size(ring) >= min_size);
+}
+
+template <typename Ring, typename Strategy>
+inline void fix_closure(Ring& ring, Strategy const& strategy)
+{
+    if (BOOST_GEOMETRY_CONDITION(geometry::closure<Ring>::value == geometry::open))
+    {
+        if (! boost::empty(ring)
+            && detail::equals::equals_point_point(range::front(ring), range::back(ring), strategy))
+        {
+            // Correct closure: traversal automatically closes rings.
+            // Depending on the geometric configuration,
+            // remove_spikes_at_closure can remove the closing point.
+            // But it does not always do that. Therefore it is corrected here explicitly.
+            range::resize(ring, boost::size(ring) - 1);
+        }
+    }
 }
 
 

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

@@ -274,6 +274,9 @@ struct traversal_ring_creator
 
         if (traverse_error == traverse_error_none)
         {
+            remove_spikes_at_closure(ring, m_strategy, m_robust_policy);
+            fix_closure(ring, m_strategy);
+
             std::size_t const min_num_points
                     = core_detail::closure::minimum_ring_size
                             <
@@ -282,7 +285,6 @@ struct traversal_ring_creator
 
             if (geometry::num_points(ring) >= min_num_points)
             {
-                clean_closing_dups_and_spikes(ring, m_strategy, m_robust_policy);
                 rings.push_back(ring);
 
                 m_trav.finalize_visit_info(m_turn_info_map);

test/algorithms/overlay/overlay_cases.hpp

@@ -1113,6 +1113,12 @@ static std::string issue_1108[2] =
     "POLYGON((22 1,22 0,14 0,18 -1.2696790939262529996,12 0,22 1))"
 };
 
+static std::string issue_1184[2] =
+{
+    "POLYGON((1169 177,2004 177,2004 1977,1262 1977,1169 177))",
+    "POLYGON((1179 371,1175 287,1179 287,1179 371))"
+};
+
 static std::string ggl_list_20120229_volker[3] =
     {
     "POLYGON((1716 1554,2076 2250,2436 2352,2796 1248,3156 2484,3516 2688,3516 2688,3156 2484,2796 1248,2436 2352,2076 2250, 1716 1554))",

test/algorithms/set_operations/intersection/Jamfile

@@ -29,4 +29,5 @@ test-suite boost-geometry-algorithms-intersection
     [ run intersection_pl_l.cpp               : : : : algorithms_intersection_pl_l ]
     [ run intersection_pl_pl.cpp              : : : : algorithms_intersection_pl_pl ]
     [ run intersection_tupled.cpp             : : : : algorithms_intersection_tupled ]
+    [ run intersection_integer.cpp            : : : : algorithms_intersection_integer ]
     ;

test/algorithms/set_operations/intersection/intersection.cpp

@@ -826,33 +826,6 @@ void test_rational()
         1, 7, 5.47363293);
 }
 
-template <typename CoordinateType>
-void test_ticket_10868(std::string const& wkt_out)
-{
-    typedef bg::model::point<CoordinateType, 2, bg::cs::cartesian> point_type;
-    typedef bg::model::polygon
-        <
-            point_type, /*ClockWise*/false, /*Closed*/false
-        > polygon_type;
-    typedef bg::model::multi_polygon<polygon_type> multipolygon_type;
-
-    polygon_type polygon1;
-    bg::read_wkt(ticket_10868[0], polygon1);
-    polygon_type polygon2;
-    bg::read_wkt(ticket_10868[1], polygon2);
-
-    multipolygon_type multipolygon_out;
-    bg::intersection(polygon1, polygon2, multipolygon_out);
-    std::stringstream stream;
-    stream << bg::wkt(multipolygon_out);
-
-    BOOST_CHECK_EQUAL(stream.str(), wkt_out);
-
-    test_one<polygon_type, polygon_type, polygon_type>("ticket_10868",
-        ticket_10868[0], ticket_10868[1],
-        1, 7, 20266195244586.0);
-}
-
 int test_main(int, char* [])
 {
     BoostGeometryWriteTestConfiguration();
@@ -869,21 +842,6 @@ int test_main(int, char* [])
     test_rational<bg::model::d2::point_xy<boost::rational<int> > >();
 #endif
 
-#if defined(BOOST_GEOMETRY_TEST_FAILURES)
-    // ticket #10868 still fails for 32-bit integers
-    test_ticket_10868<int32_t>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
-
-#if !defined(BOOST_NO_INT64_T) || defined(BOOST_HAS_MS_INT64)
-    test_ticket_10868<int64_t>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
-#endif
-
-    if (BOOST_GEOMETRY_CONDITION(sizeof(long) * CHAR_BIT >= 64))
-    {
-        test_ticket_10868<long>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
-    }
-
-    test_ticket_10868<long long>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
-#endif
 #endif
 
 #if defined(BOOST_GEOMETRY_TEST_FAILURES)

test/algorithms/set_operations/intersection/intersection_integer.cpp

@@ -0,0 +1,122 @@
+// Boost.Geometry
+// Unit Test
+
+// Copyright (c) 2007-2023 Barend Gehrels, Amsterdam, the Netherlands.
+
+// This file was modified by Oracle on 2015-2022.
+// Modifications copyright (c) 2015-2022, Oracle and/or its affiliates.
+// Contributed and/or modified by Menelaos Karavelas, on behalf of Oracle
+// Contributed and/or modified by Adam Wulkiewicz, on behalf of Oracle
+
+// 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 "test_intersection.hpp"
+#include <algorithms/test_overlay.hpp>
+
+#include <algorithms/overlay/overlay_cases.hpp>
+
+#define TEST_INTERSECTION(caseid, clips, points, area) \
+    (test_one<Polygon, Polygon, Polygon>) \
+    ( #caseid, caseid[0], caseid[1], clips, points, area, settings)
+
+namespace
+{
+    std::string rectangles[2] =
+    {
+        "POLYGON((1000 1000,2000 1000,2000 2000,1000 2000))",
+        "POLYGON((500 500,1500 500,1500 1500,500 1500))"
+    };
+
+    std::string start_within[2] =
+    {
+        "POLYGON((1000 1000,2000 1000,2000 2000,1000 2000))",
+        "POLYGON((1250 1250,1500 750,1750 1250,1750 1750))"
+    };
+}
+
+
+template <typename Polygon>
+void test_areal()
+{
+    static bool is_open = bg::closure<Polygon>::value == bg::open;
+
+    ut_settings settings;
+    settings.test_point_count = true;
+
+    TEST_INTERSECTION(rectangles, 1, is_open ? 4 : 5, 250000.0);
+    TEST_INTERSECTION(start_within, 1, is_open ? 5 : 6, 218750.0);
+    TEST_INTERSECTION(issue_1184, 1, is_open ? 3 : 4, 156.0);
+}
+
+template <typename P>
+void test_all()
+{
+    using polygon = bg::model::polygon<P>;
+    using polygon_ccw = bg::model::polygon<P, false>;
+    using polygon_open = bg::model::polygon<P, true, false>;
+    using polygon_ccw_open = bg::model::polygon<P, false, false>;
+    test_areal<polygon>();
+    test_areal<polygon_ccw>();
+    test_areal<polygon_open>();
+    test_areal<polygon_ccw_open>();
+}
+
+template <typename CoordinateType>
+void test_ticket_10868(std::string const& wkt_out)
+{
+    using point_type = bg::model::point<CoordinateType, 2, bg::cs::cartesian>;
+    using polygon_type = bg::model::polygon
+        <
+            point_type, /*ClockWise*/false, /*Closed*/false
+        >;
+    using multipolygon_type = bg::model::multi_polygon<polygon_type>;
+
+    polygon_type polygon1;
+    polygon_type polygon2;
+    bg::read_wkt(ticket_10868[0], polygon1);
+    bg::read_wkt(ticket_10868[1], polygon2);
+
+    multipolygon_type multipolygon_out;
+    bg::intersection(polygon1, polygon2, multipolygon_out);
+    std::stringstream stream;
+    stream << bg::wkt(multipolygon_out);
+
+    // BOOST_CHECK_EQUAL(stream.str(), wkt_out);
+
+    test_one<polygon_type, polygon_type, polygon_type>("ticket_10868",
+        ticket_10868[0], ticket_10868[1],
+        1, 7, 20266195244586.0);
+}
+
+
+
+int test_main(int, char* [])
+{
+    BoostGeometryWriteTestConfiguration();
+
+    test_all<bg::model::d2::point_xy<std::int32_t> >();
+
+
+#if defined(BOOST_GEOMETRY_TEST_FAILURES)
+    // ticket #10868 still fails for 32-bit integers
+    test_ticket_10868<std::int32_t>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
+
+    test_ticket_10868<std::int64_t>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
+#endif
+
+ #if defined(BOOST_GEOMETRY_TEST_FAILURES)
+    // Ticket #10868 was normally not run for other types
+    // It results in a different area (might be fine)
+    // and it reports self intersections.
+    if (BOOST_GEOMETRY_CONDITION(sizeof(long) * CHAR_BIT >= 64))
+    {
+        test_ticket_10868<long>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
+    }
+
+    test_ticket_10868<long long>("MULTIPOLYGON(((33520458 6878575,33480192 14931538,31446819 18947953,30772384 19615678,30101303 19612322,30114725 16928001,33520458 6878575)))");
+ #endif
+
+    return 0;
+}