From 7cc52b12d53af41de242dde7807934964729db0c Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Mon, 4 Sep 2017 08:42:55 +0000 Subject: [PATCH 1/4] Backport Python test of bbox reprojection Originaly in Python from https://github.com/mapnik/mapnik/pull/2657 --- test/unit/projection/proj_transform.cpp | 66 +++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/test/unit/projection/proj_transform.cpp b/test/unit/projection/proj_transform.cpp index 95c959587..42700b68b 100644 --- a/test/unit/projection/proj_transform.cpp +++ b/test/unit/projection/proj_transform.cpp @@ -120,4 +120,70 @@ SECTION("test pj_transform failure behavior") #endif +// Github Issue https://github.com/mapnik/mapnik/issues/2648 +SECTION("Test proj antimeridian bbox") +{ + mapnik::projection prj_geog("+init=epsg:4326"); + mapnik::projection prj_proj("+init=epsg:2193"); + + mapnik::proj_transform prj_trans_fwd(prj_proj, prj_geog); + mapnik::proj_transform prj_trans_rev(prj_geog, prj_proj); + + // bad = mapnik.Box2d(-177.31453250437079, -62.33374815225163, 178.02778363316355, -24.584597490955804) + const mapnik::box2d better(-180.0, -62.33374815225163, + 180.0, -24.584597490955804); + + { + mapnik::box2d ext(274000, 3087000, 3327000, 7173000); + prj_trans_fwd.forward(ext, PROJ_ENVELOPE_POINTS); + CHECK(ext.minx() == Approx(better.minx())); + CHECK(ext.miny() == Approx(better.miny())); + CHECK(ext.maxx() == Approx(better.maxx())); + CHECK(ext.maxy() == Approx(better.maxy())); + } + + { + // check the same logic works for .backward() + mapnik::box2d ext(274000, 3087000, 3327000, 7173000); + prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS); + CHECK(ext.minx() == Approx(better.minx())); + CHECK(ext.miny() == Approx(better.miny())); + CHECK(ext.maxx() == Approx(better.maxx())); + CHECK(ext.maxy() == Approx(better.maxy())); + } + + { + // checks for not being snapped (ie. not antimeridian) + mapnik::box2d ext(274000, 3087000, 3327000, 7173000); + prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS); + CHECK(ext.minx() == Approx(better.minx())); + CHECK(ext.miny() == Approx(better.miny())); + CHECK(ext.maxx() == Approx(better.maxx())); + CHECK(ext.maxy() == Approx(better.maxy())); + } + + const mapnik::box2d normal(148.766759749, -60.1222810238, + 159.95484893, -24.9774643167); + + { + // checks for not being snapped (ie. not antimeridian) + mapnik::box2d ext(274000, 3087000, 276000, 7173000); + prj_trans_fwd.forward(ext, PROJ_ENVELOPE_POINTS); + CHECK(ext.minx() == Approx(normal.minx())); + CHECK(ext.miny() == Approx(normal.miny())); + CHECK(ext.maxx() == Approx(normal.maxx())); + CHECK(ext.maxy() == Approx(normal.maxy())); + } + + { + // check the same logic works for .backward() + mapnik::box2d ext(274000, 3087000, 276000, 7173000); + prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS); + CHECK(ext.minx() == Approx(normal.minx())); + CHECK(ext.miny() == Approx(normal.miny())); + CHECK(ext.maxx() == Approx(normal.maxx())); + CHECK(ext.maxy() == Approx(normal.maxy())); + } +} + } From 6714207379538fba299c07a6e88b6761e562c0a0 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 10 Jul 2018 10:26:49 +0200 Subject: [PATCH 2/4] proj_transform test: use reference values from cs2cs tool --- test/unit/projection/proj_transform.cpp | 40 +++++++++++++++---------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/test/unit/projection/proj_transform.cpp b/test/unit/projection/proj_transform.cpp index 42700b68b..cbaa4e13d 100644 --- a/test/unit/projection/proj_transform.cpp +++ b/test/unit/projection/proj_transform.cpp @@ -129,9 +129,20 @@ SECTION("Test proj antimeridian bbox") mapnik::proj_transform prj_trans_fwd(prj_proj, prj_geog); mapnik::proj_transform prj_trans_rev(prj_geog, prj_proj); - // bad = mapnik.Box2d(-177.31453250437079, -62.33374815225163, 178.02778363316355, -24.584597490955804) - const mapnik::box2d better(-180.0, -62.33374815225163, - 180.0, -24.584597490955804); + // reference values taken from proj4 command line tool: + // (non-corner points assume PROJ_ENVELOPE_POINTS == 20) + // + // cs2cs -Ef %.10f +init=epsg:2193 +to +init=epsg:4326 < better(-180.0, -62.3337481525, + 180.0, -24.5845974912); { mapnik::box2d ext(274000, 3087000, 3327000, 7173000); @@ -152,18 +163,17 @@ SECTION("Test proj antimeridian bbox") CHECK(ext.maxy() == Approx(better.maxy())); } - { - // checks for not being snapped (ie. not antimeridian) - mapnik::box2d ext(274000, 3087000, 3327000, 7173000); - prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS); - CHECK(ext.minx() == Approx(better.minx())); - CHECK(ext.miny() == Approx(better.miny())); - CHECK(ext.maxx() == Approx(better.maxx())); - CHECK(ext.maxy() == Approx(better.maxy())); - } - - const mapnik::box2d normal(148.766759749, -60.1222810238, - 159.95484893, -24.9774643167); + // reference values taken from proj4 command line tool: + // + // cs2cs -Ef %.10f +init=epsg:2193 +to +init=epsg:4326 < normal(148.7667597489, -60.1222810241, + 159.9548489296, -24.9771195155); { // checks for not being snapped (ie. not antimeridian) From b2f8c0816b302672174be19393a357c929b8e930 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 10 Jul 2018 10:37:43 +0200 Subject: [PATCH 3/4] proj_transform: fix bbox reprojection - remove buggy calculate_bbox, use boost::geometry::envelope instead - move helper envelope_points to anonymous namespace and make it always produce exactly the requested number of points, even if it's not evenly divisible by 4 --- src/proj_transform.cpp | 130 ++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 60 deletions(-) diff --git a/src/proj_transform.cpp b/src/proj_transform.cpp index 9c23d805a..0a2237082 100644 --- a/src/proj_transform.cpp +++ b/src/proj_transform.cpp @@ -21,13 +21,17 @@ *****************************************************************************/ // mapnik -#include #include +#include +#include #include #include #include #include +// boost +#include + #ifdef MAPNIK_USE_PROJ4 // proj4 #include @@ -39,6 +43,56 @@ namespace mapnik { +namespace { // (local) + +// Returns points in clockwise order. This allows us to do anti-meridian checks. +template +auto envelope_points(box2d const& env, std::size_t num_points) + -> geometry::multi_point +{ + auto width = env.width(); + auto height = env.height(); + + geometry::multi_point coords; + coords.reserve(num_points); + + // top side: left >>> right + // gets extra point if (num_points % 4 >= 1) + for (std::size_t i = 0, n = (num_points + 3) / 4; i < n; ++i) + { + auto x = env.minx() + (i * width) / n; + coords.emplace_back(x, env.maxy()); + } + + // right side: top >>> bottom + // gets extra point if (num_points % 4 >= 3) + for (std::size_t i = 0, n = (num_points + 1) / 4; i < n; ++i) + { + auto y = env.maxy() - (i * height) / n; + coords.emplace_back(env.maxx(), y); + } + + // bottom side: right >>> left + // gets extra point if (num_points % 4 >= 2) + for (std::size_t i = 0, n = (num_points + 2) / 4; i < n; ++i) + { + auto x = env.maxx() - (i * width) / n; + coords.emplace_back(x, env.miny()); + } + + // left side: bottom >>> top + // never gets extra point + for (std::size_t i = 0, n = (num_points + 0) / 4; i < n; ++i) + { + auto y = env.miny() + (i * height) / n; + coords.emplace_back(env.minx(), y); + } + + return coords; +} + +} // namespace mapnik::(local) + proj_transform::proj_transform(projection const& source, projection const& dest) : source_(source), @@ -334,49 +388,6 @@ bool proj_transform::backward (box2d & box) const return true; } -// Returns points in clockwise order. This allows us to do anti-meridian checks. -void envelope_points(std::vector< coord > & coords, box2d& env, int points) -{ - double width = env.width(); - double height = env.height(); - - int steps; - - if (points <= 4) { - steps = 0; - } else { - steps = static_cast(std::ceil((points - 4) / 4.0)); - } - - steps += 1; - double xstep = width / steps; - double ystep = height / steps; - - coords.resize(points); - for (int i=0; iright - coords[i] = coord(env.minx() + i * xstep, env.maxy()); - // right: top>bottom - coords[i + steps] = coord(env.maxx(), env.maxy() - i * ystep); - // bottom: right>left - coords[i + steps * 2] = coord(env.maxx() - i * xstep, env.miny()); - // left: bottom>top - coords[i + steps * 3] = coord(env.minx(), env.miny() + i * ystep); - } -} - -box2d calculate_bbox(std::vector > & points) { - std::vector >::iterator it = points.begin(); - std::vector >::iterator it_end = points.end(); - - box2d env(*it, *(++it)); - for (; it!=it_end; ++it) { - env.expand_to_include(*it); - } - return env; -} - - // More robust, but expensive, bbox transform // in the face of proj4 out of bounds conditions. // Can result in 20 -> 10 r/s performance hit. @@ -393,18 +404,18 @@ bool proj_transform::backward(box2d& env, int points) const return backward(env); } - std::vector > coords; - envelope_points(coords, env, points); // this is always clockwise + auto coords = envelope_points(env, points); // this is always clockwise - double z; - for (std::vector >::iterator it = coords.begin(); it!=coords.end(); ++it) { - z = 0; - if (!backward(it->x, it->y, z)) { + for (auto & p : coords) + { + double z = 0; + if (!backward(p.x, p.y, z)) return false; - } } - box2d result = calculate_bbox(coords); + box2d result; + boost::geometry::envelope(coords, result); + if (is_source_longlat_ && !util::is_clockwise(coords)) { // we've gone to a geographic CS, and our clockwise envelope has @@ -432,18 +443,17 @@ bool proj_transform::forward(box2d& env, int points) const return forward(env); } - std::vector > coords; - envelope_points(coords, env, points); // this is always clockwise + auto coords = envelope_points(env, points); // this is always clockwise - double z; - for (std::vector >::iterator it = coords.begin(); it!=coords.end(); ++it) { - z = 0; - if (!forward(it->x, it->y, z)) { + for (auto & p : coords) + { + double z = 0; + if (!forward(p.x, p.y, z)) return false; - } } - box2d result = calculate_bbox(coords); + box2d result; + boost::geometry::envelope(coords, result); if (is_dest_longlat_ && !util::is_clockwise(coords)) { From d4af8f177b1ad42e0a45b283d38aa7726267036d Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 10 Jul 2018 18:51:34 +0200 Subject: [PATCH 4/4] Revert "allow visual test failures with g++" This reverts commit 05936826b378e79306fa9aef0c890ceb822f2224. --- .travis.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 38793fdb3..f794280f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -103,10 +103,7 @@ script: # (and might work) for the next build - DURATION=2400 - scripts/travis-command-wrapper.py -s "date" -i 120 --deadline=$(( $(date +%s) + ${DURATION} )) make - - RESULT=0 - - make test || RESULT=$? - # we allow visual failures with g++ for now: https://github.com/mapnik/mapnik/issues/3567 - - if [[ ${RESULT} != 0 ]] && [[ ${CXX} =~ 'clang++' ]]; then false; fi; + - make test - enabled ${COVERAGE} coverage - enabled ${BENCH} make bench - ./scripts/check_glibcxx.sh