refactor feature_grammar_x3 to have a cleaner separation between feature and geometry rules

include/mapnik/json/feature_grammar_x3.hpp

@@ -23,6 +23,8 @@
 #ifndef MAPNIK_JSON_FEATURE_GRAMMAR_X3_HPP
 #define MAPNIK_JSON_FEATURE_GRAMMAR_X3_HPP
 
+#include <mapnik/geometry.hpp>
+
 #pragma GCC diagnostic push
 #include <mapnik/warning_ignore.hpp>
 #include <boost/spirit/home/x3.hpp>
@@ -32,7 +34,7 @@ namespace mapnik { namespace json { namespace grammar {
 
 namespace x3 = boost::spirit::x3;
 using feature_grammar_type = x3::rule<class feature_rule_tag>;
-using geometry_grammar_type = x3::rule<class geometry_rule_tag>;
+using geometry_grammar_type = x3::rule<struct geomerty_rule_tag, mapnik::geometry::geometry<double> >;
 
 BOOST_SPIRIT_DECLARE(feature_grammar_type);
 BOOST_SPIRIT_DECLARE(geometry_grammar_type);

include/mapnik/json/feature_grammar_x3_def.hpp

@@ -35,6 +35,7 @@
 #include <mapnik/value.hpp>
 #include <mapnik/geometry/geometry_types.hpp>
 
+
 namespace mapnik { namespace json {
 
 struct stringifier
@@ -177,24 +178,26 @@ auto const assign_geometry_type = [] (auto const& ctx)
     std::get<0>(_val(ctx)) = _attr(ctx);
 };
 
-auto const create_collection = [] (auto const& ctx)
+auto const create_geometry = [] (auto const& ctx)
 {
-    mapnik::feature_impl & feature = x3::get<grammar::feature_tag>(ctx);
+    mapnik::geometry::geometry<double> geom;
-    feature.set_geometry(mapnik::geometry::geometry_collection<double>());
+    auto const type = std::get<0>(_attr(ctx));
+    if (type == mapnik::geometry::geometry_types::GeometryCollection)
+    {
+        _val(ctx) = std::move(std::get<2>(_attr(ctx)));
+    }
+    else
+    {
+        auto const& coordinates = std::get<1>(_attr(ctx));
+        mapnik::json::create_geometry(geom, type, coordinates);
+        _val(ctx) = std::move(geom);
+    }
 };
 
 auto const assign_geometry = [] (auto const& ctx)
 {
     mapnik::feature_impl & feature = x3::get<grammar::feature_tag>(ctx);
-    // first check if we're not dealing with GeometryCollection
+    feature.set_geometry(std::move(_attr(ctx)));
-    if (feature.get_geometry().is<mapnik::geometry::geometry_empty>())
-    {
-        mapnik::geometry::geometry<double> geom;
-        auto const type = std::get<0>(_attr(ctx));
-        auto const& coordinates = std::get<1>(_attr(ctx));
-        mapnik::json::create_geometry(geom, type, coordinates);
-        feature.set_geometry(std::move(geom));
-    }
 };
 
 auto const push_geometry = [] (auto const& ctx)
@@ -203,14 +206,7 @@ auto const push_geometry = [] (auto const& ctx)
     auto const type = std::get<0>(_attr(ctx));
     auto const& coordinates = std::get<1>(_attr(ctx));
     mapnik::json::create_geometry(geom, type, coordinates);
-    mapnik::feature_impl & feature = x3::get<grammar::feature_tag>(ctx);
+    _val(ctx).push_back(std::move(geom));
-    geometry::geometry<double> & collection = feature.get_geometry();
-
-    if (collection.is<mapnik::geometry::geometry_collection<double>>())
-    {
-        auto & col = collection.get<mapnik::geometry::geometry_collection<double>>();
-        col.push_back(std::move(geom));
-    }
 };
 
 auto const assign_positions = [] (auto const& ctx)
@@ -218,6 +214,11 @@ auto const assign_positions = [] (auto const& ctx)
     std::get<1>(_val(ctx)) = std::move(_attr(ctx));
 };
 
+auto const assign_collection = [] (auto const& ctx)
+{
+    std::get<2>(_val(ctx)) = std::move(_attr(ctx));
+};
+
 auto assign_property = [](auto const& ctx)
 {
     mapnik::feature_impl & feature = x3::get<grammar::feature_tag>(ctx);
@@ -229,17 +230,18 @@ auto assign_property = [](auto const& ctx)
 
 
 //exported rules
-feature_grammar_type const feature_rule = "Feature";
+feature_grammar_type const feature_rule = "Feature Rule";
-geometry_grammar_type const geometry_rule = "Geometry";
+geometry_grammar_type const geometry_rule = "Feature Rule";
+
 // rules
 x3::rule<struct feature_type_tag> const feature_type = "Feature Type";
 x3::rule<struct geometry_type_tag, mapnik::geometry::geometry_types> const geometry_type = "Geometry Type";
 x3::rule<struct coordinates_tag, mapnik::json::positions> const coordinates = "Coordinates";
-x3::rule<struct geometry_tag, std::tuple<mapnik::geometry::geometry_types, mapnik::json::positions>> const geometry_tuple = "Geometry";
+x3::rule<struct geomerty_tag, std::tuple<mapnik::geometry::geometry_types, mapnik::json::positions, mapnik::geometry::geometry_collection<double>>> const geometry_tuple = "Geometry";
-x3::rule<struct property_tag, std::tuple<std::string, json_value>> const property = "Property";
+x3::rule<struct property, std::tuple<std::string, json_value>> const property = "Property";
 x3::rule<struct properties_tag> const properties = "Properties";
 x3::rule<struct feature_part_rule_tag> const feature_part = "Feature part";
-x3::rule<struct geometry_collection> const geometry_collection = "GeometryCollection";
+x3::rule<struct geometry_collection, mapnik::geometry::geometry_collection<double>> const geometry_collection = "GeometryCollection";
 
 auto const feature_type_def = lit("\"type\"") > lit(':') > lit("\"Feature\"");
 
@@ -247,17 +249,16 @@ auto const geometry_type_def = lit("\"type\"") > lit(':') > geometry_type_symbol
 
 auto const coordinates_def = lit("\"coordinates\"") > lit(':') > positions_rule;
 
-auto const geometry_collection_def = lit("\"geometries\"")[create_collection] > lit(':')
+auto const geometry_collection_def = lit("\"geometries\"") > lit(':')
     > lit('[')
     > ((lit('{') > geometry_tuple[push_geometry] > lit('}')) % lit(','))
     > lit(']');
 
-
 auto const geometry_tuple_def = (geometry_type[assign_geometry_type]
                                  |
                                  coordinates[assign_positions]
                                  |
-                                 geometry_collection
+                                 geometry_collection[assign_collection]
                                  |
                                  (omit[geojson_string] > lit(':') > omit[value])) % lit(',');
 
@@ -268,7 +269,7 @@ auto const properties_def = property[assign_property] % lit(',');
 
 auto const feature_part_def = feature_type
     |
-    (lit("\"geometry\"") > lit(':') > (lit('{') > geometry_tuple[assign_geometry] > lit('}')) | lit("null"))
+    (lit("\"geometry\"") > lit(':') >  geometry_rule[assign_geometry])
     |
     (lit("\"properties\"") > lit(':') > lit('{') > -properties > lit('}'))
     |
@@ -278,7 +279,8 @@ auto const feature_part_def = feature_type
 
 auto const feature_rule_def = lit('{') > feature_part % lit(',') > lit('}');
 
-auto const geometry_rule_def =  (lit('{') > geometry_tuple[assign_geometry] > lit('}')) | lit("null");
+
+auto const geometry_rule_def =  (lit('{') > geometry_tuple[create_geometry] > lit('}')) | lit("null");
 
 #pragma GCC diagnostic push
 #include <mapnik/warning_ignore.hpp>
@@ -299,7 +301,6 @@ BOOST_SPIRIT_DEFINE(
 
 }}}
 
-
 namespace mapnik { namespace json {
 
 grammar::feature_grammar_type const& feature_grammar()

include/mapnik/json/json_grammar_config.hpp

@@ -92,13 +92,11 @@ using context_type = x3::with_context<keys_tag,
                                       std::reference_wrapper<keys_map> const,
                                       phrase_parse_context_type>::type;
 
-using geometry_context_type = x3::with_context<feature_tag,
-                                               std::reference_wrapper<mapnik::feature_impl> const,
-                                               phrase_parse_context_type>::type;
-
 using feature_context_type = x3::with_context<transcoder_tag,
                                               std::reference_wrapper<mapnik::transcoder> const,
-                                              geometry_context_type>::type;
+                                              x3::with_context<feature_tag,
+                                                               std::reference_wrapper<mapnik::feature_impl> const,
+                                                               phrase_parse_context_type>::type>::type;
 
 // our spirit x3 grammars needs this one with changed order of feature_impl and transcoder (??)
 using feature_context_const_type = x3::with_context<feature_tag,

src/json/feature_grammar_x3.cpp

@@ -25,9 +25,9 @@
 
 namespace mapnik { namespace json { namespace grammar {
 
-BOOST_SPIRIT_INSTANTIATE(geometry_grammar_type, iterator_type, geometry_context_type);
 BOOST_SPIRIT_INSTANTIATE(feature_grammar_type, iterator_type, feature_context_type);
-BOOST_SPIRIT_INSTANTIATE_UNUSED(geometry_grammar_type, iterator_type, geometry_context_type);
+BOOST_SPIRIT_INSTANTIATE(geometry_grammar_type, iterator_type, phrase_parse_context_type);
+
 BOOST_SPIRIT_INSTANTIATE_UNUSED(feature_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(feature_grammar_type, iterator_type, feature_context_const_type);
 

src/json/generic_json_grammar_x3.cpp

@@ -27,10 +27,11 @@ namespace mapnik { namespace json { namespace grammar {
 
 BOOST_SPIRIT_INSTANTIATE(generic_json_grammar_type, iterator_type, phrase_parse_context_type);
 BOOST_SPIRIT_INSTANTIATE(generic_json_grammar_type, iterator_type, context_type);
-BOOST_SPIRIT_INSTANTIATE(generic_json_grammar_type, iterator_type, geometry_context_type);
 BOOST_SPIRIT_INSTANTIATE(generic_json_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE(generic_json_grammar_type, iterator_type, feature_context_const_type);
-BOOST_SPIRIT_INSTANTIATE_UNUSED(generic_json_grammar_type, iterator_type, geometry_context_type);
+
+BOOST_SPIRIT_INSTANTIATE_UNUSED(generic_json_grammar_type, iterator_type, phrase_parse_context_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(generic_json_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(generic_json_grammar_type, iterator_type, feature_context_const_type);
+
 }}}

src/json/parse_feature.cpp

@@ -49,9 +49,8 @@ void parse_geometry(Iterator start, Iterator end, feature_impl& feature)
 {
     namespace x3 = boost::spirit::x3;
     using space_type = mapnik::json::grammar::space_type;
-    auto grammar = x3::with<mapnik::json::grammar::feature_tag>(std::ref(feature))
+    auto grammar = mapnik::json::geometry_grammar();
-        [ mapnik::json::geometry_grammar() ];
+    if (!x3::phrase_parse(start, end, grammar, space_type(), feature.get_geometry()))
-    if (!x3::phrase_parse(start, end, grammar, space_type()))
     {
         throw std::runtime_error("Can't parser GeoJSON Geometry");
     }

src/json/positions_grammar_x3.cpp

@@ -28,7 +28,6 @@ namespace mapnik { namespace json { namespace grammar {
 
 BOOST_SPIRIT_INSTANTIATE(positions_grammar_type, iterator_type, phrase_parse_context_type);
 BOOST_SPIRIT_INSTANTIATE(positions_grammar_type, iterator_type, context_type);
-BOOST_SPIRIT_INSTANTIATE(positions_grammar_type, iterator_type, geometry_context_type);
 BOOST_SPIRIT_INSTANTIATE(positions_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE(positions_grammar_type, iterator_type, feature_context_const_type);
 

src/json/unicode_string_grammar_x3.cpp

@@ -28,7 +28,6 @@ namespace mapnik { namespace json { namespace grammar {
 
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, phrase_parse_context_type);
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, context_type);
-BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, geometry_context_type);
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, feature_context_const_type);
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, extract_bounding_boxes_context_type);
@@ -36,7 +35,7 @@ BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, extract_bou
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, extract_bounding_boxes_context_type_f);
 BOOST_SPIRIT_INSTANTIATE(unicode_string_grammar_type, iterator_type, extract_bounding_boxes_reverse_context_type_f);
 
-BOOST_SPIRIT_INSTANTIATE_UNUSED(unicode_string_grammar_type, iterator_type, geometry_context_type);
+BOOST_SPIRIT_INSTANTIATE_UNUSED(unicode_string_grammar_type, iterator_type, phrase_parse_context_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(unicode_string_grammar_type, iterator_type, feature_context_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(unicode_string_grammar_type, iterator_type, feature_context_const_type);
 BOOST_SPIRIT_INSTANTIATE_UNUSED(unicode_string_grammar_type, iterator_type, extract_bounding_boxes_context_type);