Merge pull request #4432 from mapnik/geojson-empty-geometries

Geojson - empty positions arrays handling

include/mapnik/geometry/is_empty.hpp

@@ -39,18 +39,39 @@ struct geometry_is_empty
 
     bool operator()(mapnik::geometry::point<double> const&) const { return false; }
 
-    bool operator()(mapnik::geometry::line_string<double> const& geom) const { return geom.empty(); }
+    bool operator()(mapnik::geometry::line_string<double> const& line) const { return line.empty(); }
 
-    bool operator()(mapnik::geometry::polygon<double> const& geom) const
+    bool operator()(mapnik::geometry::polygon<double> const& poly) const
     {
-        return geom.empty() || geom.front().empty();
+        for (auto const& ring : poly)
+        {
+            if (!ring.empty())
+                return false;
+        }
+        return true;
     }
 
     bool operator()(mapnik::geometry::multi_point<double> const& geom) const { return geom.empty(); }
 
-    bool operator()(mapnik::geometry::multi_line_string<double> const& geom) const { return geom.empty(); }
+    bool operator()(mapnik::geometry::multi_line_string<double> const& mline) const
+    {
+        for (auto const& line : mline)
+        {
+            if (!line.empty())
+                return false;
+        }
+        return true;
+    }
 
-    bool operator()(mapnik::geometry::multi_polygon<double> const& geom) const { return geom.empty(); }
+    bool operator()(mapnik::geometry::multi_polygon<double> const& mpoly) const
+    {
+        for (auto const& poly : mpoly)
+        {
+            if (!operator()(poly))
+                return false;
+        }
+        return true;
+    }
 
     bool operator()(mapnik::geometry::geometry_collection<double> const& geom) const { return geom.empty(); }
 

include/mapnik/json/create_geometry.hpp

