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 137d9fdb40429d6803f93e86ef937f707cf97b1a Author: Bas Couwenberg <sebas...@xs4all.nl> Date: Fri Sep 11 22:55:15 2015 +0200 Imported Upstream version 0.8.4+dfsg --- CHANGELOG.md | 8 + bench/vtile-transform.cpp | 25 ++- package.json | 2 +- src/vector_tile_backend_pbf.hpp | 11 +- src/vector_tile_geometry_decoder.hpp | 374 +++++++++++++++++------------------ src/vector_tile_processor.hpp | 12 +- src/vector_tile_processor.ipp | 191 ++++++++++-------- src/vector_tile_strategy.hpp | 55 +++++- test/clipper_test.cpp | 59 +++++- test/data/NZ_Coastline_NZMG.dbf | Bin 0 -> 143 bytes test/data/NZ_Coastline_NZMG.prj | 1 + test/data/NZ_Coastline_NZMG.qpj | 1 + test/data/NZ_Coastline_NZMG.shp | Bin 0 -> 1268 bytes test/data/NZ_Coastline_NZMG.shx | Bin 0 -> 116 bytes test/data/invalid-interior-ring.json | 95 +++++++++ test/geometry_encoding.cpp | 122 +++--------- test/test_utils.cpp | 16 ++ test/test_utils.hpp | 1 + test/vector_tile.cpp | 102 +++++++++- test/vector_tile_pbf.cpp | 20 +- 20 files changed, 692 insertions(+), 403 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad096c4..d38b756 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,15 @@ # Changelog +## 0.8.4 + + - Started to skip coordinates that are out of range (#121) + - Fix clipping box used when reprojecting data on the fly (#128) + - Fixed decoding of degenerate polygons - we need to gracefully support these as they + are commonly in the wild based on that AGG clipper used in v0.7.1 and earlier (#123) + ## 0.8.3 + - Started to skip coordinates that cannot be reprojected (#117) - Minor optimization in attribute encoding by using `emplace` instead of `insert` - Now depends on `pbf_writer.hpp` for zigzag implementation (no change in behavior) - Minor code cleanup to avoid unnecessary compiler warnings diff --git a/bench/vtile-transform.cpp b/bench/vtile-transform.cpp index 01cc25f..772a90d 100644 --- a/bench/vtile-transform.cpp +++ b/bench/vtile-transform.cpp @@ -46,8 +46,9 @@ int main() { unsigned count = 0; unsigned count2 = 0; + unsigned count3 = 0; { - mapnik::vector_tile_impl::vector_tile_strategy vs(prj_trans, tr, 16); + mapnik::vector_tile_impl::vector_tile_strategy vs(tr, 16); mapnik::progress_timer __stats__(std::clog, "boost::geometry::transform"); for (unsigned i=0;i<10000;++i) { @@ -57,9 +58,9 @@ int main() { } } { - mapnik::vector_tile_impl::vector_tile_strategy vs(prj_trans, tr, 16); - mapnik::progress_timer __stats__(std::clog, "transform_visitor with reserve"); - mapnik::vector_tile_impl::transform_visitor transit(vs); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs(prj_trans,tr, 16); + mapnik::progress_timer __stats__(std::clog, "transform_visitor with reserve with proj no-op"); + mapnik::vector_tile_impl::transform_visitor<mapnik::vector_tile_impl::vector_tile_strategy_proj> transit(vs); for (unsigned i=0;i<10000;++i) { mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::util::apply_visitor(transit,geom); @@ -72,5 +73,21 @@ int main() { return -1; } } + { + mapnik::vector_tile_impl::vector_tile_strategy vs(tr, 16); + mapnik::progress_timer __stats__(std::clog, "transform_visitor with reserve with no proj function call overhead"); + mapnik::vector_tile_impl::transform_visitor<mapnik::vector_tile_impl::vector_tile_strategy> transit(vs); + for (unsigned i=0;i<10000;++i) + { + mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::util::apply_visitor(transit,geom); + auto const& poly = mapnik::util::get<mapnik::geometry::multi_polygon<std::int64_t>>(new_geom); + count3 += poly.size(); + } + if (count != count3) + { + std::clog << "tests did not run as expected!\n"; + return -1; + } + } return 0; } \ No newline at end of file diff --git a/package.json b/package.json index 311f840..7dfbbd3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mapnik-vector-tile", - "version": "0.8.3", + "version": "0.8.4", "description": "Mapnik vector tile API", "main": "./package.json", "repository" : { diff --git a/src/vector_tile_backend_pbf.hpp b/src/vector_tile_backend_pbf.hpp index 3198e19..0090663 100644 --- a/src/vector_tile_backend_pbf.hpp +++ b/src/vector_tile_backend_pbf.hpp @@ -12,12 +12,11 @@ #include "vector_tile_geometry_encoder.hpp" #include <mapnik/value.hpp> #include <mapnik/geometry.hpp> +#include <unordered_map> -// boost -#include <boost/unordered_map.hpp> - -namespace mapnik { - class feature_impl; +namespace mapnik +{ +class feature_impl; } namespace mapnik { namespace vector_tile_impl { @@ -25,7 +24,7 @@ namespace mapnik { namespace vector_tile_impl { struct backend_pbf { typedef std::map<std::string, unsigned> keys_container; - typedef boost::unordered_map<mapnik::value, unsigned> values_container; + typedef std::unordered_map<mapnik::value, unsigned> values_container; private: vector_tile::Tile & tile_; unsigned path_multiplier_; diff --git a/src/vector_tile_geometry_decoder.hpp b/src/vector_tile_geometry_decoder.hpp index 1a5d9da..c3abdf8 100644 --- a/src/vector_tile_geometry_decoder.hpp +++ b/src/vector_tile_geometry_decoder.hpp @@ -10,7 +10,6 @@ #include "pbf_reader.hpp" #include <mapnik/util/is_clockwise.hpp> - //std #include <algorithm> @@ -55,12 +54,13 @@ public: double tile_x, double tile_y, double scale_x, double scale_y); - enum command : uint8_t { + enum command : uint8_t + { end = 0, - move_to = 1, - line_to = 2, - close = 7 - }; + move_to = 1, + line_to = 2, + close = 7 + }; inline command next(double& rx, double& ry); @@ -87,9 +87,12 @@ 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) { - if (k < geoms_) { - if (length == 0) { +Geometry::command Geometry::next(double& rx, double& ry) +{ + if (k < geoms_) + { + if (length == 0) + { uint32_t cmd_length = static_cast<uint32_t>(f_.geometry(k++)); cmd = cmd_length & 0x7; length = cmd_length >> 3; @@ -97,7 +100,8 @@ Geometry::command Geometry::next(double& rx, double& ry) { --length; - if (cmd == move_to || cmd == line_to) { + if (cmd == move_to || cmd == line_to) + { int32_t dx = f_.geometry(k++); int32_t dy = f_.geometry(k++); dx = ((dx >> 1) ^ (-(dx & 1))); @@ -113,11 +117,15 @@ Geometry::command Geometry::next(double& rx, double& ry) { } else { return line_to; } - } else if (cmd == close) { + } + else if (cmd == close) + { rx = ox; ry = oy; return close; - } else { + } + else + { fprintf(stderr, "unknown command: %d\n", cmd); return end; } @@ -137,9 +145,12 @@ GeometryPBF::GeometryPBF(std::pair<mapbox::util::pbf::const_uint32_iterator, map x(tile_x), y(tile_y), ox(0), oy(0) {} -GeometryPBF::command GeometryPBF::next(double& rx, double& ry) { - if (geo_iterator_.first != geo_iterator_.second) { - if (length == 0) { +GeometryPBF::command GeometryPBF::next(double& rx, double& ry) +{ + if (geo_iterator_.first != geo_iterator_.second) + { + if (length == 0) + { uint32_t cmd_length = static_cast<uint32_t>(*geo_iterator_.first++); cmd = cmd_length & 0x7; length = cmd_length >> 3; @@ -147,7 +158,8 @@ GeometryPBF::command GeometryPBF::next(double& rx, double& ry) { --length; - if (cmd == move_to || cmd == line_to) { + if (cmd == move_to || cmd == line_to) + { int32_t dx = *geo_iterator_.first++; int32_t dy = *geo_iterator_.first++; dx = ((dx >> 1) ^ (-(dx & 1))); @@ -156,244 +168,224 @@ GeometryPBF::command GeometryPBF::next(double& rx, double& ry) { y += (static_cast<double>(dy) / scale_y_); rx = x; ry = y; - if (cmd == move_to) { + if (cmd == move_to) + { ox = x; oy = y; return move_to; - } else { + } + else + { return line_to; } - } else if (cmd == close) { + } + else if (cmd == close) + { rx = ox; ry = oy; return close; - } else { + } + else + { fprintf(stderr, "unknown command: %d\n", cmd); return end; } - } else { + } + else + { return end; } } +namespace detail { + template <typename T> -inline mapnik::geometry::geometry<double> decode_geometry(T & geoms, int32_t geom_type, - bool treat_all_rings_as_exterior=false) +void decode_point(mapnik::geometry::geometry<double> & geom, T & paths) { typename T::command cmd; double x1, y1; - mapnik::geometry::geometry<double> geom; // output geometry - - switch (geom_type) + mapnik::geometry::multi_point<double> mp; + while ((cmd = paths.next(x1, y1)) != T::end) { - case vector_tile::Tile_GeomType_POINT: + mp.emplace_back(mapnik::geometry::point<double>(x1,y1)); + } + std::size_t num_points = mp.size(); + if (num_points == 1) { - mapnik::geometry::multi_point<double> mp; - while ((cmd = geoms.next(x1, y1)) != T::end) - { - mp.emplace_back(mapnik::geometry::point<double>(x1,y1)); - } - std::size_t num_points = mp.size(); - if (num_points == 1) - { - // return the single point - geom = std::move(mp[0]); - return geom; - } - else if (num_points > 1) + geom = std::move(mp[0]); + } + else if (num_points > 1) + { + // return multipoint + geom = std::move(mp); + } +} + +template <typename T> +void decode_linestring(mapnik::geometry::geometry<double> & geom, T & paths) +{ + 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) + { + if (cmd == T::move_to) { - // return multipoint - geom = std::move(mp); - return geom; + if (first) first = false; + else multi_line.emplace_back(); } - break; + multi_line.back().add_coord(x1,y1); } - case vector_tile::Tile_GeomType_LINESTRING: + std::size_t num_lines = multi_line.size(); + if (num_lines == 1) { - mapnik::geometry::multi_line_string<double> multi_line; - multi_line.emplace_back(); - bool first = true; - while ((cmd = geoms.next(x1, y1)) != T::end) + auto itr = std::make_move_iterator(multi_line.begin()); + if (itr->size() > 1) { - if (cmd == T::move_to) - { - if (first) - { - first = false; - } - else - { - multi_line.emplace_back(); - } - } - multi_line.back().add_coord(x1,y1); + geom = std::move(*itr); } - if (multi_line.empty()) + } + else if (num_lines > 1) + { + geom = std::move(multi_line); + } +} + +template <typename T> +std::vector<mapnik::geometry::linear_ring<double>> read_rings(T & paths) +{ + 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) + { + if (cmd == T::move_to) { - return geom; + x2 = x1; + y2 = y1; + if (first) first = false; + else rings.emplace_back(); } - std::size_t num_lines = multi_line.size(); - if (num_lines == 1) + else if (cmd == T::close) { - // return the single line - auto itr = std::make_move_iterator(multi_line.begin()); - if (itr->size() > 1) + auto & ring = rings.back(); + if (ring.size() > 2 && !(ring.back().x == x2 && ring.back().y == y2)) { - geom = std::move(*itr); + ring.add_coord(x2,y2); } - return geom; + continue; } - else if (num_lines > 1) + rings.back().add_coord(x1,y1); + } + return rings; +} + +template <typename T> +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 (rings_itr->size() < 4) return; + if (mapnik::util::is_clockwise(*rings_itr)) { - // return multiline - geom = std::move(multi_line); - return geom; + // Its clockwise, so lets reverse it. + std::reverse(rings_itr->begin(), rings_itr->end()); } - break; + // return the single polygon without interior rings + mapnik::geometry::polygon<double> poly; + poly.set_exterior_ring(std::move(*rings_itr)); + geom = std::move(poly); } - case vector_tile::Tile_GeomType_POLYGON: + else { - std::vector<mapnik::geometry::linear_ring<double>> rings; - rings.emplace_back(); - double x2,y2; + mapnik::geometry::multi_polygon<double> multi_poly; bool first = true; - while ((cmd = geoms.next(x1, y1)) != T::end) + bool is_clockwise = true; + for (; rings_itr != rings_end; ++rings_itr) { - if (cmd == T::move_to) + if (rings_itr->size() < 4) continue; // skip degenerate rings + if (first) { - x2 = x1; - y2 = y1; - if (first) - { - first = false; - } - else + is_clockwise = mapnik::util::is_clockwise(*rings_itr); + // first ring always exterior and sets all future winding order + multi_poly.emplace_back(); + if (is_clockwise) { - rings.emplace_back(); + // Going into mapnik we want the outer ring to be CCW + std::reverse(rings_itr->begin(), rings_itr->end()); } + multi_poly.back().set_exterior_ring(std::move(*rings_itr)); + first = false; } - else if (cmd == T::close) - { - rings.back().add_coord(x2,y2); - continue; - } - rings.back().add_coord(x1,y1); - } - - 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 (rings_itr->size() < 4) - { - return geom; - } - if (mapnik::util::is_clockwise(*rings_itr)) + else if (is_clockwise == mapnik::util::is_clockwise(*rings_itr)) { - // Its clockwise, so lets reverse it. - std::reverse(rings_itr->begin(), rings_itr->end()); - } - // return the single polygon without interior rings - mapnik::geometry::polygon<double> poly; - poly.set_exterior_ring(std::move(*rings_itr)); - geom = std::move(poly); - return geom; - } - - // Multiple rings represent either: - // 1) a polygon with interior ring(s) - // 2) a multipolygon with polygons with no interior ring(s) - // 3) a multipolygon with polygons with interior ring(s) - mapnik::geometry::multi_polygon<double> multi_poly; - first = true; - // back compatibility mode to previous Mapnik (pre new geometry) - // which pushed all rings into single path - if (treat_all_rings_as_exterior) - { - for (; rings_itr != rings_end; ++rings_itr) - { - bool degenerate_ring = (rings_itr->size() < 4); - if (degenerate_ring) continue; - multi_poly.emplace_back(); - if (mapnik::util::is_clockwise(*rings_itr)) + // hit a new exterior ring, so start a new polygon + multi_poly.emplace_back(); // start new polygon + if (is_clockwise) { - // Its clockwise, so lets reverse it. + // Going into mapnik we want the outer ring to be CCW, + // since first winding order was CW, we need to reverse + // these rings. std::reverse(rings_itr->begin(), rings_itr->end()); } multi_poly.back().set_exterior_ring(std::move(*rings_itr)); } - } - else - { - bool exterior_was_degenerate = false; - bool first_winding_order = true; - for (; rings_itr != rings_end; ++rings_itr) + else { - bool degenerate_ring = (rings_itr->size() < 4); - if (first) - { - if (degenerate_ring) - { - exterior_was_degenerate = true; - continue; - } - first_winding_order = mapnik::util::is_clockwise(*rings_itr); - // first ring always exterior and sets all future winding order - multi_poly.emplace_back(); - if (first_winding_order) - { - // Going into mapnik we want the outer ring to be CCW - std::reverse(rings_itr->begin(), rings_itr->end()); - } - multi_poly.back().set_exterior_ring(std::move(*rings_itr)); - first = false; - } - else if (first_winding_order == mapnik::util::is_clockwise(*rings_itr)) - { - if (degenerate_ring) continue; - // hit a new exterior ring, so start a new polygon - multi_poly.emplace_back(); // start new polygon - if (first_winding_order) - { - // Going into mapnik we want the outer ring to be CCW, - // since first winding order was CW, we need to reverse - // these rings. - std::reverse(rings_itr->begin(), rings_itr->end()); - } - multi_poly.back().set_exterior_ring(std::move(*rings_itr)); - exterior_was_degenerate = false; - } - else + if (is_clockwise) { - if (exterior_was_degenerate || degenerate_ring) continue; - if (first_winding_order) - { - // Going into mapnik we want the inner ring to be CW, - // since first winding order of the outer ring CW, we - // need to reverse these rings as they are CCW. - std::reverse(rings_itr->begin(), rings_itr->end()); - } - multi_poly.back().add_hole(std::move(*rings_itr)); + // Going into mapnik we want the inner ring to be CW, + // since first winding order of the outer ring CW, we + // need to reverse these rings as they are CCW. + std::reverse(rings_itr->begin(), rings_itr->end()); } + multi_poly.back().add_hole(std::move(*rings_itr)); } } + auto num_poly = multi_poly.size(); - if (num_poly == 0) - { - return geom; - } - else if (num_poly == 1) + if (num_poly == 1) { auto itr = std::make_move_iterator(multi_poly.begin()); geom = mapnik::geometry::polygon<double>(std::move(*itr)); - return geom; } else { geom = std::move(multi_poly); - return geom; } + } +} + +} // ns detail + +template <typename T> +inline mapnik::geometry::geometry<double> decode_geometry(T & paths, int32_t geom_type) +{ + mapnik::geometry::geometry<double> geom; // output geometry + switch (geom_type) + { + case vector_tile::Tile_GeomType_POINT: + { + detail::decode_point(geom, paths); + break; + } + case vector_tile::Tile_GeomType_LINESTRING: + { + detail::decode_linestring(geom, paths); + break; + } + case vector_tile::Tile_GeomType_POLYGON: + { + auto rings = detail::read_rings(paths); + detail::decode_polygons(geom, rings); break; } case vector_tile::Tile_GeomType_UNKNOWN: diff --git a/src/vector_tile_processor.hpp b/src/vector_tile_processor.hpp index 3cd41ea..2bf2808 100644 --- a/src/vector_tile_processor.hpp +++ b/src/vector_tile_processor.hpp @@ -63,6 +63,11 @@ public: return simplify_distance_; } + inline mapnik::view_transform const& get_transform() + { + return t_; + } + MAPNIK_VECTOR_INLINE void apply(double scale_denom=0.0); MAPNIK_VECTOR_INLINE bool painted() const; @@ -76,10 +81,11 @@ public: box2d<double> const& extent, int buffer_size); - MAPNIK_VECTOR_INLINE unsigned handle_geometry(mapnik::feature_impl const& feature, + template <typename T2> + MAPNIK_VECTOR_INLINE unsigned handle_geometry(T2 const& vs, + mapnik::feature_impl const& feature, mapnik::geometry::geometry<double> const& geom, - mapnik::proj_transform const& prj_trans, - mapnik::box2d<double> const& buffered_query_ext); + mapnik::box2d<int> const& tile_clipping_extent); }; }} // end ns diff --git a/src/vector_tile_processor.ipp b/src/vector_tile_processor.ipp index 31ea8bf..72da363 100644 --- a/src/vector_tile_processor.ipp +++ b/src/vector_tile_processor.ipp @@ -640,7 +640,7 @@ bool processor<T>::painted() const template <typename T> void processor<T>::apply_to_layer(mapnik::layer const& lay, - mapnik::projection const& proj0, + mapnik::projection const& target_proj, double scale, double scale_denom, unsigned width, @@ -651,10 +651,21 @@ void processor<T>::apply_to_layer(mapnik::layer const& lay, mapnik::datasource_ptr ds = lay.datasource(); if (!ds) return; - mapnik::projection proj1(lay.srs(),true); - mapnik::proj_transform prj_trans(proj0,proj1); - box2d<double> query_ext = extent; // unbuffered - box2d<double> buffered_query_ext(query_ext); // buffered + mapnik::projection source_proj(lay.srs(),true); + + // set up a transform from target to source + // target == final map (aka tile) projection, usually epsg:3857 + // source == projection of the data being queried + mapnik::proj_transform prj_trans(target_proj,source_proj); + + // working version of unbuffered extent + box2d<double> query_ext(extent); + + // working version of buffered extent + box2d<double> buffered_query_ext(query_ext); + + // transform the user-driven buffer size into the right + // size buffer into the target projection double buffer_padding = 2.0 * scale; boost::optional<int> layer_buffer_size = lay.buffer_size(); if (layer_buffer_size) // if layer overrides buffer size, use this value to compute buffered extent @@ -667,19 +678,29 @@ void processor<T>::apply_to_layer(mapnik::layer const& lay, } buffered_query_ext.width(query_ext.width() + buffer_padding); buffered_query_ext.height(query_ext.height() + buffer_padding); + + // ^^ now `buffered_query_ext` is actually buffered out. + // clip buffered extent by maximum extent, if supplied - boost::optional<box2d<double> > const& maximum_extent = m_.maximum_extent(); + // Note: Carto.js used to set this by default but no longer does after: + // https://github.com/mapbox/carto/pull/342 + boost::optional<box2d<double>> const& maximum_extent = m_.maximum_extent(); if (maximum_extent) { buffered_query_ext.clip(*maximum_extent); } + + // buffered_query_ext is transformed below + // into the coordinate system of the source data + // so grab a pristine copy of it to use later + box2d<double> target_clipping_extent(buffered_query_ext); + mapnik::box2d<double> layer_ext = lay.envelope(); - bool fw_success = false; bool early_return = false; // first, try intersection of map extent forward projected into layer srs if (prj_trans.forward(buffered_query_ext, PROJ_ENVELOPE_POINTS) && buffered_query_ext.intersects(layer_ext)) { - fw_success = true; + // this modifies the layer_ext by clipping to the buffered_query_ext layer_ext.clip(buffered_query_ext); } // if no intersection and projections are also equal, early return @@ -709,28 +730,6 @@ void processor<T>::apply_to_layer(mapnik::layer const& lay, return; } - // if we've got this far, now prepare the unbuffered extent - // which is used as a bbox for clipping geometries - if (maximum_extent) - { - query_ext.clip(*maximum_extent); - } - mapnik::box2d<double> layer_ext2 = lay.envelope(); - if (fw_success) - { - if (prj_trans.forward(query_ext, PROJ_ENVELOPE_POINTS)) - { - layer_ext2.clip(query_ext); - } - } - else - { - if (prj_trans.backward(layer_ext2, PROJ_ENVELOPE_POINTS)) - { - layer_ext2.clip(query_ext); - prj_trans.forward(layer_ext2, PROJ_ENVELOPE_POINTS); - } - } double qw = query_ext.width()>0 ? query_ext.width() : 1; double qh = query_ext.height()>0 ? query_ext.height() : 1; mapnik::query::resolution_type res(width/qw, @@ -784,22 +783,58 @@ void processor<T>::apply_to_layer(mapnik::layer const& lay, return; } // vector pathway - while (feature) + if (prj_trans.equal()) { - mapnik::geometry::geometry<double> const& geom = feature->get_geometry(); - if (mapnik::geometry::is_empty(geom)) + mapnik::vector_tile_impl::vector_tile_strategy vs(t_,backend_.get_path_multiplier()); + mapnik::geometry::point<double> p1_min(target_clipping_extent.minx(), target_clipping_extent.miny()); + mapnik::geometry::point<double> p1_max(target_clipping_extent.maxx(), target_clipping_extent.maxy()); + mapnik::geometry::point<std::int64_t> p2_min = mapnik::geometry::transform<std::int64_t>(p1_min, vs); + mapnik::geometry::point<std::int64_t> p2_max = mapnik::geometry::transform<std::int64_t>(p1_max, vs); + box2d<int> tile_clipping_extent(p2_min.x, p2_min.y, p2_max.x, p2_max.y); + while (feature) { + mapnik::geometry::geometry<double> const& geom = feature->get_geometry(); + if (mapnik::geometry::is_empty(geom)) + { + feature = features->next(); + continue; + } + if (handle_geometry(vs, + *feature, + geom, + tile_clipping_extent) > 0) + { + painted_ = true; + } feature = features->next(); - continue; } - if (handle_geometry(*feature, - geom, - prj_trans, - buffered_query_ext) > 0) + } + else + { + mapnik::vector_tile_impl::vector_tile_strategy vs(t_,backend_.get_path_multiplier()); + mapnik::geometry::point<double> p1_min(target_clipping_extent.minx(), target_clipping_extent.miny()); + mapnik::geometry::point<double> p1_max(target_clipping_extent.maxx(), target_clipping_extent.maxy()); + mapnik::geometry::point<std::int64_t> p2_min = mapnik::geometry::transform<std::int64_t>(p1_min, vs); + mapnik::geometry::point<std::int64_t> p2_max = mapnik::geometry::transform<std::int64_t>(p1_max, vs); + box2d<int> tile_clipping_extent(p2_min.x, p2_min.y, p2_max.x, p2_max.y); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs2(prj_trans,t_,backend_.get_path_multiplier()); + while (feature) { - painted_ = true; + mapnik::geometry::geometry<double> const& geom = feature->get_geometry(); + if (mapnik::geometry::is_empty(geom)) + { + feature = features->next(); + continue; + } + if (handle_geometry(vs2, + *feature, + geom, + tile_clipping_extent) > 0) + { + painted_ = true; + } + feature = features->next(); } - feature = features->next(); } backend_.stop_tile_layer(); } @@ -852,11 +887,11 @@ struct encoder_visitor { typedef T backend_type; encoder_visitor(backend_type & backend, mapnik::feature_impl const& feature, - mapnik::box2d<int> const& buffered_query_ext, + mapnik::box2d<int> const& tile_clipping_extent, double area_threshold) : backend_(backend), feature_(feature), - buffered_query_ext_(buffered_query_ext), + tile_clipping_extent_(tile_clipping_extent), area_threshold_(area_threshold) {} unsigned operator() (mapnik::geometry::geometry_empty const&) @@ -877,7 +912,7 @@ struct encoder_visitor { unsigned operator() (mapnik::geometry::point<std::int64_t> const& geom) { unsigned path_count = 0; - if (buffered_query_ext_.intersects(geom.x,geom.y)) + if (tile_clipping_extent_.intersects(geom.x,geom.y)) { backend_.start_tile_feature(feature_); backend_.current_feature_->set_type(vector_tile::Tile_GeomType_POINT); @@ -893,7 +928,7 @@ struct encoder_visitor { bool first = true; for (auto const& pt : geom) { - if (buffered_query_ext_.intersects(pt.x,pt.y)) + if (tile_clipping_extent_.intersects(pt.x,pt.y)) { if (first) { @@ -917,11 +952,11 @@ struct encoder_visitor { if (geom.size() < 2) return 0; std::deque<mapnik::geometry::line_string<int64_t>> result; mapnik::geometry::linear_ring<std::int64_t> clip_box; - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); boost::geometry::intersection(clip_box,geom,result); if (!result.empty()) { @@ -940,11 +975,11 @@ struct encoder_visitor { { unsigned path_count = 0; mapnik::geometry::linear_ring<std::int64_t> clip_box; - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); bool first = true; for (auto const& line : geom) { @@ -980,12 +1015,12 @@ struct encoder_visitor { unsigned path_count = 0; if (geom.exterior_ring.size() < 3) return 0; double clean_distance = 1.415; - mapnik::geometry::line_string<std::int64_t> clip_box; - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); + mapnik::geometry::linear_ring<std::int64_t> clip_box; + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); ClipperLib::Clipper clipper; ClipperLib::CleanPolygon(geom.exterior_ring, clean_distance); double outer_area = ClipperLib::Area(geom.exterior_ring); @@ -1070,12 +1105,12 @@ struct encoder_visitor { if (geom.empty()) return 0; double clean_distance = 1.415; - mapnik::geometry::line_string<std::int64_t> clip_box; - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.miny()); - clip_box.emplace_back(buffered_query_ext_.maxx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.maxy()); - clip_box.emplace_back(buffered_query_ext_.minx(),buffered_query_ext_.miny()); + mapnik::geometry::linear_ring<std::int64_t> clip_box; + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.miny()); + clip_box.emplace_back(tile_clipping_extent_.maxx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.maxy()); + clip_box.emplace_back(tile_clipping_extent_.minx(),tile_clipping_extent_.miny()); ClipperLib::Clipper clipper; for (auto & poly : geom) { @@ -1160,7 +1195,7 @@ struct encoder_visitor { backend_type & backend_; mapnik::feature_impl const& feature_; - mapnik::box2d<int> const& buffered_query_ext_; + mapnik::box2d<int> const& tile_clipping_extent_; double area_threshold_; }; @@ -1230,21 +1265,19 @@ struct simplify_visitor { }; -template <typename T> -unsigned processor<T>::handle_geometry(mapnik::feature_impl const& feature, +template <typename T> template <typename T2> +unsigned processor<T>::handle_geometry(T2 const& vs, + mapnik::feature_impl const& feature, mapnik::geometry::geometry<double> const& geom, - mapnik::proj_transform const& prj_trans, - mapnik::box2d<double> const& buffered_query_ext) + mapnik::box2d<int> const& tile_clipping_extent) { - vector_tile_strategy vs(prj_trans, t_, backend_.get_path_multiplier()); - mapnik::geometry::point<double> p1_min(buffered_query_ext.minx(), buffered_query_ext.miny()); - mapnik::geometry::point<double> p1_max(buffered_query_ext.maxx(), buffered_query_ext.maxy()); - mapnik::geometry::point<std::int64_t> p2_min = mapnik::geometry::transform<std::int64_t>(p1_min, vs); - mapnik::geometry::point<std::int64_t> p2_max = mapnik::geometry::transform<std::int64_t>(p1_max, vs); - box2d<int> bbox(p2_min.x, p2_min.y, p2_max.x, p2_max.y); - mapnik::vector_tile_impl::transform_visitor skipping_transformer(vs); + // TODO + // - no need to create a new skipping_transformer per geometry + // - write a non-skipping / zero copy transformer to be used when no projection is needed + using vector_tile_strategy_type = T2; + mapnik::vector_tile_impl::transform_visitor<vector_tile_strategy_type> skipping_transformer(vs); mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::util::apply_visitor(skipping_transformer,geom); - encoder_visitor<T> encoder(backend_,feature,bbox, area_threshold_); + encoder_visitor<T> encoder(backend_, feature, tile_clipping_extent, area_threshold_); if (simplify_distance_ > 0) { simplify_visitor<T> simplifier(simplify_distance_,encoder); diff --git a/src/vector_tile_strategy.hpp b/src/vector_tile_strategy.hpp index aa80caa..27a83dc 100644 --- a/src/vector_tile_strategy.hpp +++ b/src/vector_tile_strategy.hpp @@ -7,6 +7,8 @@ #include <mapnik/proj_transform.hpp> #include <mapnik/view_transform.hpp> +#include "clipper.hpp" + #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" #pragma GCC diagnostic ignored "-Wunused-local-typedef" @@ -20,9 +22,47 @@ namespace mapnik { namespace vector_tile_impl { +static constexpr double coord_max = static_cast<double>(ClipperLib::hiRange); +static constexpr double coord_min = -1 * static_cast<double>(ClipperLib::hiRange); + struct vector_tile_strategy { - vector_tile_strategy(proj_transform const& prj_trans, + vector_tile_strategy(view_transform const& tr, + double scaling) + : tr_(tr), + scaling_(scaling) {} + + template <typename P1, typename P2> + inline bool apply(P1 const& p1, P2 & p2) const + { + using p2_type = typename boost::geometry::coordinate_type<P2>::type; + double x = boost::geometry::get<0>(p1); + double y = boost::geometry::get<1>(p1); + tr_.forward(&x,&y); + x = std::round(x * scaling_); + y = std::round(y * scaling_); + if (x <= coord_min || x >= coord_max || + y <= coord_min || y >= coord_max) return false; + boost::geometry::set<0>(p2, static_cast<p2_type>(x)); + boost::geometry::set<1>(p2, static_cast<p2_type>(y)); + return true; + } + + template <typename P1, typename P2> + inline P2 execute(P1 const& p1, bool & status) const + { + P2 p2; + status = apply(p1, p2); + return p2; + } + + view_transform const& tr_; + double const scaling_; +}; + +struct vector_tile_strategy_proj +{ + vector_tile_strategy_proj(proj_transform const& prj_trans, view_transform const& tr, double scaling) : prj_trans_(prj_trans), @@ -39,8 +79,12 @@ struct vector_tile_strategy double z = 0.0; if (not_equal_ && !prj_trans_.backward(x, y, z)) return false; tr_.forward(&x,&y); - boost::geometry::set<0>(p2, static_cast<p2_type>(std::round(x * scaling_))); - boost::geometry::set<1>(p2, static_cast<p2_type>(std::round(y * scaling_))); + x = std::round(x * scaling_); + y = std::round(y * scaling_); + if (x <= coord_min || x >= coord_max || + y <= coord_min || y >= coord_max) return false; + boost::geometry::set<0>(p2, static_cast<p2_type>(x)); + boost::geometry::set<1>(p2, static_cast<p2_type>(y)); return true; } @@ -59,9 +103,10 @@ struct vector_tile_strategy }; // TODO - avoid creating degenerate polygons when first/last point of ring is skipped +template <typename TransformType> struct transform_visitor { - transform_visitor(vector_tile_strategy const& tr) : + transform_visitor(TransformType const& tr) : tr_(tr) {} inline mapnik::geometry::geometry<std::int64_t> operator() (mapnik::geometry::point<double> const& geom) @@ -201,7 +246,7 @@ struct transform_visitor { { return geom; } - vector_tile_strategy const& tr_; + TransformType const& tr_; }; } diff --git a/test/clipper_test.cpp b/test/clipper_test.cpp index e3ae16a..fcbdd0c 100644 --- a/test/clipper_test.cpp +++ b/test/clipper_test.cpp @@ -7,6 +7,7 @@ #include "vector_tile_strategy.hpp" #include "vector_tile_projection.hpp" +#include "test_utils.hpp" #include "catch.hpp" #include "clipper.hpp" @@ -20,7 +21,7 @@ TEST_CASE( "vector_tile_strategy", "should not overflow" ) { mapnik::box2d<double> z15_extent(minx,miny,maxx,maxy); mapnik::view_transform tr(tile_size,tile_size,z15_extent,0,0); { - mapnik::vector_tile_impl::vector_tile_strategy vs(prj_trans, tr, 16); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs(prj_trans, tr, 16); // even an invalid point is not expected to result in values beyond hirange mapnik::geometry::point<double> g(-20037508.342789*2.0,-20037508.342789*2.0); mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::geometry::transform<std::int64_t>(g, vs); @@ -41,7 +42,7 @@ TEST_CASE( "vector_tile_strategy", "should not overflow" ) { { // absurdly large but still should not result in values beyond hirange double path_multiplier = 100000000000.0; - mapnik::vector_tile_impl::vector_tile_strategy vs(prj_trans, tr, path_multiplier); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs(prj_trans, tr, path_multiplier); mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::geometry::transform<std::int64_t>(g, vs); REQUIRE( new_geom.is<mapnik::geometry::polygon<std::int64_t>>() ); auto const& poly = mapnik::util::get<mapnik::geometry::polygon<std::int64_t>>(new_geom); @@ -58,19 +59,59 @@ TEST_CASE( "vector_tile_strategy", "should not overflow" ) { { // expected to trigger values above hirange double path_multiplier = 1000000000000.0; - mapnik::vector_tile_impl::vector_tile_strategy vs(prj_trans, tr, path_multiplier); - mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::geometry::transform<std::int64_t>(g, vs); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs(prj_trans, tr, path_multiplier); + CHECK_THROWS( mapnik::geometry::transform<std::int64_t>(g, vs) ); + mapnik::vector_tile_impl::transform_visitor<mapnik::vector_tile_impl::vector_tile_strategy_proj> skipping_transformer(vs); + mapnik::geometry::geometry<std::int64_t> new_geom = skipping_transformer(g); REQUIRE( new_geom.is<mapnik::geometry::polygon<std::int64_t>>() ); auto const& poly = mapnik::util::get<mapnik::geometry::polygon<std::int64_t>>(new_geom); for (auto const& pt : poly.exterior_ring) { INFO( pt.x ) INFO( ClipperLib::hiRange ) - REQUIRE(( (pt.x > ClipperLib::hiRange) || - (pt.y > ClipperLib::hiRange) || - (-pt.x < ClipperLib::hiRange) || - (-pt.y < ClipperLib::hiRange) - )); + REQUIRE( (pt.x < ClipperLib::hiRange) ); + REQUIRE( (pt.y < ClipperLib::hiRange) ); + REQUIRE( (-pt.x < ClipperLib::hiRange) ); + REQUIRE( (-pt.y < ClipperLib::hiRange) ); + } + } +} + +TEST_CASE( "vector_tile_strategy2", "invalid mercator coord in interior ring" ) { + mapnik::geometry::geometry<double> geom = testing::read_geojson("./test/data/invalid-interior-ring.json"); + mapnik::projection longlat("+init=epsg:4326",true); + mapnik::proj_transform prj_trans(longlat,longlat); // no-op + unsigned tile_size = 256; + mapnik::vector_tile_impl::spherical_mercator merc_tiler(tile_size); + double minx,miny,maxx,maxy; + merc_tiler.xyz(9664,20435,15,minx,miny,maxx,maxy); + mapnik::box2d<double> z15_extent(minx,miny,maxx,maxy); + mapnik::view_transform tr(tile_size,tile_size,z15_extent,0,0); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs(prj_trans, tr, 16); + CHECK_THROWS( mapnik::geometry::transform<std::int64_t>(geom, vs) ); + mapnik::vector_tile_impl::transform_visitor<mapnik::vector_tile_impl::vector_tile_strategy_proj> skipping_transformer(vs); + mapnik::geometry::geometry<std::int64_t> new_geom = mapnik::util::apply_visitor(skipping_transformer,geom); + REQUIRE( new_geom.is<mapnik::geometry::polygon<std::int64_t>>() ); + auto const& poly = mapnik::util::get<mapnik::geometry::polygon<std::int64_t>>(new_geom); + for (auto const& pt : poly.exterior_ring) + { + INFO( pt.x ) + INFO( ClipperLib::hiRange ) + REQUIRE( (pt.x < ClipperLib::hiRange) ); + REQUIRE( (pt.y < ClipperLib::hiRange) ); + REQUIRE( (-pt.x < ClipperLib::hiRange) ); + REQUIRE( (-pt.y < ClipperLib::hiRange) ); + } + for (auto const& ring : poly.interior_rings) + { + for (auto const& pt : ring) + { + INFO( pt.x ) + INFO( ClipperLib::hiRange ) + REQUIRE( (pt.x < ClipperLib::hiRange) ); + REQUIRE( (pt.y < ClipperLib::hiRange) ); + REQUIRE( (-pt.x < ClipperLib::hiRange) ); + REQUIRE( (-pt.y < ClipperLib::hiRange) ); } } } diff --git a/test/data/NZ_Coastline_NZMG.dbf b/test/data/NZ_Coastline_NZMG.dbf new file mode 100644 index 0000000..1c9e979 Binary files /dev/null and b/test/data/NZ_Coastline_NZMG.dbf differ diff --git a/test/data/NZ_Coastline_NZMG.prj b/test/data/NZ_Coastline_NZMG.prj new file mode 100644 index 0000000..6a4197f --- /dev/null +++ b/test/data/NZ_Coastline_NZMG.prj @@ -0,0 +1 @@ +PROJCS["NZGD49_New_Zealand_Map_Grid",GEOGCS["GCS_New_Zealand_1949",DATUM["D_New_Zealand_1949",SPHEROID["International_1924",6378388,297]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]],PROJECTION["New_Zealand_Map_Grid"],PARAMETER["latitude_of_origin",-41],PARAMETER["Longitude_Of_Origin",173],PARAMETER["false_easting",2510000],PARAMETER["false_northing",6023150],UNIT["Meter",1]] \ No newline at end of file diff --git a/test/data/NZ_Coastline_NZMG.qpj b/test/data/NZ_Coastline_NZMG.qpj new file mode 100644 index 0000000..62479e3 --- /dev/null +++ b/test/data/NZ_Coastline_NZMG.qpj @@ -0,0 +1 @@ +PROJCS["NZGD49 / New Zealand Map Grid",GEOGCS["NZGD49",DATUM["New_Zealand_Geodetic_Datum_1949",SPHEROID["International 1924",6378388,297,AUTHORITY["EPSG","7022"]],TOWGS84[59.47,-5.04,187.44,0.47,-0.1,1.024,-4.5993],AUTHORITY["EPSG","6272"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4272"]],PROJECTION["New_Zealand_Map_Grid"],PARAMETER["latitude_of_origin",-41],PARAMETER["central_meridian",173],PARAMETER["fal [...] diff --git a/test/data/NZ_Coastline_NZMG.shp b/test/data/NZ_Coastline_NZMG.shp new file mode 100644 index 0000000..872179f Binary files /dev/null and b/test/data/NZ_Coastline_NZMG.shp differ diff --git a/test/data/NZ_Coastline_NZMG.shx b/test/data/NZ_Coastline_NZMG.shx new file mode 100644 index 0000000..ec190af Binary files /dev/null and b/test/data/NZ_Coastline_NZMG.shx differ diff --git a/test/data/invalid-interior-ring.json b/test/data/invalid-interior-ring.json new file mode 100644 index 0000000..99c75e4 --- /dev/null +++ b/test/data/invalid-interior-ring.json @@ -0,0 +1,95 @@ +{ + "type": "Polygon", + "coordinates": [ + [ + [ + -17.2265625, + 63.074865690586634 + ], + [ + -40.78125, + 45.583289756006316 + ], + [ + -47.109375, + 20.3034175184893 + ], + [ + -42.5390625, + 2.811371193331128 + ], + [ + -15.468749999999998, + 2.811371193331128 + ], + [ + 10.8984375, + 16.29905101458183 + ], + [ + 17.578125, + 38.272688535980976 + ], + [ + 16.5234375, + 56.36525013685606 + ], + [ + 1.0546875, + 65.5129625532949 + ], + [ + -17.2265625, + 63.074865690586634 + ] + ], + [ + [ + -10.8984375, + 54.77534585936447 + ], + [ + -23.5546875, + 32.24997445586331 + ], + [ + -10.8984375, + 18.646245142670608 + ], + [ + 4.21875, + 35.17380831799959 + ], + [ + 4.21875, + 46.800059446787316 + ], + [ + -10.8984375, + 54.77534585936447 + ] + ], + [ + [ + -10.1953125, + 44.33956524809713 + ], + [ + -12.65625, + 0.0 + ], + [ + 29256264171659283709034496, + 32.84267363195431 + ], + [ + -3.515625, + 42.032974332441405 + ], + [ + -10.1953125, + 44.33956524809713 + ] + ] + ] +} \ No newline at end of file diff --git a/test/geometry_encoding.cpp b/test/geometry_encoding.cpp index 69a94eb..4520f0e 100644 --- a/test/geometry_encoding.cpp +++ b/test/geometry_encoding.cpp @@ -284,6 +284,38 @@ TEST_CASE( "polygon with valid exterior ring but one degenerate interior ring of CHECK( holes.size() == 1 ); } +TEST_CASE( "Polygon with de-generate ring(s)", "should skip invalid ring(s)" ) +{ + // create invalid (exterior) polygon + mapnik::geometry::polygon<std::int64_t> p0; + p0.exterior_ring.add_coord(10,10); + p0.exterior_ring.add_coord(10,10); + p0.exterior_ring.add_coord(10,10); + p0.exterior_ring.add_coord(10,10); + mapnik::geometry::linear_ring<std::int64_t> hole; + hole.add_coord(-7,7); + hole.add_coord(-3,7); + hole.add_coord(-3,3); + hole.add_coord(-7,3); + hole.add_coord(-7,7); + p0.add_hole(std::move(hole)); + + std::string wkt0; + CHECK( mapnik::util::to_wkt(wkt0,mapnik::geometry::geometry<std::int64_t>(p0) ) ); + std::string expected_wkt0("POLYGON((10 10,10 10,10 10,10 10),(-7 7,-3 7,-3 3,-7 3,-7 7))"); + 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); + 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> >() ); + wkt0.clear(); + CHECK( mapnik::util::to_wkt(wkt0,p1) ); + CHECK( wkt0 == expected_wkt1); +} + + TEST_CASE( "(multi)polygon with hole", "should round trip without changes" ) { // NOTE: this polygon should have correct winding order: // CCW for exterior, CW for interior @@ -324,96 +356,6 @@ TEST_CASE( "(multi)polygon with hole", "should round trip without changes" ) { CHECK( mapnik::util::to_wkt(wkt0,p1) ); CHECK( wkt0 == expected_wkt0); - // now test back compatibility mode where we decode all rings into exterior rings - // for polygons rings that were encoded correctly in vtiles (CCW exterior, CW interior) - // then this should be unneeded, but for rings with incorrect order then this style of - // decoding should allow them still to be queried correctly using the current mapnik hit_test algos - // Note, we need a new Geometry here, the old object can't be rewound. - mapnik::vector_tile_impl::Geometry geoms2(feature,0.0,0.0,1.0,1.0); - auto _p1 = mapnik::vector_tile_impl::decode_geometry(geoms2, feature.type(), true); - wkt0.clear(); - CHECK( mapnik::util::to_wkt(wkt0,_p1) ); - CHECK( _p1.is<mapnik::geometry::multi_polygon<double> >() ); - std::string expected_wkt2("MULTIPOLYGON(((0 0,0 10,-10 10,-10 0,0 0)),((-7 7,-7 3,-3 3,-3 7,-7 7)))"); - CHECK( wkt0 == expected_wkt2 ); - mapnik::geometry::correct(_p1); - wkt0.clear(); - CHECK( mapnik::util::to_wkt(wkt0,_p1) ); - CHECK( wkt0 == expected_wkt2 ); - - std::string expected_p0( - "move_to(0,0)\n" - "line_to(0,10)\n" - "line_to(-10,10)\n" - "line_to(-10,0)\n" - "close_path(0,0)\n" - "move_to(-7,7)\n" - "line_to(-3,7)\n" - "line_to(-3,3)\n" - "line_to(-7,3)\n" - "close_path(0,0)\n" - ); - - CHECK(decode_to_path_string(p1) == expected_p0); - - std::string expected_p1( - "move_to(0,0)\n" - "line_to(0,10)\n" - "line_to(-10,10)\n" - "line_to(-10,0)\n" - "close_path(0,0)\n" - "move_to(-7,7)\n" - "line_to(-7,3)\n" - "line_to(-3,3)\n" - "line_to(-3,7)\n" - "close_path(0,0)\n" - ); - - CHECK(decode_to_path_string(_p1) == expected_p1); - - // make into multi_polygon - mapnik::geometry::multi_polygon<std::int64_t> multi_poly; - multi_poly.push_back(std::move(p0)); - mapnik::geometry::polygon<std::int64_t> p2; - p2.exterior_ring.add_coord(-6,4); - p2.exterior_ring.add_coord(-4,4); - p2.exterior_ring.add_coord(-4,6); - p2.exterior_ring.add_coord(-6,6); - p2.exterior_ring.add_coord(-6,4); - multi_poly.push_back(std::move(p2)); - - mapnik::box2d<double> multi_extent = mapnik::geometry::envelope(multi_poly); - - vector_tile::Tile_Feature feature1 = geometry_to_feature<std::int64_t>(multi_poly); - mapnik::vector_tile_impl::Geometry geoms1(feature1,0.0,0.0,1.0,1.0); - auto mp = mapnik::vector_tile_impl::decode_geometry(geoms1, feature1.type()); - CHECK( mp.is<mapnik::geometry::multi_polygon<double> >() ); - - CHECK( multi_extent == mapnik::geometry::envelope(mp) ); - - wkt0.clear(); - CHECK( mapnik::util::to_wkt(wkt0,mp) ); - std::string expected_wkt3("MULTIPOLYGON(((0 0,0 10,-10 10,-10 0,0 0),(-7 7,-3 7,-3 3,-7 3,-7 7)),((-6 4,-4 4,-4 6,-6 6,-6 4)))"); - CHECK( wkt0 == expected_wkt3); - // ensure correcting geometry has no effect - // as a way of confirming the original was correct - mapnik::geometry::correct(mp); - wkt0.clear(); - CHECK( mapnik::util::to_wkt(wkt0,mp) ); - CHECK( wkt0 == expected_wkt3); - - std::string expected_multi = expected_p0 += std::string( - "move_to(-6,4)\n" - "line_to(-4,4)\n" - "line_to(-4,6)\n" - "line_to(-6,6)\n" - "close_path(0,0)\n" - ); - - CHECK(decode_to_path_string(mp) == expected_multi); - CHECK(mapnik::geometry::is_valid(mp)); - CHECK(mapnik::geometry::is_simple(mp)); - } // We no longer drop coincidental points in the encoder it should be diff --git a/test/test_utils.cpp b/test/test_utils.cpp index 02c2012..b26b5ae 100644 --- a/test/test_utils.cpp +++ b/test/test_utils.cpp @@ -75,6 +75,22 @@ std::shared_ptr<mapnik::memory_datasource> build_geojson_ds(std::string const& g return ds; } +mapnik::geometry::geometry<double> read_geojson(std::string const& geojson_file) { + mapnik::util::file input(geojson_file); + if (!input.open()) + { + throw std::runtime_error("failed to open geojson"); + } + mapnik::geometry::geometry<double> geom; + std::string json_string(input.data().get(), input.size()); + if (!mapnik::json::from_geojson(json_string, geom)) + { + throw std::runtime_error("failed to parse geojson"); + } + mapnik::geometry::correct(geom); + return geom; +} + unsigned compare_images(mapnik::image_rgba8 const& src1, std::string const& filepath, int threshold, diff --git a/test/test_utils.hpp b/test/test_utils.hpp index 25a4b17..fe51943 100644 --- a/test/test_utils.hpp +++ b/test/test_utils.hpp @@ -10,6 +10,7 @@ namespace testing { std::shared_ptr<mapnik::memory_datasource> build_ds(double x,double y, bool second=false); +mapnik::geometry::geometry<double> read_geojson(std::string const& geojson_file); std::shared_ptr<mapnik::memory_datasource> build_geojson_ds(std::string const& geojson_file); unsigned compare_images(std::string const& src_fn, diff --git a/test/vector_tile.cpp b/test/vector_tile.cpp index 00ee133..d5a349a 100644 --- a/test/vector_tile.cpp +++ b/test/vector_tile.cpp @@ -26,6 +26,7 @@ // vector output api #include "vector_tile_compression.hpp" #include "vector_tile_processor.hpp" +#include "vector_tile_strategy.hpp" #include "vector_tile_backend_pbf.hpp" #include "vector_tile_util.hpp" #include "vector_tile_projection.hpp" @@ -606,7 +607,14 @@ mapnik::geometry::geometry<double> round_trip(mapnik::geometry::geometry<double> mapnik::projection merc("+init=epsg:4326",true); mapnik::proj_transform prj_trans(merc,wgs84); ren.set_simplify_distance(simplify_distance); - ren.handle_geometry(*feature,geom,prj_trans,bbox); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs2(prj_trans,ren.get_transform(),backend.get_path_multiplier()); + mapnik::vector_tile_impl::vector_tile_strategy vs(ren.get_transform(),backend.get_path_multiplier()); + mapnik::geometry::point<double> p1_min(bbox.minx(), bbox.miny()); + mapnik::geometry::point<double> p1_max(bbox.maxx(), bbox.maxy()); + mapnik::geometry::point<std::int64_t> p2_min = mapnik::geometry::transform<std::int64_t>(p1_min, vs); + mapnik::geometry::point<std::int64_t> p2_max = mapnik::geometry::transform<std::int64_t>(p1_max, vs); + mapnik::box2d<int> clipping_extent(p2_min.x, p2_min.y, p2_max.x, p2_max.y); + ren.handle_geometry(vs2,*feature,geom,clipping_extent); backend.stop_tile_layer(); if (tile.layers_size() != 1) { @@ -956,7 +964,7 @@ TEST_CASE( "vector tile from simplified geojson", "should create vector tile wit REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); CHECK(std::string("layer") == layer.name()); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); unsigned z = 0; unsigned x = 0; @@ -1006,7 +1014,14 @@ mapnik::geometry::geometry<double> round_trip2(mapnik::geometry::geometry<double { throw std::runtime_error("simplify_distance setter did not work"); } - ren.handle_geometry(*feature,geom,prj_trans,bbox); + mapnik::vector_tile_impl::vector_tile_strategy_proj vs2(prj_trans,ren.get_transform(),backend.get_path_multiplier()); + mapnik::vector_tile_impl::vector_tile_strategy vs(ren.get_transform(),backend.get_path_multiplier()); + mapnik::geometry::point<double> p1_min(bbox.minx(), bbox.miny()); + mapnik::geometry::point<double> p1_max(bbox.maxx(), bbox.maxy()); + mapnik::geometry::point<std::int64_t> p2_min = mapnik::geometry::transform<std::int64_t>(p1_min, vs); + mapnik::geometry::point<std::int64_t> p2_max = mapnik::geometry::transform<std::int64_t>(p1_max, vs); + mapnik::box2d<int> clipping_extent(p2_min.x, p2_min.y, p2_max.x, p2_max.y); + ren.handle_geometry(vs2,*feature,geom,clipping_extent); backend.stop_tile_layer(); if (tile.layers_size() != 1) { @@ -1054,11 +1069,11 @@ TEST_CASE( "vector tile line_string is verify direction", "should line string wi mapnik::geometry::geometry<double> xgeom = mapnik::geometry::transform<double>(new_geom, proj_strat); std::string wkt; mapnik::util::to_wkt(wkt, xgeom); - CHECK( wkt == "MULTILINESTRING((0 1.99992945603165,2.00006103515625 1.99992945603165,2.00006103515625 0),(7.99996948242188 0,7.99996948242188 1.99992945603165,59.9999084472656 1.99992945603165,59.9999084472656 7.99994115658818,7.99996948242188 7.99994115658818,7.99996948242188 59.9999474398107,2.00006103515625 59.9999474398107,2.00006103515625 7.99994115658818,0.0000000000000005 7.99994115658818))" ); + CHECK( wkt == "MULTILINESTRING((0 1.99992945603165,2.00006103515625 1.99992945603165,2.00006103515625 0),(7.99996948242188 0,7.99996948242188 1.99992945603165,11.25 1.99992945603165),(11.25 7.99994115658818,7.99996948242188 7.99994115658818,7.99996948242188 11.1784018737118),(2.00006103515625 11.1784018737118,2.00006103515625 7.99994115658818,0.0000000000000005 7.99994115658818))" ); REQUIRE( !mapnik::geometry::is_empty(xgeom) ); REQUIRE( new_geom.is<mapnik::geometry::multi_line_string<double> >() ); auto const& line2 = mapnik::util::get<mapnik::geometry::multi_line_string<double> >(new_geom); - CHECK( line2.size() == 2 ); + CHECK( line2.size() == 4 ); } TEST_CASE( "vector tile transform", "should not throw on coords outside merc range" ) { @@ -1136,3 +1151,80 @@ TEST_CASE( "vector tile transform", "should not throw on coords outside merc ran mapnik::save_to_file(im,"test/fixtures/transform-actual-1.png","png32"); } } + +TEST_CASE( "vector tile transform2", "should not throw reprojected data from local NZ projection" ) { + typedef mapnik::vector_tile_impl::backend_pbf backend_type; + typedef mapnik::vector_tile_impl::processor<backend_type> renderer_type; + typedef vector_tile::Tile tile_type; + tile_type tile; + backend_type backend(tile,64); + unsigned tile_size = 256; + mapnik::box2d<double> bbox(-20037508.342789,-20037508.342789,20037508.342789,20037508.342789); + mapnik::Map map(tile_size,tile_size,"+init=epsg:3857"); + // Note: 4269 is key. 4326 will trigger custom mapnik reprojection code + // that does not hit proj4 and clamps values + mapnik::layer lyr("layer","+proj=nzmg +lat_0=-41 +lon_0=173 +x_0=2510000 +y_0=6023150 +ellps=intl +units=m +no_defs"); + mapnik::parameters params; + params["type"] = "shape"; + params["file"] = "./test/data/NZ_Coastline_NZMG.shp"; + std::shared_ptr<mapnik::datasource> ds = + mapnik::datasource_cache::instance().create(params); + lyr.set_datasource(ds); + map.add_layer(lyr); + map.zoom_to_box(bbox); + mapnik::request m_req(tile_size,tile_size,bbox); + renderer_type ren(backend,map,m_req); + // should no longer throw after https://github.com/mapbox/mapnik-vector-tile/pull/128 + ren.apply(); + + // serialize to message + std::string buffer; + CHECK(tile.SerializeToString(&buffer)); + CHECK(231 == buffer.size()); + // now create new objects + mapnik::Map map2(tile_size,tile_size,"+init=epsg:3857"); + tile_type tile2; + CHECK(tile2.ParseFromString(buffer)); + std::string key(""); + CHECK(false == mapnik::vector_tile_impl::is_solid_extent(tile2,key)); + CHECK("" == key); + CHECK(1 == tile2.layers_size()); + vector_tile::Tile_Layer const& layer2 = tile2.layers(0); + CHECK(std::string("layer") == layer2.name()); + CHECK(2 == layer2.features_size()); + + mapnik::layer lyr2("layer",map.srs()); + std::shared_ptr<mapnik::vector_tile_impl::tile_datasource> ds2 = std::make_shared< + mapnik::vector_tile_impl::tile_datasource>( + layer2,0,0,0,map2.width()); + ds2->set_envelope(bbox); + CHECK( ds2->type() == mapnik::datasource::Vector ); + CHECK( ds2->get_geometry_type() == mapnik::datasource_geometry_t::Collection ); + mapnik::layer_descriptor lay_desc = ds2->get_descriptor(); + std::vector<std::string> expected_names; + expected_names.push_back("featurecla"); + expected_names.push_back("scalerank"); + std::vector<std::string> names; + for (auto const& desc : lay_desc.get_descriptors()) + { + names.push_back(desc.get_name()); + } + CHECK(names == expected_names); + lyr2.set_datasource(ds2); + lyr2.add_style("style"); + map2.add_layer(lyr2); + mapnik::load_map(map2,"test/data/polygon-style.xml"); + //std::clog << mapnik::save_map_to_string(map2) << "\n"; + map2.zoom_to_box(bbox); + mapnik::image_rgba8 im(map2.width(),map2.height()); + mapnik::agg_renderer<mapnik::image_rgba8> ren2(map2,im); + ren2.apply(); + if (!mapnik::util::exists("test/fixtures/transform-expected-2.png")) { + mapnik::save_to_file(im,"test/fixtures/transform-expected-2.png","png32"); + } + unsigned diff = testing::compare_images(im,"test/fixtures/transform-expected-2.png"); + CHECK(0 == diff); + if (diff > 0) { + mapnik::save_to_file(im,"test/fixtures/transform-actual-2.png","png32"); + } +} \ No newline at end of file diff --git a/test/vector_tile_pbf.cpp b/test/vector_tile_pbf.cpp index 47e8bf0..fb0e3c0 100644 --- a/test/vector_tile_pbf.cpp +++ b/test/vector_tile_pbf.cpp @@ -144,10 +144,10 @@ TEST_CASE( "pbf vector tile datasource", "should filter features outside extent" std::string key(""); CHECK(false == mapnik::vector_tile_impl::is_solid_extent(tile,key)); CHECK("" == key); - CHECK(1 == tile.layers_size()); + REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); CHECK(std::string("layer") == layer.name()); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); CHECK(static_cast<mapnik::value_integer>(1) == static_cast<mapnik::value_integer>(f.id())); CHECK(3 == f.geometry_size()); @@ -261,9 +261,9 @@ TEST_CASE( "pbf encoding multi line as one path", "should maintain second move_t std::string key(""); CHECK(false == mapnik::vector_tile_impl::is_solid_extent(tile,key)); CHECK("" == key); - CHECK(1 == tile.layers_size()); + REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); CHECK(12 == f.geometry_size()); CHECK(9 == f.geometry(0)); // 1 move_to @@ -343,10 +343,10 @@ TEST_CASE( "pbf decoding some truncated buffers", "should throw exception" ) { std::string key(""); CHECK(false == mapnik::vector_tile_impl::is_solid_extent(tile,key)); CHECK("" == key); - CHECK(1 == tile.layers_size()); + REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); CHECK(std::string("layer") == layer.name()); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); CHECK(static_cast<mapnik::value_integer>(1) == static_cast<mapnik::value_integer>(f.id())); CHECK(3 == f.geometry_size()); @@ -397,10 +397,10 @@ TEST_CASE( "pbf vector tile from simplified geojson", "should create vector tile renderer_type ren(backend,map,m_req); ren.apply(); CHECK( ren.painted() == true ); - CHECK(1 == tile.layers_size()); + REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); CHECK(std::string("layer") == layer.name()); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); unsigned z = 0; unsigned x = 0; @@ -478,10 +478,10 @@ TEST_CASE( "pbf raster tile output", "should be able to overzoom raster" ) { ren.apply(); } // Done creating test data, now test created tile - CHECK(1 == tile.layers_size()); + REQUIRE(1 == tile.layers_size()); vector_tile::Tile_Layer const& layer = tile.layers(0); CHECK(std::string("layer") == layer.name()); - CHECK(1 == layer.features_size()); + REQUIRE(1 == layer.features_size()); vector_tile::Tile_Feature const& f = layer.features(0); CHECK(static_cast<mapnik::value_integer>(1) == static_cast<mapnik::value_integer>(f.id())); CHECK(0 == f.geometry_size()); -- 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