[overlay] fix ticket #10108: rings only touching should check for u/u turns

and don't call point_on_border because that might be located on the border of the
other geometry. This also avoids passing the mid_point boolean, and including
within_code in ring_properties

doc/release_notes.qbk

@@ -23,6 +23,7 @@
 
 [*Solved tickets]
 
+* [@https://svn.boost.org/trac/boost/ticket/10108 10108] Error in overlay operations in specific cases where geometries touch at one point
 * [@https://svn.boost.org/trac/boost/ticket/10747 10747] Error in rescaling causing errors in areal/areal set operations
 
 [*Bugfixes]

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

@@ -956,7 +956,7 @@ struct buffered_piece_collection
             if (! it->has_intersections())
             {
                 ring_identifier id(0, index, -1);
-                selected[id] = properties(*it, true);
+                selected[id] = properties(*it);
             }
         }
 
@@ -968,7 +968,7 @@ struct buffered_piece_collection
                 ++it, ++index)
         {
             ring_identifier id(2, index, -1);
-            selected[id] = properties(*it, true);
+            selected[id] = properties(*it);
         }
 
         detail::overlay::assign_parents(offsetted_rings, traversed_rings, selected, true);

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

@@ -58,8 +58,9 @@ namespace detail { namespace overlay
 {
 
 
-template <typename TurnPoints, typename CountMap>
+template <typename TurnPoints, typename TurnInfoMap>
-inline void count_turns(CountMap& count_map, TurnPoints const& turn_points)
+inline void get_ring_turn_info(TurnInfoMap& turn_info_map,
+        TurnPoints const& turn_points)
 {
     typedef typename boost::range_value<TurnPoints>::type turn_point_type;
     typedef typename turn_point_type::container_type container_type;
@@ -90,8 +91,11 @@ inline void count_turns(CountMap& count_map, TurnPoints const& turn_points)
 
             if (! skip)
             {
-                // We set it to 1, will be changed on next commit
+                turn_info_map[ring_id].has_normal_turn = true;
-                count_map[ring_id] = 1;
+            }
+            else if (both_uu)
+            {
+                turn_info_map[ring_id].has_uu_turn = true;
             }
         }
     }
@@ -136,10 +140,10 @@ inline OutputIterator return_if_one_input_is_empty(Geometry1 const& geometry1,
 #endif
 
 
-    std::map<ring_identifier, int> empty;
+    std::map<ring_identifier, ring_turn_info> empty;
     std::map<ring_identifier, properties> all_of_one_of_them;
 
-    select_rings<Direction>(geometry1, geometry2, empty, all_of_one_of_them, false);
+    select_rings<Direction>(geometry1, geometry2, empty, all_of_one_of_them);
     ring_container_type rings;
     assign_parents(geometry1, geometry2, rings, all_of_one_of_them);
     return add_rings<GeometryOut>(all_of_one_of_them, geometry1, geometry2, rings, out);
@@ -248,8 +252,8 @@ std::cout << "traverse" << std::endl;
         std::cout << "traverse: " << timer.elapsed() << std::endl;
 #endif
 
-        std::map<ring_identifier, int> turn_count_per_ring;
+        std::map<ring_identifier, ring_turn_info> turn_info_per_ring;
-        count_turns(turn_count_per_ring, turn_points);
+        get_ring_turn_info(turn_info_per_ring, turn_points);
 
 #ifdef BOOST_GEOMETRY_TIME_OVERLAY
         std::cout << "count_turns: " << timer.elapsed() << std::endl;
@@ -262,8 +266,8 @@ std::cout << "traverse" << std::endl;
 
         // Select all rings which are NOT touched by any intersection point
         std::map<ring_identifier, properties> selected_ring_properties;
-        select_rings<Direction>(geometry1, geometry2, turn_count_per_ring,
+        select_rings<Direction>(geometry1, geometry2, turn_info_per_ring,
-                selected_ring_properties, ! turn_points.empty());
+                selected_ring_properties);
 
 #ifdef BOOST_GEOMETRY_TIME_OVERLAY
         std::cout << "select_rings: " << timer.elapsed() << std::endl;
@@ -277,7 +281,7 @@ std::cout << "traverse" << std::endl;
                  it != boost::end(rings);
                  ++it)
             {
-                selected_ring_properties[id] = properties(*it, true);
+                selected_ring_properties[id] = properties(*it);
                 selected_ring_properties[id].reversed = ReverseOut;
                 id.multi_index++;
             }

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

@@ -33,8 +33,7 @@ struct ring_properties
     Point point;
     area_type area;
 
-    // Filled by "update_selection_map"
+    // Filled by "update_ring_selection"
-    int within_code;
     bool reversed;
 
     // Filled/used by "assign_rings"
@@ -45,21 +44,22 @@ struct ring_properties
 
     inline ring_properties()
         : area(area_type())
-        , within_code(-1)
         , reversed(false)
         , discarded(false)
         , parent_area(-1)
     {}
 
     template <typename RingOrBox>
-    inline ring_properties(RingOrBox const& ring_or_box, bool midpoint)
+    inline ring_properties(RingOrBox const& ring_or_box)
-        : within_code(-1)
+        : reversed(false)
-        , reversed(false)
         , discarded(false)
         , parent_area(-1)
     {
         this->area = geometry::area(ring_or_box);
-        geometry::point_on_border(this->point, ring_or_box, midpoint);
+        // We should take a point somewhere in the middle of the ring,
+        // to avoid taking a point on a (self)tangency,
+        // in cases where multiple points come together
+        geometry::point_on_border(this->point, ring_or_box, true);
     }
 
     inline area_type get_area() const

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

@@ -33,6 +33,18 @@ namespace boost { namespace geometry
 namespace detail { namespace overlay
 {
 
+struct ring_turn_info
+{
+    bool has_uu_turn;
+    bool has_normal_turn;
+    bool within_other;
+
+    ring_turn_info()
+        : has_uu_turn(false)
+        , has_normal_turn(false)
+        , within_other(false)
+    {}
+};
 
 namespace dispatch
 {
@@ -46,18 +58,16 @@ namespace dispatch
     {
         template <typename Geometry, typename RingPropertyMap>
         static inline void apply(Box const& box, Geometry const& ,
-                ring_identifier const& id, RingPropertyMap& ring_properties,
+                ring_identifier const& id, RingPropertyMap& ring_properties)
-                bool midpoint)
         {
-            ring_properties[id] = typename RingPropertyMap::mapped_type(box, midpoint);
+            ring_properties[id] = typename RingPropertyMap::mapped_type(box);
         }
 
         template <typename RingPropertyMap>
         static inline void apply(Box const& box,
-                ring_identifier const& id, RingPropertyMap& ring_properties,
+                ring_identifier const& id, RingPropertyMap& ring_properties)
-                bool midpoint)
         {
-            ring_properties[id] = typename RingPropertyMap::mapped_type(box, midpoint);
+            ring_properties[id] = typename RingPropertyMap::mapped_type(box);
         }
     };
 
@@ -66,23 +76,21 @@ namespace dispatch
     {
         template <typename Geometry, typename RingPropertyMap>
         static inline void apply(Ring const& ring, Geometry const& ,
-                    ring_identifier const& id, RingPropertyMap& ring_properties,
+                    ring_identifier const& id, RingPropertyMap& ring_properties)
-                    bool midpoint)
         {
             if (boost::size(ring) > 0)
             {
-                ring_properties[id] = typename RingPropertyMap::mapped_type(ring, midpoint);
+                ring_properties[id] = typename RingPropertyMap::mapped_type(ring);
             }
         }
 
         template <typename RingPropertyMap>
         static inline void apply(Ring const& ring,
-                    ring_identifier const& id, RingPropertyMap& ring_properties,
+                    ring_identifier const& id, RingPropertyMap& ring_properties)
-                    bool midpoint)
         {
             if (boost::size(ring) > 0)
             {
-                ring_properties[id] = typename RingPropertyMap::mapped_type(ring, midpoint);
+                ring_properties[id] = typename RingPropertyMap::mapped_type(ring);
             }
         }
     };
@@ -93,13 +101,12 @@ namespace dispatch
     {
         template <typename Geometry, typename RingPropertyMap>
         static inline void apply(Polygon const& polygon, Geometry const& geometry,
-                    ring_identifier id, RingPropertyMap& ring_properties,
+                    ring_identifier id, RingPropertyMap& ring_properties)
-                    bool midpoint)
         {
             typedef typename geometry::ring_type<Polygon>::type ring_type;
             typedef select_rings<ring_tag, ring_type> per_ring;
 
-            per_ring::apply(exterior_ring(polygon), geometry, id, ring_properties, midpoint);
+            per_ring::apply(exterior_ring(polygon), geometry, id, ring_properties);
 
             typename interior_return_type<Polygon const>::type
                 rings = interior_rings(polygon);
@@ -107,18 +114,18 @@ namespace dispatch
                     it = boost::begin(rings); it != boost::end(rings); ++it)
             {
                 id.ring_index++;
-                per_ring::apply(*it, geometry, id, ring_properties, midpoint);
+                per_ring::apply(*it, geometry, id, ring_properties);
             }
         }
 
         template <typename RingPropertyMap>
         static inline void apply(Polygon const& polygon,
-                ring_identifier id, RingPropertyMap& ring_properties, bool midpoint)
+                ring_identifier id, RingPropertyMap& ring_properties)
         {
             typedef typename geometry::ring_type<Polygon>::type ring_type;
             typedef select_rings<ring_tag, ring_type> per_ring;
 
-            per_ring::apply(exterior_ring(polygon), id, ring_properties, midpoint);
+            per_ring::apply(exterior_ring(polygon), id, ring_properties);
 
             typename interior_return_type<Polygon const>::type
                 rings = interior_rings(polygon);
@@ -126,7 +133,7 @@ namespace dispatch
                     it = boost::begin(rings); it != boost::end(rings); ++it)
             {
                 id.ring_index++;
-                per_ring::apply(*it, id, ring_properties, midpoint);
+                per_ring::apply(*it, id, ring_properties);
             }
         }
     };
