Merge pull request #3164 from mapycz/postgis-empty-geoms

check input data of centroid algorithm

include/mapnik/geometry_centroid.hpp

@@ -26,6 +26,7 @@
 #include <mapnik/geometry.hpp>
 #include <mapnik/geometry_adapters.hpp>
 #include <boost/geometry/algorithms/centroid.hpp>
+#include <mapnik/geometry_is_empty.hpp>
 
 namespace mapnik { namespace geometry {
 
@@ -63,30 +64,50 @@ struct geometry_centroid
 
     result_type operator() (line_string<T> const& geom) const
     {
+        if (mapnik::geometry::is_empty(geom))
+        {
+            return false;
+        }
         boost::geometry::centroid(geom, pt_);
         return true;
     }
 
     result_type operator() (polygon<T> const& geom) const
     {
+        if (mapnik::geometry::is_empty(geom))
+        {
+            return false;
+        }
         boost::geometry::centroid(geom, pt_);
         return true;
     }
 
     result_type operator() (multi_point<T> const& geom) const
     {
+        if (mapnik::geometry::is_empty(geom))
+        {
+            return false;
+        }
         boost::geometry::centroid(geom, pt_);
         return true;
     }
 
     result_type operator() (multi_line_string<T> const& geom) const
     {
+        if (mapnik::geometry::is_empty(geom) || mapnik::geometry::has_empty(geom))
+        {
+            return false;
+        }
         boost::geometry::centroid(geom, pt_);
         return true;
     }
 
     result_type operator() (multi_polygon<T> const& geom) const
     {
+        if (mapnik::geometry::is_empty(geom) || mapnik::geometry::has_empty(geom))
+        {
+            return false;
+        }
         boost::geometry::centroid(geom, pt_);
         return true;
     }

include/mapnik/geometry_is_empty.hpp

@@ -79,6 +79,81 @@ struct geometry_is_empty
 
 };
 
+struct geometry_has_empty
+{
+    bool operator() (mapnik::geometry::geometry<double> const& geom) const
+    {
+        return mapnik::util::apply_visitor(*this, geom);
+    }
+
+    bool operator() (mapnik::geometry::geometry_empty const&) const
+    {
+        return false;
+    }
+
+    bool operator() (mapnik::geometry::point<double> const&) const
+    {
+        return false;
+    }
+
+    bool operator() (mapnik::geometry::line_string<double> const& geom) const
+    {
+        return false;
+    }
+
+    bool operator() (mapnik::geometry::polygon<double> const& geom) const
+    {
+        return false;
+    }
+
+    bool operator() (mapnik::geometry::multi_point<double> const& geom) const
+    {
+        return false;
+    }
+
+    bool operator() (mapnik::geometry::multi_line_string<double> const& geom) const
+    {
+        return test_multigeometry(geom);
+    }
+
+    bool operator() (mapnik::geometry::multi_polygon<double> const& geom) const
+    {
+        return test_multigeometry(geom);
+    }
+
+    bool operator() (mapnik::geometry::geometry_collection<double> const& geom) const
+    {
+        for (auto const & item : geom)
+        {
+            if (geometry_is_empty()(item) || (*this)(item))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    template <typename T>
+    bool operator() (T const& geom) const
+    {
+        return true;
+    }
+
+private:
+    template <typename T>
+    bool test_multigeometry(T const & geom) const
+    {
+        for (auto const & item : geom)
+        {
+            if (item.empty())
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+};
+
 }
 
 template <typename GeomType>
@@ -87,6 +162,12 @@ inline bool is_empty(GeomType const& geom)
     return detail::geometry_is_empty()(geom);
 }
 
+template <typename GeomType>
+inline bool has_empty(GeomType const& geom)
+{
+    return detail::geometry_has_empty()(geom);
+}
+
 }}
 
 #endif // MAPNIK_GEOMETRY_IS_EMPTY_HPP

test/unit/geometry/centroid.cpp

@@ -0,0 +1,201 @@
+#include "catch.hpp"
+
+#include <mapnik/geometry_centroid.hpp>
+
+TEST_CASE("geometry centroid") {
+
+SECTION("empty geometry") {
+
+    mapnik::geometry::geometry_empty geom;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("geometry collection") {
+
+    mapnik::geometry::geometry_collection<double> geom;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("point") {
+
+    mapnik::geometry::point<double> pt(10, 10);
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(pt, centroid));
+    REQUIRE(pt.x == centroid.x);
+    REQUIRE(pt.y == centroid.y);
+}
+
+SECTION("linestring") {
+
+    mapnik::geometry::line_string<double> line;
+    line.add_coord(0, 0);
+    line.add_coord(25, 25);
+    line.add_coord(50, 50);
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(line, centroid));
+    REQUIRE(centroid.x == 25);
+    REQUIRE(centroid.y == 25);
+}
+
+SECTION("empty linestring") {
+
+    mapnik::geometry::line_string<double> line;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(line, centroid));
+}
+
+SECTION("polygon") {
+
+    mapnik::geometry::polygon<double> poly;
+    mapnik::geometry::linear_ring<double> ring;
+    ring.add_coord(0, 0);
+    ring.add_coord(1, 0);
+    ring.add_coord(1, 1);
+    ring.add_coord(0, 1);
+    ring.add_coord(0, 0);
+    poly.set_exterior_ring(std::move(ring));
+
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(poly, centroid));
+    REQUIRE(centroid.x == 0.5);
+    REQUIRE(centroid.y == 0.5);
+}
+
+SECTION("polygon with empty exterior ring") {
+
+    mapnik::geometry::polygon<double> poly;
+    mapnik::geometry::linear_ring<double> ring;
+    poly.set_exterior_ring(std::move(ring));
+
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(poly, centroid));
+}
+
+SECTION("empty polygon") {
+
+    mapnik::geometry::polygon<double> poly;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(poly, centroid));
+}
+
+SECTION("multi-point") {
+
+    mapnik::geometry::multi_point<double> geom;
+    geom.add_coord(0, 0);
+    geom.add_coord(25, 25);
+    geom.add_coord(50, 50);
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(geom, centroid));
+    REQUIRE(centroid.x == 25);
+    REQUIRE(centroid.y == 25);
+}
+
+SECTION("empty multi-point") {
+
+    mapnik::geometry::multi_point<double> geom;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("multi-linestring") {
+
+    mapnik::geometry::multi_line_string<double> geom;
+    {
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        line.add_coord(0, 25);
+        line.add_coord(0, 50);
+        geom.emplace_back(std::move(line));
+    }
+    {
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        line.add_coord(25, 0);
+        line.add_coord(50, 0);
+        geom.emplace_back(std::move(line));
+    }
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(geom, centroid));
+    REQUIRE(centroid.x == 12.5);
+    REQUIRE(centroid.y == 12.5);
+}
+
+SECTION("multi-linestring: one component empty") {
+
+    mapnik::geometry::multi_line_string<double> geom;
+    mapnik::geometry::line_string<double> line;
+    line.add_coord(0, 0);
+    line.add_coord(0, 25);
+    line.add_coord(0, 50);
+    geom.emplace_back(std::move(line));
+    geom.emplace_back();
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("empty multi-linestring") {
+
+    mapnik::geometry::multi_line_string<double> geom;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("multi-polygon") {
+
+    mapnik::geometry::multi_polygon<double> geom;
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        ring.add_coord(0, 0);
+        ring.add_coord(1, 0);
+        ring.add_coord(1, 1);
+        ring.add_coord(0, 1);
+        ring.add_coord(0, 0);
+        poly.set_exterior_ring(std::move(ring));
+        geom.emplace_back(std::move(poly));
+    }
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        ring.add_coord(1, 1);
+        ring.add_coord(2, 1);
+        ring.add_coord(2, 2);
+        ring.add_coord(1, 2);
+        ring.add_coord(1, 1);
+        poly.set_exterior_ring(std::move(ring));
+        geom.emplace_back(std::move(poly));
+    }
+
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(mapnik::geometry::centroid(geom, centroid));
+    REQUIRE(centroid.x == 1);
+    REQUIRE(centroid.y == 1);
+}
+
+SECTION("multi-polygon: one component empty") {
+
+    mapnik::geometry::multi_polygon<double> geom;
+    mapnik::geometry::polygon<double> poly;
+    mapnik::geometry::linear_ring<double> ring;
+    ring.add_coord(0, 0);
+    ring.add_coord(1, 0);
+    ring.add_coord(1, 1);
+    ring.add_coord(0, 1);
+    ring.add_coord(0, 0);
+    poly.set_exterior_ring(std::move(ring));
+    geom.emplace_back(std::move(poly));
+    geom.emplace_back();
+
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+
+SECTION("empty multi-polygon") {
+
+    mapnik::geometry::multi_polygon<double> geom;
+    mapnik::geometry::point<double> centroid;
+    REQUIRE(!mapnik::geometry::centroid(geom, centroid));
+}
+}

