[correct] Split off correct_closure from correct

Including unit test

include/boost/geometry/algorithms/correct.hpp

@@ -32,6 +32,7 @@
 #include <boost/variant/static_visitor.hpp>
 #include <boost/variant/variant_fwd.hpp>
 
+#include <boost/geometry/algorithms/correct_closure.hpp>
 #include <boost/geometry/algorithms/detail/interior_iterator.hpp>
 
 #include <boost/geometry/core/closure.hpp>
@@ -45,7 +46,6 @@
 #include <boost/geometry/geometries/concepts/check.hpp>
 
 #include <boost/geometry/algorithms/area.hpp>
-#include <boost/geometry/algorithms/disjoint.hpp>
 #include <boost/geometry/algorithms/detail/multi_modify.hpp>
 #include <boost/geometry/util/order_as_direction.hpp>
 
@@ -140,23 +140,9 @@ struct correct_ring
     template <typename Strategy>
     static inline void apply(Ring& r, Strategy const& strategy)
     {
-        // Check close-ness
-        if (boost::size(r) > 2)
-        {
-            // check if closed, if not, close it
-            bool const disjoint = geometry::disjoint(*boost::begin(r), *(boost::end(r) - 1));
-            closure_selector const s = geometry::closure<Ring>::value;
+        // Correct closure if necessary
+        detail::correct_closure::close_or_open_ring<Ring>::apply(r);
 
-            if (disjoint && (s == closed))
-            {
-                geometry::append(r, *boost::begin(r));
-            }
-            if (! disjoint && s != closed)
-            {
-                // Open it by removing last point
-                geometry::traits::resize<Ring>::apply(r, boost::size(r) - 1);
-            }
-        }
         // Check area
         typedef typename Strategy::return_type area_result_type;
         Predicate<area_result_type> predicate;

include/boost/geometry/algorithms/correct_closure.hpp

@@ -0,0 +1,235 @@
+// Boost.Geometry
+
+// Copyright (c) 2017 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_ALGORITHMS_CORRECT_CLOSURE_HPP
+#define BOOST_GEOMETRY_ALGORITHMS_CORRECT_CLOSURE_HPP
+
+#include <cstddef>
+
+#include <boost/range.hpp>
+
+#include <boost/variant/apply_visitor.hpp>
+#include <boost/variant/static_visitor.hpp>
+#include <boost/variant/variant_fwd.hpp>
+
+#include <boost/geometry/algorithms/detail/interior_iterator.hpp>
+
+#include <boost/geometry/core/closure.hpp>
+#include <boost/geometry/core/exterior_ring.hpp>
+#include <boost/geometry/core/interior_rings.hpp>
+#include <boost/geometry/core/ring_type.hpp>
+#include <boost/geometry/core/tags.hpp>
+
+#include <boost/geometry/geometries/concepts/check.hpp>
+
+#include <boost/geometry/algorithms/disjoint.hpp>
+#include <boost/geometry/algorithms/detail/multi_modify.hpp>
+
+namespace boost { namespace geometry
+{
+
+// Silence warning C4127: conditional expression is constant
+#if defined(_MSC_VER)
+#pragma warning(push)
+#pragma warning(disable : 4127)
+#endif
+
+#ifndef DOXYGEN_NO_DETAIL
+namespace detail { namespace correct_closure
+{
+
+template <typename Geometry>
+struct nop
+{
+    static inline void apply(Geometry& )
+    {}
+};
+
+
+// Close a ring, if not closed, or open it
+template <typename Ring>
+struct close_or_open_ring
+{
+    static inline void apply(Ring& r)
+    {
+        if (boost::size(r) <= 2)
+        {
+            return;
+        }
+
+        bool const disjoint = geometry::disjoint(*boost::begin(r),
+                                                 *(boost::end(r) - 1));
+        closure_selector const s = geometry::closure<Ring>::value;
+
+        if (disjoint && s == closed)
+        {
+            // Close it by adding first point
+            geometry::append(r, *boost::begin(r));
+        }
+        else if (! disjoint && s != closed)
+        {
+            // Open it by removing last point
+            geometry::traits::resize<Ring>::apply(r, boost::size(r) - 1);
+        }
+    }
+};
+
+// Close/open exterior ring and all its interior rings
+template <typename Polygon>
+struct close_or_open_polygon
+{
+    typedef typename ring_type<Polygon>::type ring_type;
+
+    static inline void apply(Polygon& poly)
+    {
+        close_or_open_ring<ring_type>::apply(exterior_ring(poly));
+
+        typename interior_return_type<Polygon>::type
+            rings = interior_rings(poly);
+
+        for (typename detail::interior_iterator<Polygon>::type
+                it = boost::begin(rings); it != boost::end(rings); ++it)
+        {
+            close_or_open_ring<ring_type>::apply(*it);
+        }
+    }
+};
+
+}} // namespace detail::correct_closure
+#endif // DOXYGEN_NO_DETAIL
+
+
+#ifndef DOXYGEN_NO_DISPATCH
+namespace dispatch
+{
+
+template <typename Geometry, typename Tag = typename tag<Geometry>::type>
+struct correct_closure: not_implemented<Tag>
+{};
+
+template <typename Point>
+struct correct_closure<Point, point_tag>
+    : detail::correct_closure::nop<Point>
+{};
+
+template <typename LineString>
+struct correct_closure<LineString, linestring_tag>
+    : detail::correct_closure::nop<LineString>
+{};
+
+template <typename Segment>
+struct correct_closure<Segment, segment_tag>
+    : detail::correct_closure::nop<Segment>
+{};
+
+
+template <typename Box>
+struct correct_closure<Box, box_tag>
+    : detail::correct_closure::nop<Box>
+{};
+
+template <typename Ring>
+struct correct_closure<Ring, ring_tag>
+    : detail::correct_closure::close_or_open_ring<Ring>
+{};
+
+template <typename Polygon>
+struct correct_closure<Polygon, polygon_tag>
+    : detail::correct_closure::close_or_open_polygon<Polygon>
+{};
+
+
+template <typename MultiPoint>
+struct correct_closure<MultiPoint, multi_point_tag>
+    : detail::correct_closure::nop<MultiPoint>
+{};
+
+
+template <typename MultiLineString>
+struct correct_closure<MultiLineString, multi_linestring_tag>
+    : detail::correct_closure::nop<MultiLineString>
+{};
+
+
+template <typename Geometry>
+struct correct_closure<Geometry, multi_polygon_tag>
+    : detail::multi_modify
+        <
+            Geometry,
+            detail::correct_closure::close_or_open_polygon
+                <
+                    typename boost::range_value<Geometry>::type
+                >
+        >
+{};
+
+
+} // namespace dispatch
+#endif // DOXYGEN_NO_DISPATCH
+
+
+namespace resolve_variant
+{
+
+template <typename Geometry>
+struct correct_closure
+{
+    static inline void apply(Geometry& geometry)
+    {
+        concepts::check<Geometry const>();
+        dispatch::correct_closure<Geometry>::apply(geometry);
+    }
+};
+
+template <BOOST_VARIANT_ENUM_PARAMS(typename T)>
+struct correct_closure<boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)> >
+{
+    struct visitor: boost::static_visitor<void>
+    {
+        template <typename Geometry>
+        void operator()(Geometry& geometry) const
+        {
+            correct_closure<Geometry>::apply(geometry);
+        }
+    };
+
+    static inline void
+    apply(boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)>& geometry)
+    {
+        visitor vis;
+        boost::apply_visitor(vis, geometry);
+    }
+};
+
+} // namespace resolve_variant
+
+
+/*!
+\brief Closes or opens a geometry, according to its type
+\details Corrects a geometry w.r.t. closure points to all rings which do not
+    have a closing point and are typed as they should have one, the first point
+    is appended.
+\ingroup correct_closure
+\tparam Geometry \tparam_geometry
+\param geometry \param_geometry which will be corrected if necessary
+*/
+template <typename Geometry>
+inline void correct_closure(Geometry& geometry)
+{
+    resolve_variant::correct_closure<Geometry>::apply(geometry);
+}
+
+
+#if defined(_MSC_VER)
+#pragma warning(pop)
+#endif
+
+}} // namespace boost::geometry
+
+
+#endif // BOOST_GEOMETRY_ALGORITHMS_CORRECT_CLOSURE_HPP