@@ -136,7 +143,7 @@ namespace dispatch
     {
         template <typename Geometry, typename RingPropertyMap>
         static inline void apply(Multi const& multi, Geometry const& geometry,
-                    ring_identifier id, RingPropertyMap& ring_properties, bool midpoint)
+                    ring_identifier id, RingPropertyMap& ring_properties)
         {
             typedef typename boost::range_iterator
                 <
@@ -149,7 +156,7 @@ namespace dispatch
             for (iterator_type it = boost::begin(multi); it != boost::end(multi); ++it)
             {
                 id.ring_index = -1;
-                per_polygon::apply(*it, geometry, id, ring_properties, midpoint);
+                per_polygon::apply(*it, geometry, id, ring_properties);
                 id.multi_index++;
             }
         }
@@ -165,14 +172,12 @@ struct decide
 template<>
 struct decide<overlay_union>
 {
-    template <typename Properties>
+    static bool include(ring_identifier const& , ring_turn_info const& info)
-    static bool include(ring_identifier const& , Properties const& properties)
     {
-        return properties.within_code * -1 == 1;
+        return info.has_uu_turn || ! info.within_other;
     }
 
-    template <typename Properties>
+    static bool reversed(ring_identifier const& , ring_turn_info const& )
-    static bool reversed(ring_identifier const& , Properties const& )
     {
         return false;
     }
@@ -181,31 +186,29 @@ struct decide<overlay_union>
 template<>
 struct decide<overlay_difference>
 {
-    template <typename Properties>
+    static bool include(ring_identifier const& id, ring_turn_info const& info)
-    static bool include(ring_identifier const& id, Properties const& properties)
     {
-        bool is_first = id.source_index == 0;
+        // u/u turns for difference: one ring lies inside the other
-        return properties.within_code * -1 * (is_first ? 1 : -1) == 1;
+        bool const within_other = info.has_uu_turn || info.within_other;
+        bool const is_second = id.source_index == 1;
+        return is_second ? within_other : ! within_other;
     }
 
-    template <typename Properties>
+    static bool reversed(ring_identifier const& id, ring_turn_info const& info)
-    static bool reversed(ring_identifier const& id, Properties const& properties)
     {
-        return include(id, properties) && id.source_index == 1;
+        return include(id, info) && id.source_index == 1;
     }
 };
 
 template<>
 struct decide<overlay_intersection>
 {
-    template <typename Properties>
+    static bool include(ring_identifier const& , ring_turn_info const& info)
-    static bool include(ring_identifier const& , Properties const& properties)
     {
-        return properties.within_code * 1 == 1;
+        return ! info.has_uu_turn && info.within_other;
     }
 
-    template <typename Properties>
+    static bool reversed(ring_identifier const& , ring_turn_info const& )
-    static bool reversed(ring_identifier const& , Properties const& )
     {
         return false;
     }
@@ -217,12 +220,12 @@ template
     overlay_type OverlayType,
     typename Geometry1,
     typename Geometry2,
-    typename TurnCountMap,
+    typename TurnInfoMap,
     typename RingPropertyMap
 >
 inline void update_ring_selection(Geometry1 const& geometry1,
             Geometry2 const& geometry2,
-            TurnCountMap const& turn_count_map,
+            TurnInfoMap const& turn_info_map,
             RingPropertyMap const& all_ring_properties,
             RingPropertyMap& selected_ring_properties)
 {
@@ -232,33 +235,45 @@ inline void update_ring_selection(Geometry1 const& geometry1,
         it != boost::end(all_ring_properties);
         ++it)
     {
-        typename TurnCountMap::const_iterator tcit = turn_count_map.find(it->first);
+        ring_identifier const& id = it->first;
-        if (tcit == turn_count_map.end())
-        {
-            ring_identifier const id = it->first;
-            typename RingPropertyMap::mapped_type properties = it->second; // Copy by value
 
-            // Calculate the "within code" (previously this was done earlier but is
+        ring_turn_info info;
-            // much efficienter here - it can be even more efficient doing it all at once,
+
-            // using partition, TODO)
+        typename TurnInfoMap::const_iterator tcit = turn_info_map.find(id);
-            // So though this is less elegant than before, it avoids many unused point-in-poly calculations
+        if (tcit != turn_info_map.end())
+        {
+            info = tcit->second; // Copy by value
+        }
+
+        if (info.has_normal_turn)
+        {
+            // There are normal turns on this ring. It should be traversed, we
+            // don't include the original ring
+            continue;
+        }
+
+        if (! info.has_uu_turn)
+        {
+            // There are no turns on this ring
+
+            // Check if the ring is within the other geometry, by taking
+            // a point lying on the ring
             switch(id.source_index)
             {
                 case 0 :
-                    properties.within_code
+                    info.within_other = geometry::within(it->second.point, geometry2);
-                        = geometry::within(properties.point, geometry2) ? 1 : -1;
                     break;
                 case 1 :
-                    properties.within_code
+                    info.within_other = geometry::within(it->second.point, geometry1);
-                        = geometry::within(properties.point, geometry1) ? 1 : -1;
                     break;
             }
-
-            if (decide<OverlayType>::include(id, properties))
-            {
-                properties.reversed = decide<OverlayType>::reversed(id, properties);
-                selected_ring_properties[id] = properties;
         }
+
+        if (decide<OverlayType>::include(id, info))
+        {
+            typename RingPropertyMap::mapped_type properties = it->second; // Copy by value
+            properties.reversed = decide<OverlayType>::reversed(id, info);
+            selected_ring_properties[id] = properties;
         }
     }
 }
@@ -272,23 +287,23 @@ template
     overlay_type OverlayType,
     typename Geometry1,
     typename Geometry2,
-    typename TurnCountMap,
+    typename RingTurnInfoMap,
     typename RingPropertyMap
 >
 inline void select_rings(Geometry1 const& geometry1, Geometry2 const& geometry2,
-            TurnCountMap const& turn_count_per_ring,
+            RingTurnInfoMap const& turn_info_per_ring,
-            RingPropertyMap& selected_ring_properties, bool midpoint)
+            RingPropertyMap& selected_ring_properties)
 {
     typedef typename geometry::tag<Geometry1>::type tag1;
     typedef typename geometry::tag<Geometry2>::type tag2;
 
     RingPropertyMap all_ring_properties;
     dispatch::select_rings<tag1, Geometry1>::apply(geometry1, geometry2,
-                ring_identifier(0, -1, -1), all_ring_properties, midpoint);
+                ring_identifier(0, -1, -1), all_ring_properties);
     dispatch::select_rings<tag2, Geometry2>::apply(geometry2, geometry1,
-                ring_identifier(1, -1, -1), all_ring_properties, midpoint);
+                ring_identifier(1, -1, -1), all_ring_properties);
 
-    update_ring_selection<OverlayType>(geometry1, geometry2, turn_count_per_ring,
+    update_ring_selection<OverlayType>(geometry1, geometry2, turn_info_per_ring,
                 all_ring_properties, selected_ring_properties);
 }
 
@@ -296,20 +311,20 @@ template
 <
     overlay_type OverlayType,
     typename Geometry,
-    typename TurnCountMap,
+    typename RingTurnInfoMap,
     typename RingPropertyMap
 >
 inline void select_rings(Geometry const& geometry,
-            TurnCountMap const& turn_count_per_ring,
+            RingTurnInfoMap const& turn_info_per_ring,
-            RingPropertyMap& selected_ring_properties, bool midpoint)
+            RingPropertyMap& selected_ring_properties)
 {
     typedef typename geometry::tag<Geometry>::type tag;
 
     RingPropertyMap all_ring_properties;
     dispatch::select_rings<tag, Geometry>::apply(geometry,
-                ring_identifier(0, -1, -1), all_ring_properties, midpoint);
+                ring_identifier(0, -1, -1), all_ring_properties);
 
-    update_ring_selection<OverlayType>(geometry, geometry, turn_count_per_ring,
+    update_ring_selection<OverlayType>(geometry, geometry, turn_info_per_ring,
                 all_ring_properties, selected_ring_properties);
 }
 

include/boost/geometry/extensions/algorithms/dissolve.hpp

@@ -161,14 +161,14 @@ struct dissolve_ring_or_polygon
                             rescale_policy,
                             turns, rings);
 