test/unit/geometry/has_empty.cpp

@@ -0,0 +1,150 @@
+#include "catch.hpp"
+
+#include <mapnik/geometry_is_empty.hpp>
+
+TEST_CASE("geometry has_empty") {
+
+SECTION("empty geometry") {
+
+    mapnik::geometry::geometry_empty geom;
+    REQUIRE(!mapnik::geometry::has_empty(geom));
+}
+
+SECTION("geometry collection") {
+
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        mapnik::geometry::geometry_empty geom1;
+        geom.emplace_back(std::move(geom1));
+        REQUIRE(mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        mapnik::geometry::multi_line_string<double> mls;
+        mapnik::geometry::line_string<double> line;
+        mls.emplace_back(std::move(line));
+        geom.emplace_back(std::move(mls));
+        REQUIRE(mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        mapnik::geometry::multi_line_string<double> mls;
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        mls.emplace_back(std::move(line));
+        geom.emplace_back(std::move(mls));
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+}
+
+SECTION("point") {
+
+    mapnik::geometry::point<double> pt(10, 10);
+    REQUIRE(!mapnik::geometry::has_empty(pt));
+}
+
+SECTION("linestring") {
+
+    {
+        mapnik::geometry::line_string<double> line;
+        REQUIRE(!mapnik::geometry::has_empty(line));
+    }
+    {
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        line.add_coord(25, 25);
+        line.add_coord(50, 50);
+        REQUIRE(!mapnik::geometry::has_empty(line));
+    }
+}
+
+SECTION("polygon") {
+
+    {
+        mapnik::geometry::polygon<double> poly;
+        REQUIRE(!mapnik::geometry::has_empty(poly));
+    }
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        poly.set_exterior_ring(std::move(ring));
+        REQUIRE(!mapnik::geometry::has_empty(poly));
+    }
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        ring.add_coord(0, 0);
+        ring.add_coord(1, 0);
+        ring.add_coord(1, 1);
+        ring.add_coord(0, 1);
+        ring.add_coord(0, 0);
+        poly.set_exterior_ring(std::move(ring));
+        REQUIRE(!mapnik::geometry::has_empty(poly));
+    }
+}
+
+SECTION("multi-point") {
+
+    {
+        mapnik::geometry::multi_point<double> geom;
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_point<double> geom;
+        geom.add_coord(0, 0);
+        geom.add_coord(25, 25);
+        geom.add_coord(50, 50);
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+}
+
+SECTION("multi-linestring") {
+
+    {
+        mapnik::geometry::multi_line_string<double> geom;
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_line_string<double> geom;
+        mapnik::geometry::line_string<double> line;
+        geom.emplace_back(std::move(line));
+        REQUIRE(mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_line_string<double> geom;
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        geom.emplace_back(std::move(line));
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+}
+
+SECTION("multi-polygon") {
+
+    {
+        mapnik::geometry::multi_polygon<double> geom;
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_polygon<double> geom;
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        poly.set_exterior_ring(std::move(ring));
+        geom.emplace_back(std::move(poly));
+        REQUIRE(mapnik::geometry::has_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_polygon<double> geom;
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        ring.add_coord(0, 0);
+        poly.set_exterior_ring(std::move(ring));
+        geom.emplace_back(std::move(poly));
+        REQUIRE(!mapnik::geometry::has_empty(geom));
+    }
+}
+}

test/unit/geometry/is_empty.cpp

@@ -0,0 +1,117 @@
+#include "catch.hpp"
+
+#include <mapnik/geometry_is_empty.hpp>
+
+TEST_CASE("geometry is_empty") {
+
+SECTION("empty geometry") {
+
+    mapnik::geometry::geometry_empty geom;
+    REQUIRE(mapnik::geometry::is_empty(geom));
+}
+
+SECTION("geometry collection") {
+
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        REQUIRE(mapnik::geometry::is_empty(geom));
+    }
+    {
+        mapnik::geometry::geometry_collection<double> geom;
+        mapnik::geometry::geometry_empty geom1;
+        geom.emplace_back(std::move(geom1));
+        REQUIRE(!mapnik::geometry::is_empty(geom));
+    }
+}
+
+SECTION("point") {
+
+    mapnik::geometry::point<double> pt(10, 10);
+    REQUIRE(!mapnik::geometry::is_empty(pt));
+}
+
+SECTION("linestring") {
+
+    {
+        mapnik::geometry::line_string<double> line;
+        REQUIRE(mapnik::geometry::is_empty(line));
+    }
+    {
+        mapnik::geometry::line_string<double> line;
+        line.add_coord(0, 0);
+        line.add_coord(25, 25);
+        line.add_coord(50, 50);
+        REQUIRE(!mapnik::geometry::is_empty(line));
+    }
+}
+
+SECTION("polygon") {
+
+    {
+        mapnik::geometry::polygon<double> poly;
+        REQUIRE(mapnik::geometry::is_empty(poly));
+    }
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        poly.set_exterior_ring(std::move(ring));
+        REQUIRE(mapnik::geometry::is_empty(poly));
+    }
+    {
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        ring.add_coord(0, 0);
+        ring.add_coord(1, 0);
+        ring.add_coord(1, 1);
+        ring.add_coord(0, 1);
+        ring.add_coord(0, 0);
+        poly.set_exterior_ring(std::move(ring));
+        REQUIRE(!mapnik::geometry::is_empty(poly));
+    }
+}
+
+SECTION("multi-point") {
+
+    {
+        mapnik::geometry::multi_point<double> geom;
+        REQUIRE(mapnik::geometry::is_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_point<double> geom;
+        geom.add_coord(0, 0);
+        geom.add_coord(25, 25);
+        geom.add_coord(50, 50);
+        REQUIRE(!mapnik::geometry::is_empty(geom));
+    }
+}
+
+SECTION("multi-linestring") {
+
+    {
+        mapnik::geometry::multi_line_string<double> geom;
+        REQUIRE(mapnik::geometry::is_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_line_string<double> geom;
+        mapnik::geometry::line_string<double> line;
+        geom.emplace_back(std::move(line));
+        REQUIRE(!mapnik::geometry::is_empty(geom));
+    }
+}
+
+SECTION("multi-polygon") {
+
+    {
+        mapnik::geometry::multi_polygon<double> geom;
+        REQUIRE(mapnik::geometry::is_empty(geom));
+    }
+    {
+        mapnik::geometry::multi_polygon<double> geom;
+        mapnik::geometry::polygon<double> poly;
+        mapnik::geometry::linear_ring<double> ring;
+        poly.set_exterior_ring(std::move(ring));
+        geom.emplace_back(std::move(poly));
+        REQUIRE(!mapnik::geometry::is_empty(geom));
+    }
+}
+}

test/unit/geometry/label_algo_test.cpp

@@ -1,33 +0,0 @@
-#include "catch.hpp"
-
-#include <iostream>
-#include <mapnik/geometry.hpp>
-#include <mapnik/geometry_centroid.hpp>
-#include <algorithm>
-
-TEST_CASE("labeling") {
-
-SECTION("algorithms") {
-
-    // reused these for simplicity
-    mapnik::geometry::point<double> centroid;
-    {
-        // single point
-        mapnik::geometry::point<double> pt(10,10);
-        REQUIRE( mapnik::geometry::centroid(pt, centroid));
-        REQUIRE( pt.x == centroid.x);
-        REQUIRE( pt.y == centroid.y);
-    }
-
-    // linestring with three consecutive verticies
-    {
-        mapnik::geometry::line_string<double> line;
-        line.add_coord(0, 0);
-        line.add_coord(25, 25);
-        line.add_coord(50, 50);
-        REQUIRE(mapnik::geometry::centroid(line, centroid));
-        REQUIRE( centroid.x == 25 );
-        REQUIRE( centroid.y == 25 );
-    }
-}
-}