This is an automated email from the git hooks/post-receive script. sebastic pushed a commit to branch master in repository mapnik-vector-tile.
commit 9b742160076f0f9a097f81b891d4d3a2f8195b63 Author: Bas Couwenberg <sebas...@xs4all.nl> Date: Fri Sep 11 22:55:35 2015 +0200 Imported Upstream version 0.9.2+dfsg --- CHANGELOG.md | 6 + bench/enf.t5yd5cdi_14_13089_8506.vector.pbf | Bin 0 -> 1353537 bytes bench/multi_line_13_1310_3166.vector.pbf | Bin 0 -> 265593 bytes bench/notes.md | 58 ++++++++ bench/vtile-decode.cpp | 127 ++++++++++++++++++ gyp/build.gyp | 16 +++ package.json | 2 +- scripts/coverage.sh | 4 +- src/vector_tile_datasource.ipp | 9 +- src/vector_tile_datasource_pbf.ipp | 9 +- src/vector_tile_geometry_decoder.hpp | 200 ++++++++++++++++++++++++---- src/vector_tile_geometry_encoder.hpp | 57 +++++++- src/vector_tile_processor.ipp | 16 +-- src/vector_tile_util.ipp | 3 +- test/encoding_util.hpp | 96 +++++-------- test/geometry_encoding.cpp | 34 ++--- 16 files changed, 510 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76c85ba..373fea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 0.9.2 + + - Fixed multipoint encoding (#144) + - Optimized decoding by filtering geometry parts not within bbox (#146) + - Optimized decoding by calling `vector.reserve` before `vector.emplace_back` (#119) + ## 0.9.1 - Added `is_solid_extent` implementation based on protozero decoder diff --git a/bench/enf.t5yd5cdi_14_13089_8506.vector.pbf b/bench/enf.t5yd5cdi_14_13089_8506.vector.pbf new file mode 100644 index 0000000..587020c Binary files /dev/null and b/bench/enf.t5yd5cdi_14_13089_8506.vector.pbf differ diff --git a/bench/multi_line_13_1310_3166.vector.pbf b/bench/multi_line_13_1310_3166.vector.pbf new file mode 100644 index 0000000..00e258a Binary files /dev/null and b/bench/multi_line_13_1310_3166.vector.pbf differ diff --git a/bench/notes.md b/bench/notes.md new file mode 100644 index 0000000..7948c7f --- /dev/null +++ b/bench/notes.md @@ -0,0 +1,58 @@ +With reserve + +``` +$ ./build/Release/vtile-decode bench/multi_line_13_1310_3166.vector.pbf 13 1310 3166 +z:13 x:1310 y:3166 iterations:100 +message: zlib compressed +4026.30ms (cpu 4020.96ms) | decode as datasource_pbf: bench/multi_line_13_1310_3166.vector.pbf +``` + +without reserve code + +``` +$ ./build/Release/vtile-decode bench/multi_line_13_1310_3166.vector.pbf 13 1310 3166 +z:13 x:1310 y:3166 iterations:100 +message: zlib compressed +4296.88ms (cpu 4289.05ms) | decode as datasource_pbf: bench/multi_line_13_1310_3166.vector.pbf +``` + + +--------- + + +baseline (using mapnik::geometry::envelope + filter.pass) + +$ .//build/Release/vtile-decode bench/enf.t5yd5cdi_14_13089_8506.vector.pbf 14 13089 8506 200 +z:14 x:13089 y:8506 iterations:200 +2822.66ms (cpu 2821.10ms) | decode as datasource_pbf: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +2070.90ms (cpu 2070.44ms) | decode as datasource: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +processed 6800 features + +$ ./build/Release/vtile-decode bench/multi_line_13_1310_3166.vector.pbf 13 1310 3166 100 +z:13 x:1310 y:3166 iterations:100 +message: zlib compressed +4289.26ms (cpu 4275.29ms) | decode as datasource_pbf: bench/multi_line_13_1310_3166.vector.pbf + + + +commenting filter.pass/geometry::envelope in datasource_pbf + +$ .//build/Release/vtile-decode bench/enf.t5yd5cdi_14_13089_8506.vector.pbf 14 13089 8506 200 +z:14 x:13089 y:8506 iterations:200 +2305.45ms (cpu 2301.07ms) | decode as datasource_pbf: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +2142.56ms (cpu 2140.40ms) | decode as datasource: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +processed 6800 features + + +with bbox filter: + +$ .//build/Release/vtile-decode bench/enf.t5yd5cdi_14_13089_8506.vector.pbf 14 13089 8506 200 +z:14 x:13089 y:8506 iterations:200 +2497.01ms (cpu 2493.40ms) | decode as datasource_pbf: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +1753.55ms (cpu 1751.10ms) | decode as datasource: bench/enf.t5yd5cdi_14_13089_8506.vector.pbf +processed 6600 features + +$ ./build/Release/vtile-decode bench/multi_line_13_1310_3166.vector.pbf 13 1310 3166 100 +z:13 x:1310 y:3166 iterations:100 +message: zlib compressed +4090.67ms (cpu 4083.65ms) | decode as datasource_pbf: bench/multi_line_13_1310_3166.vector.pbf diff --git a/bench/vtile-decode.cpp b/bench/vtile-decode.cpp new file mode 100644 index 0000000..7dafbe5 --- /dev/null +++ b/bench/vtile-decode.cpp @@ -0,0 +1,127 @@ +#include <mapnik/timer.hpp> +#include <mapnik/util/file_io.hpp> +#include "vector_tile_datasource_pbf.hpp" +#include "vector_tile_datasource.hpp" +#include "vector_tile_compression.hpp" + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Wsign-conversion" +#include "vector_tile.pb.h" +#pragma GCC diagnostic pop + +#include <iostream> + +#include <protozero/pbf_reader.hpp> + +int main(int argc, char** argv) +{ + try + { + + if (argc < 4) + { + std::clog << "usage: vtile-decode /path/to/tile.vector.pbf z x y [iterations]\n"; + return -1; + } + std::string vtile(argv[1]); + mapnik::util::file input(vtile); + if (!input.open()) + { + std::clog << std::string("failed to open ") + vtile << "\n"; + return -1; + } + + int z = std::stoi(argv[2]); + int x = std::stoi(argv[3]); + int y = std::stoi(argv[4]); + + std::size_t iterations = 100; + if (argc > 5) + { + iterations = std::stoi(argv[5]); + } + + std::clog << "z:" << z << " x:" << x << " y:" << y << " iterations:" << iterations << "\n"; + + std::string message(input.data().get(), input.size()); + bool is_zlib = mapnik::vector_tile_impl::is_zlib_compressed(message); + bool is_gzip = mapnik::vector_tile_impl::is_gzip_compressed(message); + if (is_zlib || is_gzip) + { + if (is_zlib) + { + std::cout << "message: zlib compressed\n"; + } + else if (is_gzip) + { + std::cout << "message: gzip compressed\n"; + } + std::string uncompressed; + mapnik::vector_tile_impl::zlib_decompress(message,uncompressed); + message = uncompressed; + } + + std::size_t feature_count = 0; + std::size_t layer_count = 0; + { + mapnik::progress_timer __stats__(std::clog, std::string("decode as datasource_pbf: ") + vtile); + for (std::size_t i=0;i<iterations;++i) + { + protozero::pbf_reader tile(message); + while (tile.next()) { + if (tile.tag() == 3) { + ++layer_count; + protozero::pbf_reader layer = tile.get_message(); + auto ds = std::make_shared<mapnik::vector_tile_impl::tile_datasource_pbf>(layer,x,y,z,256); + mapnik::query q(ds->get_tile_extent()); + auto fs = ds->features(q); + while (fs->next()) { + ++feature_count; + } + } else { + tile.skip(); + } + } + } + } + + + std::size_t feature_count2 = 0; + std::size_t layer_count2 = 0; + { + mapnik::progress_timer __stats__(std::clog, std::string("decode as datasource: ") + vtile); + vector_tile::Tile tiledata; + tiledata.ParseFromString(message); + for (std::size_t i=0;i<iterations;++i) + { + for (int j=0;j<tiledata.layers_size(); ++j) + { + ++layer_count2; + auto const& layer = tiledata.layers(j); + auto ds = std::make_shared<mapnik::vector_tile_impl::tile_datasource>(layer,x,y,z,256); + mapnik::query q(ds->get_tile_extent()); + auto fs = ds->features(q); + while (fs->next()) { + ++feature_count2; + } + } + } + } + if (feature_count!= feature_count2) { + std::clog << "error: tile datasource impl did not return same # of features " << feature_count << " vs " << feature_count2 << "\n"; + return -1; + } else if (feature_count == 0) { + std::clog << "error: no features processed\n"; + return -1; + } else { + std::clog << "processed " << feature_count << " features\n"; + } + } + catch (std::exception const& ex) + { + std::clog << "error: " << ex.what() << "\n"; + return -1; + } + return 0; +} diff --git a/gyp/build.gyp b/gyp/build.gyp index a9d8a5f..1bfccf6 100644 --- a/gyp/build.gyp +++ b/gyp/build.gyp @@ -161,6 +161,22 @@ ] }, { + "target_name": "vtile-decode", + 'dependencies': [ 'mapnik_vector_tile_impl' ], + "type": "executable", + "defines": [ + "<@(common_defines)", + "MAPNIK_PLUGINDIR=<(MAPNIK_PLUGINDIR)" + ], + "sources": [ + "../bench/vtile-decode.cpp" + ], + "include_dirs": [ + "../src", + '../deps/protozero/include' + ] + }, + { "target_name": "tileinfo", 'dependencies': [ 'vector_tile' ], "type": "executable", diff --git a/package.json b/package.json index fb3cda9..b0b90a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mapnik-vector-tile", - "version": "0.9.1", + "version": "0.9.2", "description": "Mapnik vector tile API", "main": "./package.json", "repository" : { diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 3d0e223..ff4e494 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -15,8 +15,10 @@ if [[ ${COVERAGE} == true ]]; then ./mason_packages/.link/bin/cpp-coveralls \ --build-root build \ --gcov-options '\-lp' \ - --exclude test/catch.hpp \ --exclude examples \ + --exclude deps \ + --exclude test \ + --exclude bench \ --exclude build/Debug/obj/gen/ \ --exclude mason_packages \ --exclude scripts > /dev/null diff --git a/src/vector_tile_datasource.ipp b/src/vector_tile_datasource.ipp index 994a46a..aa35119 100644 --- a/src/vector_tile_datasource.ipp +++ b/src/vector_tile_datasource.ipp @@ -35,6 +35,10 @@ #include <boost/optional.hpp> #include <unicode/unistr.h> +#if defined(DEBUG) +#include <mapnik/debug.hpp> +#endif + namespace mapnik { namespace vector_tile_impl { void add_attributes(mapnik::feature_ptr feature, @@ -196,16 +200,19 @@ namespace mapnik { namespace vector_tile_impl { continue; } mapnik::vector_tile_impl::Geometry geoms(f,tile_x_, tile_y_, scale_, -1*scale_); - mapnik::geometry::geometry<double> geom = decode_geometry(geoms, f.type()); + mapnik::geometry::geometry<double> geom = decode_geometry(geoms, f.type(), filter_.box_); if (geom.is<mapnik::geometry::geometry_empty>()) { continue; } + #if defined(DEBUG) mapnik::box2d<double> envelope = mapnik::geometry::envelope(geom); if (!filter_.pass(envelope)) { + MAPNIK_LOG_ERROR(tile_datasource_pbf) << "tile_datasource: filter:pass should not get here"; continue; } + #endif mapnik::feature_ptr feature = mapnik::feature_factory::create(ctx_,feature_id); feature->set_geometry(std::move(geom)); add_attributes(feature,f,layer_,tr_); diff --git a/src/vector_tile_datasource_pbf.ipp b/src/vector_tile_datasource_pbf.ipp index 65ef644..3bc5a7a 100644 --- a/src/vector_tile_datasource_pbf.ipp +++ b/src/vector_tile_datasource_pbf.ipp @@ -32,6 +32,10 @@ #include <boost/optional.hpp> #include <unicode/unistr.h> +#if defined(DEBUG) +#include <mapnik/debug.hpp> +#endif + namespace mapnik { namespace vector_tile_impl { template <typename Filter> @@ -210,16 +214,19 @@ namespace mapnik { namespace vector_tile_impl { { auto geom_itr = f.get_packed_uint32(); mapnik::vector_tile_impl::GeometryPBF geoms(geom_itr, tile_x_,tile_y_,scale_,-1*scale_); - mapnik::geometry::geometry<double> geom = decode_geometry(geoms, geometry_type); + mapnik::geometry::geometry<double> geom = decode_geometry(geoms, geometry_type, filter_.box_); if (geom.is<mapnik::geometry::geometry_empty>()) { continue; } + #if defined(DEBUG) mapnik::box2d<double> envelope = mapnik::geometry::envelope(geom); if (!filter_.pass(envelope)) { + MAPNIK_LOG_ERROR(tile_datasource_pbf) << "tile_datasource_pbf: filter:pass should not get here"; continue; } + #endif feature->set_geometry(std::move(geom)); return feature; } diff --git a/src/vector_tile_geometry_decoder.hpp b/src/vector_tile_geometry_decoder.hpp index 1de0441..e577014 100644 --- a/src/vector_tile_geometry_decoder.hpp +++ b/src/vector_tile_geometry_decoder.hpp @@ -13,6 +13,10 @@ //std #include <algorithm> +#if defined(DEBUG) +#include <mapnik/debug.hpp> +#endif + namespace mapnik { namespace vector_tile_impl { // NOTE: this object is for one-time use. Once you've progressed to the end @@ -31,7 +35,7 @@ public: close = 7 }; - inline command next(double& rx, double& ry); + inline command next(double& rx, double& ry, std::uint32_t & len); private: vector_tile::Tile_Feature const& f_; @@ -62,14 +66,14 @@ public: close = 7 }; - inline command next(double& rx, double& ry); + inline command next(double& rx, double& ry, std::uint32_t & len); private: std::pair< protozero::pbf_reader::const_uint32_iterator, protozero::pbf_reader::const_uint32_iterator > geo_iterator_; double scale_x_; double scale_y_; uint8_t cmd; - uint32_t length; + std::uint32_t length; double x, y; double ox, oy; }; @@ -87,7 +91,7 @@ Geometry::Geometry(vector_tile::Tile_Feature const& f, x(tile_x), y(tile_y), ox(0), oy(0) {} -Geometry::command Geometry::next(double& rx, double& ry) +Geometry::command Geometry::next(double& rx, double& ry, std::uint32_t & len) { if (k < geoms_) { @@ -95,7 +99,7 @@ Geometry::command Geometry::next(double& rx, double& ry) { uint32_t cmd_length = static_cast<uint32_t>(f_.geometry(k++)); cmd = cmd_length & 0x7; - length = cmd_length >> 3; + len = length = cmd_length >> 3; } --length; @@ -145,7 +149,7 @@ GeometryPBF::GeometryPBF(std::pair<protozero::pbf_reader::const_uint32_iterator, x(tile_x), y(tile_y), ox(0), oy(0) {} -GeometryPBF::command GeometryPBF::next(double& rx, double& ry) +GeometryPBF::command GeometryPBF::next(double& rx, double& ry, std::uint32_t & len) { if (geo_iterator_.first != geo_iterator_.second) { @@ -153,7 +157,7 @@ GeometryPBF::command GeometryPBF::next(double& rx, double& ry) { uint32_t cmd_length = static_cast<uint32_t>(*geo_iterator_.first++); cmd = cmd_length & 0x7; - length = cmd_length >> 3; + len = length = cmd_length >> 3; } --length; @@ -200,17 +204,43 @@ GeometryPBF::command GeometryPBF::next(double& rx, double& ry) namespace detail { template <typename T> -void decode_point(mapnik::geometry::geometry<double> & geom, T & paths) +void decode_point(mapnik::geometry::geometry<double> & geom, T & paths, mapnik::box2d<double> const& bbox) { typename T::command cmd; double x1, y1; mapnik::geometry::multi_point<double> mp; - while ((cmd = paths.next(x1, y1)) != T::end) + bool first = true; + std::uint32_t len; + while ((cmd = paths.next(x1, y1, len)) != T::end) { - mp.emplace_back(mapnik::geometry::point<double>(x1,y1)); + // TODO: consider profiling and trying to optimize this further + // when all points are within the bbox filter then the `mp.reserve` should be + // perfect, but when some points are thrown out we will allocate more than needed + // the "all points intersect" case I think is going to be more common/important + // however worth a future look to see if the "some or few points intersect" can be optimized + if (first) + { + first = false; + mp.reserve(len); + } + if (!bbox.intersects(x1,y1)) + { + continue; + } + mp.emplace_back(x1,y1); } std::size_t num_points = mp.size(); - if (num_points == 1) + #if defined(DEBUG) + if (num_points > 0 && len != num_points) { + // BUG: https://github.com/mapbox/mapnik-vector-tile/issues/144 + MAPNIK_LOG_ERROR(decode_point) << "warning: encountered incorrectly encoded multipoint with " << num_points << " points but only " << len << " repeated commands"; + } + #endif + if (num_points == 0) + { + geom = mapnik::geometry::geometry_empty(); + } + else if (num_points == 1) { geom = std::move(mp[0]); } @@ -222,24 +252,71 @@ void decode_point(mapnik::geometry::geometry<double> & geom, T & paths) } template <typename T> -void decode_linestring(mapnik::geometry::geometry<double> & geom, T & paths) +void decode_linestring(mapnik::geometry::geometry<double> & geom, T & paths, mapnik::box2d<double> const& bbox) { typename T::command cmd; double x1, y1; mapnik::geometry::multi_line_string<double> multi_line; multi_line.emplace_back(); bool first = true; - while ((cmd = paths.next(x1, y1)) != T::end) + bool first_line_to = true; + std::uint32_t len; + #if defined(DEBUG) + std::uint32_t pre_len; + #endif + mapnik::box2d<double> part_env; + while ((cmd = paths.next(x1, y1, len)) != T::end) { if (cmd == T::move_to) { - if (first) first = false; - else multi_line.emplace_back(); + if (first) + { + first = false; + } + else + { + #if defined(DEBUG) + if (multi_line.back().size() > 0 && pre_len != multi_line.back().size()) + { + MAPNIK_LOG_ERROR(decode_linestring) << "warning: encountered incorrectly encoded line with " << multi_line.back().size() << " points but only " << pre_len << " repeated commands"; + } + #endif + first_line_to = true; + if (!bbox.intersects(part_env)) + { + // remove last line + multi_line.pop_back(); + } + // add fresh line to start adding to + multi_line.emplace_back(); + } + part_env.init(x1,y1,x1,y1); + } + else if (first_line_to && cmd == T::line_to) + { + first_line_to = false; + multi_line.back().reserve(len+1); + #if defined(DEBUG) + pre_len = len+1; + #endif + } + if (!first) + { + part_env.expand_to_include(x1,y1); } multi_line.back().add_coord(x1,y1); } + if (!bbox.intersects(part_env)) + { + // remove last line + multi_line.pop_back(); + } std::size_t num_lines = multi_line.size(); - if (num_lines == 1) + if (num_lines == 0) + { + geom = mapnik::geometry::geometry_empty(); + } + else if (num_lines == 1) { auto itr = std::make_move_iterator(multi_line.begin()); if (itr->size() > 1) @@ -254,22 +331,56 @@ void decode_linestring(mapnik::geometry::geometry<double> & geom, T & paths) } template <typename T> -std::vector<mapnik::geometry::linear_ring<double>> read_rings(T & paths) +void read_rings(std::vector<mapnik::geometry::linear_ring<double>> & rings, + T & paths, mapnik::box2d<double> const& bbox) { typename T::command cmd; double x1, y1; - std::vector<mapnik::geometry::linear_ring<double>> rings; rings.emplace_back(); double x2,y2; bool first = true; - while ((cmd = paths.next(x1, y1)) != T::end) + bool first_line_to = true; + std::uint32_t len; + #if defined(DEBUG) + std::uint32_t pre_len; + #endif + mapnik::box2d<double> part_env; + while ((cmd = paths.next(x1, y1, len)) != T::end) { if (cmd == T::move_to) { x2 = x1; y2 = y1; - if (first) first = false; - else rings.emplace_back(); + if (first) + { + first = false; + } + else + { + #if defined(DEBUG) + // off by one is expected/okay in rare cases + if (rings.back().size() > 0 && (rings.back().size() > pre_len || std::fabs(pre_len - rings.back().size()) > 1)) + { + MAPNIK_LOG_ERROR(read_rings) << "warning: encountered incorrectly encoded ring with " << rings.back().size() << " points but " << pre_len << " repeated commands"; + } + #endif + first_line_to = true; + if (!bbox.intersects(part_env)) + { + // remove last ring + rings.pop_back(); + } + rings.emplace_back(); + } + part_env.init(x1,y1,x1,y1); + } + else if (first_line_to && cmd == T::line_to) + { + first_line_to = false; + rings.back().reserve(len+2); + #if defined(DEBUG) + pre_len = len+2; + #endif } else if (cmd == T::close) { @@ -280,9 +391,17 @@ std::vector<mapnik::geometry::linear_ring<double>> read_rings(T & paths) } continue; } + if (!first) + { + part_env.expand_to_include(x1,y1); + } rings.back().add_coord(x1,y1); } - return rings; + if (!bbox.intersects(part_env)) + { + // remove last ring + rings.pop_back(); + } } template <typename T> @@ -291,7 +410,11 @@ void decode_polygons(mapnik::geometry::geometry<double> & geom, T && rings) auto rings_itr = std::make_move_iterator(rings.begin()); auto rings_end = std::make_move_iterator(rings.end()); std::size_t num_rings = rings.size(); - if (num_rings == 1) + if (num_rings == 0) + { + geom = mapnik::geometry::geometry_empty(); + } + else if (num_rings == 1) { if (rings_itr->size() < 4) return; if (mapnik::util::is_clockwise(*rings_itr)) @@ -367,25 +490,33 @@ void decode_polygons(mapnik::geometry::geometry<double> & geom, T && rings) } // ns detail template <typename T> -inline mapnik::geometry::geometry<double> decode_geometry(T & paths, int32_t geom_type) +inline mapnik::geometry::geometry<double> decode_geometry(T & paths, int32_t geom_type, mapnik::box2d<double> const& bbox) { mapnik::geometry::geometry<double> geom; // output geometry switch (geom_type) { case vector_tile::Tile_GeomType_POINT: { - detail::decode_point(geom, paths); + detail::decode_point(geom, paths, bbox); break; } case vector_tile::Tile_GeomType_LINESTRING: { - detail::decode_linestring(geom, paths); + detail::decode_linestring(geom, paths, bbox); break; } case vector_tile::Tile_GeomType_POLYGON: { - auto rings = detail::read_rings(paths); - detail::decode_polygons(geom, rings); + std::vector<mapnik::geometry::linear_ring<double>> rings; + detail::read_rings(rings, paths, bbox); + if (rings.empty()) + { + geom = mapnik::geometry::geometry_empty(); + } + else + { + detail::decode_polygons(geom, rings); + } break; } case vector_tile::Tile_GeomType_UNKNOWN: @@ -398,6 +529,19 @@ inline mapnik::geometry::geometry<double> decode_geometry(T & paths, int32_t geo return geom; } +// For back compatibility in tests / for cases where performance is not critical +// TODO: consider removing and always requiring bbox arg +template <typename T> +inline mapnik::geometry::geometry<double> decode_geometry(T & paths, int32_t geom_type) +{ + + mapnik::box2d<double> bbox(-std::numeric_limits<double>::max(), + -std::numeric_limits<double>::max(), + std::numeric_limits<double>::max(), + std::numeric_limits<double>::max()); + return decode_geometry(paths,geom_type,bbox); +} + }} // end ns diff --git a/src/vector_tile_geometry_encoder.hpp b/src/vector_tile_geometry_encoder.hpp index 4a2c71d..ea6edd1 100644 --- a/src/vector_tile_geometry_encoder.hpp +++ b/src/vector_tile_geometry_encoder.hpp @@ -140,22 +140,71 @@ inline unsigned encode_geometry(mapnik::geometry::linear_ring<std::int64_t> cons inline unsigned encode_geometry(mapnik::geometry::polygon<std::int64_t> const& poly, vector_tile::Tile_Feature & current_feature, - int32_t & x_, - int32_t & y_) + int32_t & start_x, + int32_t & start_y) { unsigned count = 0; - count += encode_geometry(poly.exterior_ring, current_feature, x_, y_); + count += encode_geometry(poly.exterior_ring, current_feature, start_x, start_y); if (count == 0) { return count; } for (auto const& ring : poly.interior_rings) { - count += encode_geometry(ring, current_feature, x_, y_); + count += encode_geometry(ring, current_feature, start_x, start_y); } return count; } +inline unsigned encode_geometry(mapnik::geometry::multi_point<std::int64_t> const& geom, + vector_tile::Tile_Feature & current_feature, + int32_t & start_x, + int32_t & start_y) +{ + std::size_t geom_size = geom.size(); + if (geom_size <= 0) + { + return 0; + } + current_feature.add_geometry(1u | (geom_size << 3)); // move_to | (len << 3) + for (auto const& pt : geom) + { + int32_t dx = pt.x - start_x; + int32_t dy = pt.y - start_y; + // Manual zigzag encoding. + current_feature.add_geometry(protozero::encode_zigzag32(dx)); + current_feature.add_geometry(protozero::encode_zigzag32(dy)); + start_x = pt.x; + start_y = pt.y; + } + return geom_size; +} + +inline unsigned encode_geometry(mapnik::geometry::multi_line_string<std::int64_t> const& geom, + vector_tile::Tile_Feature & current_feature, + int32_t & start_x, + int32_t & start_y) +{ + unsigned count = 0; + for (auto const& poly : geom) + { + count += encode_geometry(poly, current_feature, start_x, start_y); + } + return count; +} + +inline unsigned encode_geometry(mapnik::geometry::multi_polygon<std::int64_t> const& geom, + vector_tile::Tile_Feature & current_feature, + int32_t & start_x, + int32_t & start_y) +{ + unsigned count = 0; + for (auto const& poly : geom) + { + count += encode_geometry(poly, current_feature, start_x, start_y); + } + return count; +} }} // end ns diff --git a/src/vector_tile_processor.ipp b/src/vector_tile_processor.ipp index bf820e4..08554fb 100644 --- a/src/vector_tile_processor.ipp +++ b/src/vector_tile_processor.ipp @@ -924,19 +924,11 @@ struct encoder_visitor { unsigned operator() (mapnik::geometry::multi_point<std::int64_t> const& geom) { unsigned path_count = 0; - bool first = true; - for (auto const& pt : geom) - { - if (first) - { - first = false; - backend_.start_tile_feature(feature_); - backend_.current_feature_->set_type(vector_tile::Tile_GeomType_POINT); - } - path_count += backend_.add_path(pt); - } - if (!first) + if (!geom.empty()) { + backend_.start_tile_feature(feature_); + backend_.current_feature_->set_type(vector_tile::Tile_GeomType_POINT); + path_count = backend_.add_path(geom); backend_.stop_tile_feature(); } return path_count; diff --git a/src/vector_tile_util.ipp b/src/vector_tile_util.ipp index 4f5976c..b877af1 100644 --- a/src/vector_tile_util.ipp +++ b/src/vector_tile_util.ipp @@ -207,7 +207,8 @@ namespace mapnik { namespace vector_tile_impl { double x0, y0, x1, y1; mapnik::box2d<int> box; bool first = true; - while ((cmd = paths.next(x1, y1)) != mapnik::vector_tile_impl::GeometryPBF::end) + std::uint32_t len; + while ((cmd = paths.next(x1, y1, len)) != mapnik::vector_tile_impl::GeometryPBF::end) { if (cmd == mapnik::vector_tile_impl::GeometryPBF::move_to || cmd == mapnik::vector_tile_impl::GeometryPBF::line_to) { diff --git a/test/encoding_util.hpp b/test/encoding_util.hpp index 98b9fda..98e0466 100644 --- a/test/encoding_util.hpp +++ b/test/encoding_util.hpp @@ -29,59 +29,6 @@ struct print } -struct encode_geometry -{ - vector_tile::Tile_Feature & feature_; - int32_t x_; - int32_t y_; - encode_geometry(vector_tile::Tile_Feature & feature) : - feature_(feature), - x_(0), - y_(0) { } - - void operator() (geometry_empty const&) - { - } - - template <typename T> - void operator()(T const& path) - { - mapnik::vector_tile_impl::encode_geometry(path,feature_,x_,y_); - } - - void operator()(mapnik::geometry::multi_point<std::int64_t> const & path) - { - for (auto const& pt : path) - { - mapnik::vector_tile_impl::encode_geometry(pt,feature_,x_,y_); - } - } - - void operator()(mapnik::geometry::multi_line_string<std::int64_t> const & path) - { - for (auto const& ls : path) - { - mapnik::vector_tile_impl::encode_geometry(ls,feature_,x_,y_); - } - } - - void operator()(mapnik::geometry::multi_polygon<std::int64_t> const& path) - { - for (auto const& p : path) - { - mapnik::vector_tile_impl::encode_geometry(p,feature_,x_,y_); - } - } - - void operator()(mapnik::geometry::geometry_collection<std::int64_t> const& path) - { - for (auto const& p : path) - { - mapnik::util::apply_visitor((*this), p); - } - } -}; - struct show_path { std::string & str_; @@ -111,20 +58,47 @@ struct show_path } }; -template <typename T> -vector_tile::Tile_Feature geometry_to_feature(mapnik::geometry::geometry<T> const& g) +struct encoder_visitor +{ + vector_tile::Tile_Feature & feature_; + int32_t x_; + int32_t y_; + encoder_visitor(vector_tile::Tile_Feature & feature) : + feature_(feature), + x_(0), + y_(0) { } + + void operator() (geometry_empty const&) + { + } + + void operator()(mapnik::geometry::geometry_collection<std::int64_t> const& path) + { + for (auto const& p : path) + { + mapnik::util::apply_visitor((*this), p); + } + } + + template <typename T> + void operator()(T const& geom) + { + mapnik::vector_tile_impl::encode_geometry(geom,feature_,x_,y_); + } +}; + +vector_tile::Tile_Feature geometry_to_feature(mapnik::geometry::geometry<std::int64_t> const& g) { vector_tile::Tile_Feature feature; - encode_geometry ap(feature); - if (g.template is<mapnik::geometry::point<T> >() || g.template is<mapnik::geometry::multi_point<T> >()) + if (g.template is<mapnik::geometry::point<std::int64_t>>() || g.template is<mapnik::geometry::multi_point<std::int64_t>>()) { feature.set_type(vector_tile::Tile_GeomType_POINT); } - else if (g.template is<mapnik::geometry::line_string<T> >() || g.template is<mapnik::geometry::multi_line_string<T> >()) + else if (g.template is<mapnik::geometry::line_string<std::int64_t>>() || g.template is<mapnik::geometry::multi_line_string<std::int64_t>>()) { feature.set_type(vector_tile::Tile_GeomType_LINESTRING); } - else if (g.template is<mapnik::geometry::polygon<T> >() || g.template is<mapnik::geometry::multi_polygon<T> >()) + else if (g.template is<mapnik::geometry::polygon<std::int64_t>>() || g.template is<mapnik::geometry::multi_polygon<std::int64_t>>()) { feature.set_type(vector_tile::Tile_GeomType_POLYGON); } @@ -132,6 +106,7 @@ vector_tile::Tile_Feature geometry_to_feature(mapnik::geometry::geometry<T> cons { throw std::runtime_error("could not detect valid geometry type"); } + encoder_visitor ap(feature); mapnik::util::apply_visitor(ap,g); return feature; } @@ -147,8 +122,7 @@ std::string decode_to_path_string(mapnik::geometry::geometry<T> const& g) return out; } -template <typename T> -std::string compare(mapnik::geometry::geometry<T> const& g) +std::string compare(mapnik::geometry::geometry<std::int64_t> const& g) { vector_tile::Tile_Feature feature = geometry_to_feature(g); mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); diff --git a/test/geometry_encoding.cpp b/test/geometry_encoding.cpp index 4520f0e..948fb09 100644 --- a/test/geometry_encoding.cpp +++ b/test/geometry_encoding.cpp @@ -20,7 +20,7 @@ TEST_CASE( "point", "should round trip without changes" ) { std::string expected( "move_to(0,0)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } TEST_CASE( "multi_point", "should round trip without changes" ) { @@ -33,7 +33,7 @@ TEST_CASE( "multi_point", "should round trip without changes" ) { "move_to(1,1)\n" "move_to(2,2)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } TEST_CASE( "line_string", "should round trip without changes" ) { @@ -46,7 +46,7 @@ TEST_CASE( "line_string", "should round trip without changes" ) { "line_to(1,1)\n" "line_to(100,100)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } TEST_CASE( "multi_line_string", "should round trip without changes" ) { @@ -73,7 +73,7 @@ TEST_CASE( "multi_line_string", "should round trip without changes" ) { "line_to(-20,-20)\n" "line_to(-100,-100)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } /*TEST_CASE( "degenerate line_string", "should be culled" ) { @@ -86,7 +86,7 @@ TEST_CASE( "multi_line_string", "should round trip without changes" ) { std::string expected_wkt0("LINESTRING(10 10)"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(line); + vector_tile::Tile_Feature feature = geometry_to_feature(line); CHECK( feature.geometry_size() == 0 ); auto geom = mapnik::vector_tile_impl::decode_geometry(feature,0.0,0.0,1.0,1.0); CHECK( geom.is<mapnik::geometry::geometry_empty>() ); @@ -109,7 +109,7 @@ TEST_CASE( "multi_line_string with degenerate first part", "should be culled" ) std::string expected_wkt0("MULTILINESTRING((0 0),(2 2,3 3))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(g); + vector_tile::Tile_Feature feature = geometry_to_feature(g); CHECK( feature.geometry_size() == 6 ); auto geom = mapnik::vector_tile_impl::decode_geometry(feature,0.0,0.0,1.0,1.0); @@ -140,7 +140,7 @@ TEST_CASE( "multi_line_string with degenerate first part", "should be culled" ) std::string expected_wkt0("MULTILINESTRING((0 0,1 1,100 100),(-10 -10))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(g); + vector_tile::Tile_Feature feature = geometry_to_feature(g); CHECK( feature.type() == vector_tile::Tile_GeomType_LINESTRING ); CHECK( feature.geometry_size() == 8 ); auto geom = mapnik::vector_tile_impl::decode_geometry(feature,0.0,0.0,1.0,1.0); @@ -163,7 +163,7 @@ TEST_CASE( "polygon", "should round trip without changes" ) { "line_to(100,100)\n" "close_path(0,0)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } TEST_CASE( "polygon with degenerate exterior ring ", "should be culled" ) { @@ -178,7 +178,7 @@ TEST_CASE( "polygon with degenerate exterior ring ", "should be culled" ) { std::string expected_wkt0("POLYGON((0 0,0 10))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); // since first ring is degenerate the whole polygon should be culled mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); @@ -205,7 +205,7 @@ TEST_CASE( "polygon with degenerate exterior ring ", "should be culled" ) { std::string expected_wkt0("POLYGON((0 0,0 10),(-7 7,-3 7,-3 3,-7 3,-7 7))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); // since first ring is degenerate the whole polygon should be culled mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); @@ -231,7 +231,7 @@ TEST_CASE( "polygon with valid exterior ring but degenerate interior ring", "sho std::string expected_wkt0("POLYGON((0 0,0 10,-10 10,-10 0,0 0),(-7 7,-3 7))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); CHECK( p1.is<mapnik::geometry::polygon<double> >() ); @@ -272,7 +272,7 @@ TEST_CASE( "polygon with valid exterior ring but one degenerate interior ring of std::string expected_wkt0("POLYGON((0 0,0 10,-10 10,-10 0,0 0),(-7 7,-3 7),(-6 4,-6 6,-4 6,-4 4,-6 4))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); CHECK( p1.is<mapnik::geometry::polygon<double> >() ); @@ -306,7 +306,7 @@ TEST_CASE( "Polygon with de-generate ring(s)", "should skip invalid ring(s)" ) std::string expected_wkt1("POLYGON((-7 7,-7 3,-3 3,-3 7,-7 7))"); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); CHECK( p1.is<mapnik::geometry::polygon<double> >() ); @@ -346,7 +346,7 @@ TEST_CASE( "(multi)polygon with hole", "should round trip without changes" ) { CHECK( mapnik::util::to_wkt(wkt0,mapnik::geometry::geometry<std::int64_t>(p0)) ); CHECK( wkt0 == expected_wkt0); - vector_tile::Tile_Feature feature = geometry_to_feature<std::int64_t>(p0); + vector_tile::Tile_Feature feature = geometry_to_feature(p0); mapnik::vector_tile_impl::Geometry geoms(feature,0.0,0.0,1.0,1.0); auto p1 = mapnik::vector_tile_impl::decode_geometry(geoms, feature.type()); CHECK( p1.is<mapnik::geometry::polygon<double> >() ); @@ -373,7 +373,7 @@ TEST_CASE( "(multi)polygon with hole", "should round trip without changes" ) { "line_to(3,3)\n" "line_to(4,4)\n" ); - CHECK( compare<std::int64_t>(g) == expected); + CHECK( compare(g) == expected); }*/ /* @@ -387,7 +387,7 @@ TEST_CASE( "test 2b", "should drop vertices" ) { "line_to(0,0)\n" // TODO - should we try to drop this? "line_to(1,1)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); }*/ TEST_CASE( "test 3", "should not drop first move_to or last vertex in line" ) { @@ -407,7 +407,7 @@ TEST_CASE( "test 3", "should not drop first move_to or last vertex in line" ) { "move_to(2,2)\n" "line_to(3,3)\n" ); - CHECK(compare<std::int64_t>(g) == expected); + CHECK(compare(g) == expected); } /* -- Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-grass/mapnik-vector-tile.git _______________________________________________ Pkg-grass-devel mailing list Pkg-grass-devel@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-grass-devel