-            std::map<ring_identifier, int> map;
+            std::map<ring_identifier, detail::overlay::ring_turn_info> map;
-            map_turns(map, turns);
+            get_ring_turn_info(map, turns);
 
             typedef detail::overlay::ring_properties<typename geometry::point_type<Geometry>::type> properties;
 
             std::map<ring_identifier, properties> selected;
 
-            detail::overlay::select_rings<overlay_union>(geometry, map, selected, true);
+            detail::overlay::select_rings<overlay_union>(geometry, map, selected);
 
             // Add intersected rings
             {
@@ -178,7 +178,7 @@ struct dissolve_ring_or_polygon
                         it != boost::end(rings);
                         ++it)
                 {
-                    selected[id] = properties(*it, true);
+                    selected[id] = properties(*it);
                     id.multi_index++;
                 }
             }

test/algorithms/overlay/overlay_cases.hpp

@@ -795,6 +795,17 @@ static std::string ticket_9563[2] =
         "POLYGON((16.4030230586814020 21.414709848078967, 17.7828326880709360 18.936596729241124, 17.3451877762964410 14.867777311013885, 15.2073549240394820 10.298488470659295, 11.9421647176150340 6.4530674629349125, 8.4245222359685457 4.3618917414181650, 5.5969769413185944 4.5852901519210292, 4.2171673119290620 7.0634032707588670, 4.6548122237035567 11.132222688986115, 6.7926450759605128 15.701511529340699, 10.0578352823849570 19.546932537065082, 13.5754777640314510 21.638108258581831, 16.4030230586814020 21.414709848078967))"
     };
 