@@ -64,6 +64,11 @@ struct create_linestring
     {
         mapnik::geometry::line_string<double> line;
         std::size_t size = points.size();
+        // if (size < 2)
+        //{
+        //     throw std::runtime_error("RFC 7946: For type \"LineString\", the \"coordinates\" member is an array of
+        //     two or more positions.");
+        // }
         line.reserve(size);
         for (auto&& pt : points)
         {
@@ -108,6 +113,12 @@ struct create_polygon
         mapnik::geometry::correct(geom_);
     }
 
+    void operator()(ring const& points) const
+    {
+        // POLYGON EMPTY
+        geom_ = std::move(mapnik::geometry::polygon<double>{});
+    }
+
     template<typename T>
     void operator()(T const&) const
     {
@@ -170,6 +181,12 @@ struct create_multilinestring
         geom_ = std::move(multi_line);
     }
 
+    void operator()(ring const& points) const
+    {
+        // MULTILINESTRING EMPTY
+        geom_ = std::move(mapnik::geometry::multi_line_string<double>{});
+    }
+
     template<typename T>
     void operator()(T const&) const
     {
@@ -213,6 +230,26 @@ struct create_multipolygon
         mapnik::geometry::correct(geom_);
     }
 
+    void operator()(rings const& rngs) const
+    {
+        // MULTIPOLYGON
+        mapnik::geometry::multi_polygon<double> multi_poly;
+        mapnik::geometry::polygon<double> poly;
+        std::size_t num_rings = rngs.size();
+        for (std::size_t i = 0; i < num_rings; ++i)
+        {
+            // POLYGON EMPTY
+            multi_poly.emplace_back(mapnik::geometry::polygon<double>{});
+        }
+        geom_ = std::move(multi_poly);
+    }
+
+    void operator()(ring const& points) const
+    {
+        // MULTIPOLYGON EMPTY
+        geom_ = std::move(mapnik::geometry::multi_polygon<double>{});
+    }
+
     template<typename T>
     void operator()(T const&) const
     {

include/mapnik/json/geometry_generator_grammar_impl.hpp

@@ -83,16 +83,16 @@ geometry_generator_grammar<OutputIterator, Geometry>::geometry_generator_grammar
     linear_ring_coord = lit('[') << -(point_coord % lit(',')) << lit(']')//linestring_coord.alias()
         ;
 
-    polygon_coord = lit('[') << linear_ring_coord % lit(',') << lit(']')
+    polygon_coord = lit('[') << -(linear_ring_coord % lit(',')) << lit(']')
         ;
 
     multi_point_coord = lit('[') << -(point_coord % lit(',')) << lit(']');//linestring_coord.alias()
         ;
 
-    multi_linestring_coord = lit('[') << linestring_coord  % lit(',') << lit(']')
+    multi_linestring_coord = lit('[') << -(linestring_coord  % lit(',')) << lit(']')
         ;
 
-    multi_polygon_coord = lit('[') << polygon_coord  % lit(',') << lit("]")
+    multi_polygon_coord = lit('[') << -(polygon_coord  % lit(',')) << lit("]")
         ;
 
     geometries = geometry % lit(',')

test/unit/datasource/geojson.cpp

@@ -28,6 +28,7 @@
 #include <mapnik/datasource_cache.hpp>
 #include <mapnik/geometry.hpp>
 #include <mapnik/geometry/geometry_type.hpp>
+#include <mapnik/geometry/is_empty.hpp>
 #include <mapnik/json/geometry_parser.hpp>
 #include <mapnik/util/geometry_to_geojson.hpp>
 #include <mapnik/util/fs.hpp>
@@ -126,9 +127,13 @@ TEST_CASE("geojson")
         {
             auto valid_empty_geometries = {"null", // Point can't be empty
                                            "{ \"type\": \"LineString\", \"coordinates\":[]}",
+                                           "{ \"type\": \"Polygon\", \"coordinates\":[]} ",
                                            "{ \"type\": \"Polygon\", \"coordinates\":[[]]} ",
                                            "{ \"type\": \"MultiPoint\", \"coordinates\":[]}",
+                                           "{ \"type\": \"MultiLineString\", \"coordinates\":[]}",
                                            "{ \"type\": \"MultiLineString\", \"coordinates\":[[]]}",
+                                           "{ \"type\": \"MultiPolygon\", \"coordinates\":[]}",
+                                           "{ \"type\": \"MultiPolygon\", \"coordinates\":[[]]}",
                                            "{ \"type\": \"MultiPolygon\", \"coordinates\": [[[]]] }"};
 
             for (auto const& in : valid_empty_geometries)
@@ -136,6 +141,7 @@ TEST_CASE("geojson")
                 std::string json(in);
                 mapnik::geometry::geometry<double> geom;
                 CHECK(mapnik::json::from_geojson(json, geom));
+                REQUIRE(mapnik::geometry::is_empty(geom));
                 //  round trip
                 std::string json_out;
                 CHECK(mapnik::util::to_geojson(json_out, geom));

test/unit/geometry/is_empty.cpp

@@ -1,3 +1,25 @@
+/*****************************************************************************
+ *
+ * This file is part of Mapnik (c++ mapping toolkit)
+ *
+ * Copyright (C) 2024 Artem Pavlenko
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ *****************************************************************************/
+
 #include "catch.hpp"
 
 #include <mapnik/geometry/is_empty.hpp>
@@ -95,7 +117,7 @@ TEST_CASE("geometry is_empty")
             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));
+            REQUIRE(mapnik::geometry::is_empty(geom));
         }
     }
 
@@ -111,7 +133,7 @@ TEST_CASE("geometry is_empty")
             mapnik::geometry::linear_ring<double> ring;
             poly.push_back(std::move(ring));
             geom.emplace_back(std::move(poly));
-            REQUIRE(!mapnik::geometry::is_empty(geom));
+            REQUIRE(mapnik::geometry::is_empty(geom));
         }
     }
 }