From 25217549f18005d578d9c0bb31055edda7411250 Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 17 May 2016 12:53:07 +0200 Subject: [PATCH] fix topojson parsing (work-in-progress) --- include/mapnik/json/topojson_grammar.hpp | 2 - include/mapnik/json/topojson_grammar_impl.hpp | 54 +++--- include/mapnik/json/topojson_utils.hpp | 162 ++++++++++-------- include/mapnik/json/topology.hpp | 9 +- .../input/topojson/topojson_datasource.cpp | 9 +- test/data | 2 +- test/unit/datasource/ds_test_util.hpp | 1 + test/unit/datasource/topojson.cpp | 40 +++++ 8 files changed, 178 insertions(+), 101 deletions(-) diff --git a/include/mapnik/json/topojson_grammar.hpp b/include/mapnik/json/topojson_grammar.hpp index 7f42ce58c..db6f5492e 100644 --- a/include/mapnik/json/topojson_grammar.hpp +++ b/include/mapnik/json/topojson_grammar.hpp @@ -70,8 +70,6 @@ private: // properties qi::rule properties; qi::rule attributes; - // id - qi::rule id; }; }} diff --git a/include/mapnik/json/topojson_grammar_impl.hpp b/include/mapnik/json/topojson_grammar_impl.hpp index 0bb59b041..316b5e80a 100644 --- a/include/mapnik/json/topojson_grammar_impl.hpp +++ b/include/mapnik/json/topojson_grammar_impl.hpp @@ -71,13 +71,13 @@ BOOST_FUSION_ADAPT_STRUCT( BOOST_FUSION_ADAPT_STRUCT( mapnik::topojson::linestring, - (mapnik::topojson::index_type, ring) + (std::vector, rings) (boost::optional, props) ) BOOST_FUSION_ADAPT_STRUCT( mapnik::topojson::multi_linestring, - (std::vector, rings) + (std::vector >, lines) (boost::optional, props) ) @@ -101,6 +101,8 @@ BOOST_FUSION_ADAPT_STRUCT( (boost::optional, bbox) ) + + namespace mapnik { namespace topojson { namespace qi = boost::spirit::qi; @@ -128,7 +130,6 @@ topojson_grammar::topojson_grammar() // error handler boost::phoenix::function const error_handler; - // generic JSON types json.value = json.object | json.array | json.string_ | json.number ; @@ -180,7 +181,7 @@ topojson_grammar::topojson_grammar() >> lit('{') >> -((omit[json.string_] >> lit(':') - >> (geometry_collection(_val) | geometry)) % lit(',')) + >> (geometry_collection(_val) | geometry[push_back(_val, _1)]) % lit(','))) >> lit('}') ; @@ -195,17 +196,21 @@ topojson_grammar::topojson_grammar() ; geometry_collection = lit('{') - >> lit("\"type\"") >> lit(':') >> lit("\"GeometryCollection\"") - >> -(lit(',') >> omit[bbox]) - >> lit(',') >> lit("\"geometries\"") >> lit(':') >> lit('[') >> -(geometry[push_back(_r1, _1)] % lit(',')) - >> lit(']') - >> lit('}') + >> lit("\"type\"") >> lit(':') >> lit("\"GeometryCollection\"") + >> -(lit(',') >> omit[bbox]) + >> lit(',') >> lit("\"geometries\"") >> lit(':') >> lit('[') >> -(geometry[push_back(_r1, _1)] % lit(',')) + >> lit(']') + >> lit('}') ; + point = lit('{') >> lit("\"type\"") >> lit(':') >> lit("\"Point\"") >> -(lit(',') >> omit[bbox]) >> ((lit(',') >> lit("\"coordinates\"") >> lit(':') >> coordinate) - ^ (lit(',') >> properties) /*^ (lit(',') >> omit[id])*/) + ^ + (lit(',') >> properties) + ^ + (lit(',') >> omit[json.key_value])) >> lit('}') ; @@ -214,23 +219,30 @@ topojson_grammar::topojson_grammar() >> -(lit(',') >> omit[bbox]) >> ((lit(',') >> lit("\"coordinates\"") >> lit(':') >> lit('[') >> -(coordinate % lit(',')) >> lit(']')) - ^ (lit(',') >> properties) ^ (lit(',') >> omit[id])) + ^ + (lit(',') >> properties) ^ (lit(',') >> omit[json.key_value])) >> lit('}') ; linestring = lit('{') >> lit("\"type\"") >> lit(':') >> lit("\"LineString\"") - >> ((lit(',') >> lit("\"arcs\"") >> lit(':') >> lit('[') >> int_ >> lit(']')) - ^ (lit(',') >> properties) ^ (lit(',') >> omit[id])) + >> (lit(',') >> (lit("\"arcs\"") >> lit(':') >> ring) + ^ + (lit(',') >> properties)) + //^ + // (lit(',') >> omit[json.key_value])) >> lit('}') ; multi_linestring = lit('{') >> lit("\"type\"") >> lit(':') >> lit("\"MultiLineString\"") >> -(lit(',') >> omit[bbox]) - >> ((lit(',') >> lit("\"arcs\"") >> lit(':') >> lit('[') - >> -((lit('[') >> int_ >> lit(']')) % lit(',')) >> lit(']')) - ^ (lit(',') >> properties) ^ (lit(',') >> omit[id])) + >> ((lit(',') >> lit("\"arcs\"") >> lit(':') + >> lit('[') >> -(ring % lit(',')) >> lit(']')) + ^ + (lit(',') >> properties) + ^ + (lit(',') >> omit[json.key_value])) >> lit('}') ; @@ -239,7 +251,8 @@ topojson_grammar::topojson_grammar() >> -(lit(',') >> omit[bbox]) >> ((lit(',') >> lit("\"arcs\"") >> lit(':') >> lit('[') >> -(ring % lit(',')) >> lit(']')) - ^ (lit(',') >> properties) ^ (lit(',') >> omit[id])) + ^ (lit(',') >> properties)) + //^ (lit(',') >> omit[json.key_value])) >> lit('}') ; @@ -249,19 +262,16 @@ topojson_grammar::topojson_grammar() >> ((lit(',') >> lit("\"arcs\"") >> lit(':') >> lit('[') >> -((lit('[') >> -(ring % lit(',')) >> lit(']')) % lit(',')) - >> lit(']')) ^ (lit(',') >> properties) ^ (lit(',') >> omit[id])) + >> lit(']')) ^ (lit(',') >> properties) ^ (lit(',') >> omit[json.key_value])) >> lit('}') ; - id = lit("\"id\"") >> lit(':') >> omit[json.value] - ; - ring = lit('[') >> -(int_ % lit(',')) >> lit(']') ; properties = lit("\"properties\"") >> lit(':') - >> (( lit('{') >> attributes >> lit('}')) | json.object) + >> lit('{') >> attributes >> lit('}') ; attributes = (json.string_ >> lit(':') >> json.value) % lit(',') diff --git a/include/mapnik/json/topojson_utils.hpp b/include/mapnik/json/topojson_utils.hpp index 0402bb687..39c759f77 100644 --- a/include/mapnik/json/topojson_utils.hpp +++ b/include/mapnik/json/topojson_utils.hpp @@ -40,6 +40,12 @@ struct bounding_box_visitor : topo_(topo), num_arcs_(topo_.arcs.size()) {} + box2d operator() (mapnik::topojson::empty const&) const + { + std::cerr << "EMPTY FAIL" << std::endl; + return box2d(); + } + box2d operator() (mapnik::topojson::point const& pt) const { double x = pt.coord.x; @@ -82,50 +88,15 @@ struct bounding_box_visitor box2d operator() (mapnik::topojson::linestring const& line) const { box2d bbox; + bool first = true; if (num_arcs_ > 0) { - index_type index = line.ring; - index_type arc_index = index < 0 ? std::abs(index) - 1 : index; - if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) - { - bool first = true; - double px = 0, py = 0; - auto const& arcs = topo_.arcs[arc_index]; - for (auto pt : arcs.coordinates) - { - double x = pt.x; - double y = pt.y; - if (topo_.tr) - { - x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; - y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; - } - if (first) - { - first = false; - bbox.init(x, y, x, y); - } - else - { - bbox.expand_to_include(x, y); - } - } - } - } - return bbox; - } - - box2d operator() (mapnik::topojson::multi_linestring const& multi_line) const - { - box2d bbox; - if (num_arcs_ > 0) - { - bool first = true; - for (auto index : multi_line.rings) + for (auto index : line.rings) { index_type arc_index = index < 0 ? std::abs(index) - 1 : index; if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) { + double px = 0, py = 0; auto const& arcs = topo_.arcs[arc_index]; for (auto pt : arcs.coordinates) @@ -153,6 +124,47 @@ struct bounding_box_visitor return bbox; } + box2d operator() (mapnik::topojson::multi_linestring const& multi_line) const + { + box2d bbox; + if (num_arcs_ > 0) + { + bool first = true; + for (auto const& line : multi_line.lines) + { + for (auto index : line) + { + index_type arc_index = index < 0 ? std::abs(index) - 1 : index; + if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) + { + double px = 0, py = 0; + auto const& arcs = topo_.arcs[arc_index]; + for (auto pt : arcs.coordinates) + { + double x = pt.x; + double y = pt.y; + if (topo_.tr) + { + x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; + y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; + } + if (first) + { + first = false; + bbox.init(x, y, x, y); + } + else + { + bbox.expand_to_include(x, y); + } + } + } + } + } + } + return bbox; + } + box2d operator() (mapnik::topojson::polygon const& poly) const { box2d bbox; @@ -314,29 +326,31 @@ struct feature_generator mapnik::feature_ptr feature(mapnik::feature_factory::create(ctx_,feature_id_)); if (num_arcs_ > 0) { - index_type index = line.ring; - index_type arc_index = index < 0 ? std::abs(index) - 1 : index; - if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) - { - auto const& arcs = topo_.arcs[arc_index]; - double px = 0, py = 0; - mapnik::geometry::line_string line_string; - line_string.reserve(arcs.coordinates.size()); + mapnik::geometry::line_string line_string; - for (auto pt : arcs.coordinates) + for (auto index : line.rings) + { + index_type arc_index = index < 0 ? std::abs(index) - 1 : index; + if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) { - double x = pt.x; - double y = pt.y; - if (topo_.tr) + auto const& arcs = topo_.arcs[arc_index]; + double px = 0, py = 0; + line_string.reserve(line_string.size() + arcs.coordinates.size()); + for (auto pt : arcs.coordinates) { - x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; - y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; + double x = pt.x; + double y = pt.y; + if (topo_.tr) + { + x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; + y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; + } + line_string.add_coord(x,y); } - line_string.add_coord(x,y); } - feature->set_geometry(std::move(line_string)); - assign_properties(*feature, line, tr_); } + feature->set_geometry(std::move(line_string)); + assign_properties(*feature, line, tr_); } return feature; } @@ -348,30 +362,34 @@ struct feature_generator { mapnik::geometry::multi_line_string multi_line_string; bool hit = false; - multi_line_string.reserve(multi_line.rings.size()); - for (auto const& index : multi_line.rings) + for (auto const& line : multi_line.lines) { - index_type arc_index = index < 0 ? std::abs(index) - 1 : index; - if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) + multi_line_string.reserve(multi_line_string.size() + line.size()); + mapnik::geometry::line_string line_string; + for (auto index : line) { - hit = true; - double px = 0, py = 0; - mapnik::geometry::line_string line_string; - auto const& arcs = topo_.arcs[arc_index]; - line_string.reserve(arcs.coordinates.size()); - for (auto pt : arcs.coordinates) + index_type arc_index = index < 0 ? std::abs(index) - 1 : index; + if (arc_index >= 0 && arc_index < static_cast(num_arcs_)) { - double x = pt.x; - double y = pt.y; - if (topo_.tr) + hit = true; + double px = 0, py = 0; + auto const& arcs = topo_.arcs[arc_index]; + line_string.reserve(line_string.size() + arcs.coordinates.size()); + for (auto pt : arcs.coordinates) { - x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; - y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; + double x = pt.x; + double y = pt.y; + if (topo_.tr) + { + x = (px += x) * (*topo_.tr).scale_x + (*topo_.tr).translate_x; + y = (py += y) * (*topo_.tr).scale_y + (*topo_.tr).translate_y; + } + line_string.add_coord(x, y); } - line_string.add_coord(x, y); + } - multi_line_string.push_back(std::move(line_string)); } + multi_line_string.push_back(std::move(line_string)); } if (hit) { diff --git a/include/mapnik/json/topology.hpp b/include/mapnik/json/topology.hpp index 05f35b4ad..048c85528 100644 --- a/include/mapnik/json/topology.hpp +++ b/include/mapnik/json/topology.hpp @@ -62,13 +62,13 @@ struct multi_point struct linestring { - index_type ring ; + std::vector rings ; boost::optional props; }; struct multi_linestring { - std::vector rings; + std::vector > lines; boost::optional props; }; @@ -84,7 +84,10 @@ struct multi_polygon boost::optional props; }; -using geometry = util::variant mapnik::eAttributeType operator() (T const& /*val*/) const @@ -101,6 +100,11 @@ struct geometry_type_visitor { return static_cast(mapnik::datasource_geometry_t::Polygon); } + template + int operator() (T const& ) const + { + return 0; + } }; struct collect_attributes_visitor @@ -109,6 +113,9 @@ struct collect_attributes_visitor collect_attributes_visitor(mapnik::layer_descriptor & desc): desc_(desc) {} + // no-op + void operator() (mapnik::topojson::empty) {} + // template void operator() (GeomType const& g) { diff --git a/test/data b/test/data index 765765833..b2ec613fb 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 7657658330c7b95f2d431fa199de8b7629b41fab +Subproject commit b2ec613fb5fba85e6f810dabe2436c9e5b2d90b0 diff --git a/test/unit/datasource/ds_test_util.hpp b/test/unit/datasource/ds_test_util.hpp index 46868d4e5..6a9c706e6 100644 --- a/test/unit/datasource/ds_test_util.hpp +++ b/test/unit/datasource/ds_test_util.hpp @@ -110,6 +110,7 @@ using attr = std::tuple; #define REQUIRE_ATTRIBUTES(feature, attrs) \ REQUIRE(bool(feature)); \ for (auto const &kv : attrs) { \ + std::cerr << std::get<0>(kv) << std::endl; \ REQUIRE(feature->has_key(std::get<0>(kv))); \ CHECK(feature->get(std::get<0>(kv)) == std::get<1>(kv)); \ } \ diff --git a/test/unit/datasource/topojson.cpp b/test/unit/datasource/topojson.cpp index 25f9ffdaf..78b75de87 100644 --- a/test/unit/datasource/topojson.cpp +++ b/test/unit/datasource/topojson.cpp @@ -21,6 +21,7 @@ *****************************************************************************/ #include "catch.hpp" +#include "ds_test_util.hpp" #include #include @@ -59,6 +60,7 @@ TEST_CASE("topology") mapnik::transcoder tr("utf8"); for (auto const& path : mapnik::util::list_directory("test/data/topojson/")) { + std::cerr << path << std::endl; mapnik::topojson::topology topo; REQUIRE(parse_topology(path, topo)); for (auto const& geom : topo.geometries) @@ -72,4 +74,42 @@ TEST_CASE("topology") } } } + + SECTION("TopoJSON properties are properly expressed") + { + std::string filename("./test/data/topojson/escaped.topojson"); + mapnik::context_ptr ctx = std::make_shared(); + mapnik::transcoder tr("utf8"); + mapnik::topojson::topology topo; + REQUIRE(parse_topology(filename, topo)); + mapnik::value_integer feature_id = 0; + for (auto const& geom : topo.geometries) + { + mapnik::box2d bbox = mapnik::util::apply_visitor(mapnik::topojson::bounding_box_visitor(topo), geom); + CHECK(bbox.valid()); + mapnik::topojson::feature_generator visitor(ctx, tr, topo, feature_id); + mapnik::feature_ptr feature = mapnik::util::apply_visitor(visitor, geom); + CHECK(feature); + CHECK(feature->envelope() == bbox); + std::initializer_list attrs = { + attr{"name", mapnik::value_unicode_string("Test")}, + attr{"NOM_FR", mapnik::value_unicode_string("Québec")}, + attr{"boolean", mapnik::value_bool("true")}, + attr{"description", mapnik::value_unicode_string("Test: \u005C")}, + attr{"double", mapnik::value_double(1.1)}, + attr{"int", mapnik::value_integer(1)}, + attr{"object", mapnik::value_unicode_string("{name:\"waka\",spaces:\"value with spaces\",int:1,double:1.1,boolean:false" + ",NOM_FR:\"Québec\",array:[\"string\",\"value with spaces\",3,1.1,null,true" + ",\"Québec\"],another_object:{name:\"nested object\"}}")}, + attr{"spaces", mapnik::value_unicode_string("this has spaces")}, + attr{"array", mapnik::value_unicode_string("[\"string\",\"value with spaces\",3,1.1,null,true," + "\"Québec\",{name:\"object within an array\"}," + "[\"array\",\"within\",\"an\",\"array\"]]")}, + attr{"empty_array", mapnik::value_unicode_string("[]")}, + attr{"empty_object", mapnik::value_unicode_string("{}")}, + }; + REQUIRE_ATTRIBUTES(feature, attrs); + } + } + }