+static std::string ticket_10108_a[2] =
+    {
+        "POLYGON((-0.85012528418186883439 0.66468648958045217778 , -1.0190633474909247536 0.58375169123203618504 , -0.81735787096893253167 0.85208889314502478385 , -0.85012528418186883439 0.66468648958045217778))",
+        "POLYGON((-1.0898104946524889147 1.0651111163194444398 , -1.0543813205484939832 0.82438792455048248708 , -0.81735787088719669136 0.8520888930811181261 , -1.0898104946524889147 1.0651111163194444398))"
+    };
+static std::string ticket_10108_b[2] =
+    {
+        "POLYGON((818.61020100991334 686.40744987236633, 818.94520828641623 714.37814489343316, 857.67308553468195 713.83138513092547, 857.33807828316174 685.94987141847253, 818.61020100991334 686.40744987236633))",
+        "POLYGON((857.33807828316174 685.94987141847253, 857.64395373587263 711.40684463430682, 910.49336536223325 710.67130033421233, 910.18750000000000 685.32544378698219, 857.33807828316174 685.94987141847253))"
+    };
+
 // Rescaling problem (fixed in 1.58)
 static std::string ticket_10747_a[2] =
     {

test/algorithms/set_operations/difference/difference.cpp

@@ -374,6 +374,15 @@ void test_all()
             6, 24, 20.096189);
 #endif
 