include/boost/geometry/io/wkt/write.hpp

@@ -254,7 +254,8 @@ struct wkt_box
     static inline void apply(std::basic_ostream<Char, Traits>& os,
                 Box const& box, bool force_closure)
     {
-        // Convert to open cw ring, then stream
+        // Convert to open cw ring, then stream. In this case the box can be
+        // specified as open and close (by force_closure)
         typedef model::ring<point_type, true, false> ring_type;
         ring_type ring;
         geometry::convert(box, ring);

test/algorithms/Jamfile.v2

@@ -28,6 +28,7 @@ test-suite boost-geometry-algorithms
     [ run convex_hull_multi.cpp        : : : : algorithms_convex_hull_multi ]
     [ run correct.cpp                  : : : : algorithms_correct ]
     [ run correct_multi.cpp            : : : : algorithms_correct_multi ]
+    [ run correct_closure.cpp          : : : : algorithms_correct_closure ]
     [ run for_each.cpp                 : : : : algorithms_for_each ]
     [ run for_each_multi.cpp           : : : : algorithms_for_each_multi ]
     [ run is_convex.cpp                : : : : algorithms_is_convex ]

test/algorithms/correct.cpp

@@ -136,8 +136,9 @@ void test_ring_polygon()
 template <typename P>
 void test_box()
 {
-    // Boxes
-    std::string proper_box = "POLYGON((0 0,0 2,2 2,2 0,0 0))";
+    // Boxes. Reference is an open box (because in this test WKT is not
+    // explicitly closed)
+    std::string proper_box = "POLYGON((0 0,0 2,2 2,2 0))";
     test_geometry<bg::model::box<P> >(proper_box, proper_box);
     test_geometry<bg::model::box<P> >("BOX(0 0,2 2)", proper_box);
     test_geometry<bg::model::box<P> >("BOX(2 2,0 0)", proper_box);

test/algorithms/correct_closure.cpp

@@ -0,0 +1,114 @@
+// Boost.Geometry
+// Unit Test
+
+// Copyright (c) 2017 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 <sstream>
+
+#include <geometry_test_common.hpp>
+
+#include <boost/geometry/strategies/strategies.hpp>
+
+#include <boost/geometry/geometries/point_xy.hpp>
+#include <boost/geometry/geometries/ring.hpp>
+#include <boost/geometry/geometries/polygon.hpp>
+
+#include <boost/geometry/algorithms/correct_closure.hpp>
+#include <boost/geometry/io/wkt/read.hpp>
+#include <boost/geometry/io/wkt/write.hpp>
+
+#include <boost/variant/variant.hpp>
+
+
+template <typename BaseGeometry, typename Geometry>
+void check_geometry(Geometry const& geometry, std::string const& expected)
+{
+    std::ostringstream out;
+    out << bg::wkt_manipulator<Geometry>(geometry, false);
+    BOOST_CHECK_EQUAL(out.str(), expected);
+}
+
+template <typename Geometry>
+void test_geometry(std::string const& wkt, std::string const& expected)
+{
+    Geometry geometry;
+    bg::read_wkt(wkt, geometry);
+
+    // Test tye type
+    bg::correct_closure(geometry);
+    check_geometry<Geometry>(geometry, expected);
+
+    // Test varianted type
+    boost::variant<Geometry> v(geometry);
+    bg::correct_closure(v);
+    check_geometry<Geometry>(v, expected);
+}
+
+template <typename P>
+void test_all()
+{
+    typedef bg::model::ring<P, true, true> cw_closed_ring_type;
+    typedef bg::model::ring<P, true, false> cw_open_ring_type;
+    typedef bg::model::ring<P, false, true> ccw_closed_ring_type;
+    typedef bg::model::ring<P, false, false> ccw_open_ring_type;
+
+    // Define clockwise and counter clockwise polygon
+    std::string cw_ring       = "POLYGON((0 0,0 1,1 1,1 0,0 0))";
+    std::string cw_open_ring  = "POLYGON((0 0,0 1,1 1,1 0))";
+
+    std::string ccw_ring      = "POLYGON((0 0,1 0,1 1,0 1,0 0))";
+    std::string ccw_open_ring = "POLYGON((0 0,1 0,1 1,0 1))";
+
+    // Cases which should be closed or opened
+    test_geometry<cw_closed_ring_type>(cw_open_ring, cw_ring);
+    test_geometry<cw_open_ring_type>(cw_ring, cw_open_ring);
+    test_geometry<ccw_closed_ring_type>(ccw_open_ring, ccw_ring);
+    test_geometry<ccw_open_ring_type>(ccw_ring, ccw_open_ring);
+
+    // Cases which are incorrect but should still be closed or opened
+    test_geometry<cw_closed_ring_type>(ccw_open_ring, ccw_ring);
+    test_geometry<ccw_open_ring_type>(cw_ring, cw_open_ring);
+
+    // Cases where no action is necessary (even if order is incorrect)
+    test_geometry<cw_closed_ring_type>(cw_ring, cw_ring);
+    test_geometry<cw_closed_ring_type>(ccw_ring, ccw_ring);
+    test_geometry<cw_open_ring_type>(cw_open_ring, cw_open_ring);
+    test_geometry<cw_open_ring_type>(ccw_open_ring, ccw_open_ring);
+    test_geometry<ccw_closed_ring_type>(cw_ring, cw_ring);
+    test_geometry<ccw_closed_ring_type>(ccw_ring, ccw_ring);
+    test_geometry<ccw_open_ring_type>(cw_open_ring, cw_open_ring);
+    test_geometry<ccw_open_ring_type>(ccw_open_ring, ccw_open_ring);
+
+    // Polygon cases
+    std::string cw_polygon =
+            "POLYGON((0 0,0 4,4 4,4 0,0 0),(1 1,2 1,2 2,1 2,1 1))";
+
+    std::string cw_open_polygon =
+            "POLYGON((0 0,0 4,4 4,4 0),(1 1,2 1,2 2,1 2))";
+
+    typedef bg::model::polygon<P, true, true> cw_closed_polygon_type;
+    typedef bg::model::polygon<P, true, false> cw_open_polygon_type;
+
+    test_geometry<cw_closed_polygon_type>(cw_open_polygon, cw_polygon);
+    test_geometry<cw_open_polygon_type>(cw_polygon, cw_open_polygon);
+
+    test_geometry<cw_closed_polygon_type>(cw_polygon, cw_polygon);
+    test_geometry<cw_open_polygon_type>(cw_open_polygon, cw_open_polygon);
+}
+
+
+int test_main(int, char* [])
+{
+    test_all<bg::model::d2::point_xy<int> >();
+    test_all<bg::model::d2::point_xy<float> >();
+    test_all<bg::model::d2::point_xy<double> >();
+
+    test_all<bg::model::point<double, 2, bg::cs::spherical_equatorial<bg::degree> > >();
+    test_all<bg::model::point<double, 2, bg::cs::geographic<bg::degree> > >();
+
+    return 0;
+}

test/algorithms/test_correct.hpp

@@ -27,12 +27,11 @@
 #include <boost/geometry/io/wkt/write.hpp>
 #include <boost/variant/variant.hpp>
 
-
 template <typename Geometry>
 void check_geometry(Geometry const& geometry, std::string const& expected)
 {
     std::ostringstream out;
-    out << bg::wkt(geometry);
+    out << bg::wkt_manipulator<Geometry>(geometry, false);
 
     BOOST_CHECK_EQUAL(out.str(), expected);
 }