+    test_one<polygon, polygon, polygon>("ticket_10108_a",
+            ticket_10108_a[0], ticket_10108_a[1],
+            1, 4,  0.0145037,
+            1, 4,  0.029019232);
+    test_one<polygon, polygon, polygon>("ticket_10108_b",
+            ticket_10108_b[0], ticket_10108_b[1],
+            1, 5, 1081.68697,
+            1, 5, 1342.65795);
+
     // Other combi's
     {
         test_one<polygon, polygon, ring>(

test/algorithms/set_operations/intersection/intersection.cpp

@@ -253,6 +253,13 @@ void test_areal()
                 ticket_9081_15[0], ticket_9081_15[1],
                 1, 4, 0.0068895780745301394);
 
+    test_one<Polygon, Polygon, Polygon>("ticket_10108_a",
+                ticket_10108_a[0], ticket_10108_a[1],
+                0, 0, 0.0);
+    test_one<Polygon, Polygon, Polygon>("ticket_10108_b",
+                ticket_10108_b[0], ticket_10108_b[1],
+                0, 0, 0.0);
+
     test_one<Polygon, Polygon, Polygon>("ticket_10747_a",
                 ticket_10747_a[0], ticket_10747_a[1],
                 1, 4, 70368744177664);

test/algorithms/set_operations/union/multi_union.cpp

@@ -78,6 +78,9 @@ void test_areal()
     test_one<Polygon, MultiPolygon, MultiPolygon>("case_66_multi",
         case_66_multi[0], case_66_multi[1],
         3, 0, 23, 7.0);
+    test_one<Polygon, MultiPolygon, MultiPolygon>("case_72_multi",
+        case_72_multi[0], case_72_multi[1],
+        1, 0, 13, 10.65);
     test_one<Polygon, MultiPolygon, MultiPolygon>("case_75_multi",
         case_75_multi[0], case_75_multi[1],
         5, 0, 25, 5.0);

test/algorithms/set_operations/union/union.cpp

@@ -282,6 +282,11 @@ void test_areal()
     test_one<Polygon, Polygon, Polygon>("ticket_9756", ticket_9756[0], ticket_9756[1],
             1, 0, 10, 1289.08374);
 
+    test_one<Polygon, Polygon, Polygon>("ticket_10108_a", ticket_10108_a[0], ticket_10108_a[1],
+            2, 0, 8, 0.0435229);
+    test_one<Polygon, Polygon, Polygon>("ticket_10108_b", ticket_10108_b[0], ticket_10108_b[1],
+            2, 0, 10, 2424.3449);
+
     test_one<Polygon, Polygon, Polygon>("geos_1", geos_1[0], geos_1[1],
             1, 0, -1, 3461.3203125);
     test_one<Polygon, Polygon, Polygon>("geos_2", geos_2[0], geos_2[1],