From 64b874ecb13e2458835b0629a6feeee2d13981ff Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Sun, 8 Oct 2017 14:32:14 +0000 Subject: [PATCH 1/6] New interior algorithm --- include/mapnik/geom_util.hpp | 74 ------ include/mapnik/geometry/interior.hpp | 35 +++ .../geometry/polygon_vertex_processor.hpp | 71 ++++++ include/mapnik/markers_placements/basic.hpp | 1 + .../mapnik/markers_placements/interior.hpp | 14 +- .../process_point_symbolizer.hpp | 5 +- src/build.py | 1 + src/geometry/interior.cpp | 234 ++++++++++++++++++ .../render_markers_symbolizer.cpp | 3 +- src/text/symbolizer_helpers.cpp | 18 +- 10 files changed, 370 insertions(+), 86 deletions(-) create mode 100644 include/mapnik/geometry/interior.hpp create mode 100644 include/mapnik/geometry/polygon_vertex_processor.hpp create mode 100644 src/geometry/interior.cpp diff --git a/include/mapnik/geom_util.hpp b/include/mapnik/geom_util.hpp index d16c42eee4..32283601fe 100644 --- a/include/mapnik/geom_util.hpp +++ b/include/mapnik/geom_util.hpp @@ -518,80 +518,6 @@ bool hit_test(PathType & path, double x, double y, double tol) return inside; } -template -bool interior_position(PathType & path, double & x, double & y) -{ - // start with the centroid - if (!label::centroid(path, x,y)) - return false; - - // otherwise we find a horizontal line across the polygon and then return the - // center of the widest intersection between the polygon and the line. - - std::vector intersections; // only need to store the X as we know the y - geometry::point p0, p1, move_to; - unsigned command = SEG_END; - - path.rewind(0); - - while (SEG_END != (command = path.vertex(&p0.x, &p0.y))) - { - switch (command) - { - case SEG_MOVETO: - move_to = p0; - break; - case SEG_CLOSE: - p0 = move_to; - case SEG_LINETO: - // if the segments overlap - if (p0.y == p1.y) - { - if (p0.y == y) - { - double xi = (p0.x + p1.x) / 2.0; - intersections.push_back(xi); - } - } - // if the path segment crosses the bisector - else if ((p0.y <= y && p1.y >= y) || - (p0.y >= y && p1.y <= y)) - { - // then calculate the intersection - double xi = p0.x; - if (p0.x != p1.x) - { - double m = (p1.y - p0.y) / (p1.x - p0.x); - double c = p0.y - m * p0.x; - xi = (y - c) / m; - } - - intersections.push_back(xi); - } - break; - } - p1 = p0; - } - - // no intersections we just return the default - if (intersections.empty()) - return true; - std::sort(intersections.begin(), intersections.end()); - double max_width = 0; - for (unsigned ii = 1; ii < intersections.size(); ii += 2) - { - double xlow = intersections[ii-1]; - double xhigh = intersections[ii]; - double width = xhigh - xlow; - if (width > max_width) - { - x = (xlow + xhigh) / 2.0; - max_width = width; - } - } - return true; -} - }} #endif // MAPNIK_GEOM_UTIL_HPP diff --git a/include/mapnik/geometry/interior.hpp b/include/mapnik/geometry/interior.hpp new file mode 100644 index 0000000000..50378d163a --- /dev/null +++ b/include/mapnik/geometry/interior.hpp @@ -0,0 +1,35 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2017 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#ifndef MAPNIK_GEOMETRY_INTERIOR_HPP +#define MAPNIK_GEOMETRY_INTERIOR_HPP + +#include + +namespace mapnik { namespace geometry { + +template +point interior(polygon const& polygon, double scale_factor); + +} } + +#endif // MAPNIK_GEOMETRY_INTERIOR_HPP diff --git a/include/mapnik/geometry/polygon_vertex_processor.hpp b/include/mapnik/geometry/polygon_vertex_processor.hpp new file mode 100644 index 0000000000..46fe0cdcc4 --- /dev/null +++ b/include/mapnik/geometry/polygon_vertex_processor.hpp @@ -0,0 +1,71 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2017 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#ifndef MAPNIK_GEOMETRY_POLYGON_VERTEX_PROCESSOR_HPP +#define MAPNIK_GEOMETRY_POLYGON_VERTEX_PROCESSOR_HPP + +// geometry +#include + +namespace mapnik { namespace geometry { + +template +struct polygon_vertex_processor +{ + template + void add_path(Path & path) + { + point p; + unsigned cmd; + linear_ring ring; + bool exterior = true; + while ((cmd = path.vertex(&p.x, &p.y)) != SEG_END) + { + switch (cmd) + { + case SEG_MOVETO: + case SEG_LINETO: + ring.emplace_back(p); + break; + case SEG_CLOSE: + ring.emplace_back(ring.front()); + if (exterior) + { + polygon_.exterior_ring = std::move(ring); + exterior = false; + } + else + { + polygon_.interior_rings.emplace_back(std::move(ring)); + } + ring = linear_ring(); + break; + } + } + } + + polygon polygon_; +}; + +} } + +#endif // MAPNIK_GEOMETRY_POLYGON_VERTEX_PROCESSOR_HPP diff --git a/include/mapnik/markers_placements/basic.hpp b/include/mapnik/markers_placements/basic.hpp index abcef5ea6a..de3e18e108 100644 --- a/include/mapnik/markers_placements/basic.hpp +++ b/include/mapnik/markers_placements/basic.hpp @@ -46,6 +46,7 @@ struct markers_placement_params bool allow_overlap; bool avoid_edges; direction_enum direction; + double scale_factor; }; class markers_basic_placement : util::noncopyable diff --git a/include/mapnik/markers_placements/interior.hpp b/include/mapnik/markers_placements/interior.hpp index 6a2705e15d..e0bbd69a1b 100644 --- a/include/mapnik/markers_placements/interior.hpp +++ b/include/mapnik/markers_placements/interior.hpp @@ -26,6 +26,8 @@ #include #include #include +#include +#include namespace mapnik { @@ -58,11 +60,13 @@ class markers_interior_placement : public markers_point_placementlocator_, x, y)) - { - this->done_ = true; - return false; - } + geometry::polygon_vertex_processor vertex_processor; + vertex_processor.add_path(this->locator_); + geometry::point placement = geometry::interior(vertex_processor.polygon_, + this->params_.scale_factor); + + x = placement.x; + y = placement.y; } angle = 0; diff --git a/include/mapnik/renderer_common/process_point_symbolizer.hpp b/include/mapnik/renderer_common/process_point_symbolizer.hpp index ef0863fd66..fda6f8efca 100644 --- a/include/mapnik/renderer_common/process_point_symbolizer.hpp +++ b/include/mapnik/renderer_common/process_point_symbolizer.hpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -79,9 +80,7 @@ void render_point_symbolizer(point_symbolizer const &sym, else if (type == mapnik::geometry::geometry_types::Polygon) { auto const& poly = mapnik::util::get >(geometry); - geometry::polygon_vertex_adapter va(poly); - if (!label::interior_position(va ,pt.x, pt.y)) - return; + pt = geometry::interior(poly, common.scale_factor_); } else { diff --git a/src/build.py b/src/build.py index c5889dccf3..009f4d0def 100644 --- a/src/build.py +++ b/src/build.py @@ -169,6 +169,7 @@ def ldconfig(*args,**kwargs): datasource_cache_static.cpp debug.cpp geometry_reprojection.cpp + geometry/interior.cpp expression_node.cpp expression_string.cpp expression.cpp diff --git a/src/geometry/interior.cpp b/src/geometry/interior.cpp new file mode 100644 index 0000000000..ceab6ae1a7 --- /dev/null +++ b/src/geometry/interior.cpp @@ -0,0 +1,234 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2017 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace mapnik { namespace geometry { + +// Interior algorithm is realized as a modification of Polylabel algorithm +// from https://github.com/mapbox/polylabel. +// The modification aims to improve visual output by prefering +// placements closer to centroid. + +namespace detail { + +// get squared distance from a point to a segment +template +T segment_dist_sq(const point& p, + const point& a, + const point& b) +{ + auto x = a.x; + auto y = a.y; + auto dx = b.x - x; + auto dy = b.y - y; + + if (dx != 0 || dy != 0) { + + auto t = ((p.x - x) * dx + (p.y - y) * dy) / (dx * dx + dy * dy); + + if (t > 1) { + x = b.x; + y = b.y; + + } else if (t > 0) { + x += dx * t; + y += dy * t; + } + } + + dx = p.x - x; + dy = p.y - y; + + return dx * dx + dy * dy; +} + +template +void point_to_ring_dist(point const& point, linear_ring const& ring, + bool & inside, double & min_dist_sq) +{ + for (std::size_t i = 0, len = ring.size(), j = len - 1; i < len; j = i++) + { + const auto& a = ring[i]; + const auto& b = ring[j]; + + if ((a.y > point.y) != (b.y > point.y) && + (point.x < (b.x - a.x) * (point.y - a.y) / (b.y - a.y) + a.x)) inside = !inside; + + min_dist_sq = std::min(min_dist_sq, segment_dist_sq(point, a, b)); + } +} + +// signed distance from point to polygon outline (negative if point is outside) +template +double point_to_polygon_dist(const point& point, const polygon& polygon) +{ + bool inside = false; + double min_dist_sq = std::numeric_limits::infinity(); + + point_to_ring_dist(point, polygon.exterior_ring, inside, min_dist_sq); + + for (const auto& ring : polygon.interior_rings) + { + point_to_ring_dist(point, ring, inside, min_dist_sq); + } + + return (inside ? 1 : -1) * std::sqrt(min_dist_sq); +} + +template +struct fitness_functor +{ + fitness_functor(point const& centroid, point const& polygon_size) + : centroid(centroid), + max_size(std::max(polygon_size.x, polygon_size.y)) + {} + + T operator()(point const& cell_center, T distance_polygon) const + { + if (distance_polygon <= 0) + { + return distance_polygon; + } + point d(cell_center.x - centroid.x, cell_center.y - centroid.y); + double distance_centroid = std::sqrt(d.x * d.x + d.y * d.y); + return distance_polygon * (1 - distance_centroid / max_size); + } + + point centroid; + T max_size; +}; + +template +struct cell +{ + template + cell(const point& c_, T h_, + const polygon& polygon, + const FitnessFunc& ff) + : c(c_), + h(h_), + d(point_to_polygon_dist(c, polygon)), + fitness(ff(c, d)), + max_fitness(ff(c, d + h * std::sqrt(2))) + {} + + point c; // cell center + T h; // half the cell size + T d; // distance from cell center to polygon + T fitness; // fitness of the cell center + T max_fitness; // a "potential" of the cell calculated from max distance to polygon within the cell +}; + +template +point polylabel(const polygon& polygon, T precision = 1) +{ + // find the bounding box of the outer ring + const box2d bbox = envelope(polygon.exterior_ring); + const point size { bbox.width(), bbox.height() }; + + const T cell_size = std::min(size.x, size.y); + T h = cell_size / 2; + + // a priority queue of cells in order of their "potential" (max distance to polygon) + auto compare_func = [] (const cell& a, const cell& b) + { + return a.max_fitness < b.max_fitness; + }; + using Queue = std::priority_queue, std::vector>, decltype(compare_func)>; + Queue queue(compare_func); + + if (cell_size == 0) + { + return { bbox.minx(), bbox.miny() }; + } + + point centroid; + if (!mapnik::geometry::centroid(polygon, centroid)) + { + auto center = bbox.center(); + return { center.x, center.y }; + } + + fitness_functor fitness_func(centroid, size); + + // cover polygon with initial cells + for (T x = bbox.minx(); x < bbox.maxx(); x += cell_size) + { + for (T y = bbox.miny(); y < bbox.maxy(); y += cell_size) + { + queue.push(cell({x + h, y + h}, h, polygon, fitness_func)); + } + } + + // take centroid as the first best guess + auto best_cell = cell(centroid, 0, polygon, fitness_func); + + while (!queue.empty()) + { + // pick the most promising cell from the queue + auto current_cell = queue.top(); + queue.pop(); + + // update the best cell if we found a better one + if (current_cell.fitness > best_cell.fitness) + { + best_cell = current_cell; + } + + // do not drill down further if there's no chance of a better solution + if (current_cell.max_fitness - best_cell.fitness <= precision) continue; + + // split the cell into four cells + h = current_cell.h / 2; + queue.push(cell({current_cell.c.x - h, current_cell.c.y - h}, h, polygon, fitness_func)); + queue.push(cell({current_cell.c.x + h, current_cell.c.y - h}, h, polygon, fitness_func)); + queue.push(cell({current_cell.c.x - h, current_cell.c.y + h}, h, polygon, fitness_func)); + queue.push(cell({current_cell.c.x + h, current_cell.c.y + h}, h, polygon, fitness_func)); + } + + return best_cell.c; +} + +} // namespace detail + +template +point interior(polygon const& polygon, double scale_factor) +{ + // This precision has been chosen to work well in the map (viewport) coordinates. + double precision = 10.0 * scale_factor; + return detail::polylabel(polygon, precision); +} + +template +point interior(polygon const& polygon, double scale_factor); + +} } + diff --git a/src/renderer_common/render_markers_symbolizer.cpp b/src/renderer_common/render_markers_symbolizer.cpp index 6706cb1cd6..16b863c9cd 100644 --- a/src/renderer_common/render_markers_symbolizer.cpp +++ b/src/renderer_common/render_markers_symbolizer.cpp @@ -228,7 +228,8 @@ markers_dispatch_params::markers_dispatch_params(box2d const& size, get(sym, feature, vars), get(sym, feature, vars), get(sym, feature, vars), - get(sym, feature, vars)} + get(sym, feature, vars), + scale} , placement_method(get(sym, feature, vars)) , ignore_placement(get(sym, feature, vars)) , snap_to_pixels(snap) diff --git a/src/text/symbolizer_helpers.cpp b/src/text/symbolizer_helpers.cpp index 8064e6c6f4..f777075473 100644 --- a/src/text/symbolizer_helpers.cpp +++ b/src/text/symbolizer_helpers.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -40,6 +41,10 @@ #include #include #include +#include +#include +#include +#include namespace mapnik { namespace geometry { @@ -288,9 +293,16 @@ void base_symbolizer_helper::initialize_points() const } else if (how_placed == INTERIOR_PLACEMENT && type == geometry::geometry_types::Polygon) { - auto const& poly = mapnik::util::get >(geom); - geometry::polygon_vertex_adapter va(poly); - success = label::interior_position(va, label_x, label_y); + auto const& poly = util::get>(geom); + proj_transform backwart_transform(prj_trans_.dest(), prj_trans_.source()); + view_strategy vs(t_); + proj_strategy ps(backwart_transform); + using transform_group_type = geometry::strategy_group; + transform_group_type transform_group(ps, vs); + geometry::polygon tranformed_poly(geometry::transform(poly, transform_group)); + geometry::point pt = geometry::interior(tranformed_poly, scale_factor_); + points_.emplace_back(pt.x, pt.y); + continue; } else { From fe3c2762c08dcbc76212c813d0e409bb70710ef1 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Wed, 17 Jan 2018 09:53:45 +0000 Subject: [PATCH 2/6] Fix crash in case of empty ring --- .../geometry/polygon_vertex_processor.hpp | 5 +- .../geometry/polygon_vertex_processor.cpp | 65 +++++++++++++ test/unit/vertex_adapter/fake_path.hpp | 93 +++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 test/unit/geometry/polygon_vertex_processor.cpp create mode 100644 test/unit/vertex_adapter/fake_path.hpp diff --git a/include/mapnik/geometry/polygon_vertex_processor.hpp b/include/mapnik/geometry/polygon_vertex_processor.hpp index 46fe0cdcc4..cf2abda2a8 100644 --- a/include/mapnik/geometry/polygon_vertex_processor.hpp +++ b/include/mapnik/geometry/polygon_vertex_processor.hpp @@ -47,7 +47,10 @@ struct polygon_vertex_processor ring.emplace_back(p); break; case SEG_CLOSE: - ring.emplace_back(ring.front()); + if (!ring.empty()) + { + ring.emplace_back(ring.front()); + } if (exterior) { polygon_.exterior_ring = std::move(ring); diff --git a/test/unit/geometry/polygon_vertex_processor.cpp b/test/unit/geometry/polygon_vertex_processor.cpp new file mode 100644 index 0000000000..2b5de9b9e4 --- /dev/null +++ b/test/unit/geometry/polygon_vertex_processor.cpp @@ -0,0 +1,65 @@ +#include "catch.hpp" +#include "unit/vertex_adapter/fake_path.hpp" + +#include + +TEST_CASE("polygon_vertex_processor") { + +SECTION("empty polygon") { + + fake_path path = {}; + mapnik::geometry::polygon_vertex_processor proc; + proc.add_path(path); + CHECK(proc.polygon_.exterior_ring.size() == 0); + CHECK(proc.polygon_.interior_rings.size() == 0); +} + +SECTION("empty exterior ring") { + + fake_path path = {}; + path.vertices_.emplace_back(0, 0, mapnik::SEG_CLOSE); + path.rewind(0); + + mapnik::geometry::polygon_vertex_processor proc; + proc.add_path(path); + CHECK(proc.polygon_.exterior_ring.size() == 0); + CHECK(proc.polygon_.interior_rings.size() == 0); +} + +SECTION("empty interior ring") { + + fake_path path = {}; + path.vertices_.emplace_back(-1, -1, mapnik::SEG_MOVETO); + path.vertices_.emplace_back( 1, -1, mapnik::SEG_LINETO); + path.vertices_.emplace_back( 1, 1, mapnik::SEG_LINETO); + path.vertices_.emplace_back(-1, 1, mapnik::SEG_LINETO); + path.vertices_.emplace_back( 0, 0, mapnik::SEG_CLOSE); + path.vertices_.emplace_back( 0, 0, mapnik::SEG_CLOSE); + path.rewind(0); + + mapnik::geometry::polygon_vertex_processor proc; + proc.add_path(path); + + REQUIRE(proc.polygon_.interior_rings.size() == 1); + REQUIRE(proc.polygon_.interior_rings.front().size() == 0); + + auto const& outer_ring = proc.polygon_.exterior_ring; + REQUIRE(outer_ring.size() == 5); + + CHECK(outer_ring[0].x == Approx(-1)); + CHECK(outer_ring[0].y == Approx(-1)); + + CHECK(outer_ring[1].x == Approx( 1)); + CHECK(outer_ring[1].y == Approx(-1)); + + CHECK(outer_ring[2].x == Approx( 1)); + CHECK(outer_ring[2].y == Approx( 1)); + + CHECK(outer_ring[3].x == Approx(-1)); + CHECK(outer_ring[3].y == Approx( 1)); + + CHECK(outer_ring[4].x == Approx(-1)); + CHECK(outer_ring[4].y == Approx(-1)); +} + +} diff --git a/test/unit/vertex_adapter/fake_path.hpp b/test/unit/vertex_adapter/fake_path.hpp new file mode 100644 index 0000000000..0a41b1b079 --- /dev/null +++ b/test/unit/vertex_adapter/fake_path.hpp @@ -0,0 +1,93 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2017 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +// mapnik +#include + +// stl +#include +#include + +namespace detail +{ + +template +struct fake_path +{ + using coord_type = std::tuple; + using cont_type = std::vector; + cont_type vertices_; + typename cont_type::iterator itr_; + + fake_path(std::initializer_list l) + : fake_path(l.begin(), l.size()) + { + } + + fake_path(std::vector const &v, bool make_invalid = false) + : fake_path(v.begin(), v.size(), make_invalid) + { + } + + template + fake_path(Itr itr, size_t sz, bool make_invalid = false) + { + size_t num_coords = sz >> 1; + vertices_.reserve(num_coords + (make_invalid ? 1 : 0)); + if (make_invalid) + { + vertices_.push_back(std::make_tuple(0,0,mapnik::SEG_END)); + } + + for (size_t i = 0; i < num_coords; ++i) + { + T x = *itr++; + T y = *itr++; + unsigned cmd = (i == 0) ? mapnik::SEG_MOVETO : mapnik::SEG_LINETO; + vertices_.push_back(std::make_tuple(x, y, cmd)); + } + itr_ = vertices_.begin(); + } + + unsigned vertex(T *x, T *y) + { + if (itr_ == vertices_.end()) + { + return mapnik::SEG_END; + } + *x = std::get<0>(*itr_); + *y = std::get<1>(*itr_); + unsigned cmd = std::get<2>(*itr_); + ++itr_; + return cmd; + } + + void rewind(unsigned) + { + itr_ = vertices_.begin(); + } +}; + +} + +using fake_path = detail::fake_path; + From 72989d440b96de0688e41696c86b1cd8d80c2b45 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Wed, 17 Jan 2018 12:58:42 +0000 Subject: [PATCH 3/6] Interior: cover the case of empty polygon or exterior ring --- include/mapnik/geometry/interior.hpp | 5 ++- .../mapnik/markers_placements/interior.hpp | 10 ++++-- .../process_point_symbolizer.hpp | 2 +- src/geometry/interior.cpp | 28 +++++++++++++---- src/text/symbolizer_helpers.cpp | 7 +++-- test/unit/geometry/interior.cpp | 31 +++++++++++++++++++ 6 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 test/unit/geometry/interior.cpp diff --git a/include/mapnik/geometry/interior.hpp b/include/mapnik/geometry/interior.hpp index 50378d163a..d4318dcef6 100644 --- a/include/mapnik/geometry/interior.hpp +++ b/include/mapnik/geometry/interior.hpp @@ -24,11 +24,14 @@ #define MAPNIK_GEOMETRY_INTERIOR_HPP #include +#include // for MAPNIK_DECL namespace mapnik { namespace geometry { template -point interior(polygon const& polygon, double scale_factor); +MAPNIK_DECL bool interior(polygon const& polygon, + double scale_factor, + point & pt); } } diff --git a/include/mapnik/markers_placements/interior.hpp b/include/mapnik/markers_placements/interior.hpp index e0bbd69a1b..12b1af4946 100644 --- a/include/mapnik/markers_placements/interior.hpp +++ b/include/mapnik/markers_placements/interior.hpp @@ -62,8 +62,14 @@ class markers_interior_placement : public markers_point_placement vertex_processor; vertex_processor.add_path(this->locator_); - geometry::point placement = geometry::interior(vertex_processor.polygon_, - this->params_.scale_factor); + geometry::point placement; + if (!geometry::interior(vertex_processor.polygon_, + this->params_.scale_factor, + placement)) + { + this->done_ = true; + return false; + } x = placement.x; y = placement.y; diff --git a/include/mapnik/renderer_common/process_point_symbolizer.hpp b/include/mapnik/renderer_common/process_point_symbolizer.hpp index fda6f8efca..1756135dec 100644 --- a/include/mapnik/renderer_common/process_point_symbolizer.hpp +++ b/include/mapnik/renderer_common/process_point_symbolizer.hpp @@ -80,7 +80,7 @@ void render_point_symbolizer(point_symbolizer const &sym, else if (type == mapnik::geometry::geometry_types::Polygon) { auto const& poly = mapnik::util::get >(geometry); - pt = geometry::interior(poly, common.scale_factor_); + if (!geometry::interior(poly, common.scale_factor_, pt)) return; } else { diff --git a/src/geometry/interior.cpp b/src/geometry/interior.cpp index ceab6ae1a7..164d4c46e2 100644 --- a/src/geometry/interior.cpp +++ b/src/geometry/interior.cpp @@ -30,6 +30,11 @@ #include #include +#pragma GCC diagnostic push +#include +#include +#pragma GCC diagnostic pop + namespace mapnik { namespace geometry { // Interior algorithm is realized as a modification of Polylabel algorithm @@ -148,8 +153,13 @@ struct cell }; template -point polylabel(const polygon& polygon, T precision = 1) +boost::optional> polylabel(polygon const& polygon, T precision = 1) { + if (polygon.exterior_ring.empty()) + { + return boost::none; + } + // find the bounding box of the outer ring const box2d bbox = envelope(polygon.exterior_ring); const point size { bbox.width(), bbox.height() }; @@ -167,14 +177,14 @@ point polylabel(const polygon& polygon, T precision = 1) if (cell_size == 0) { - return { bbox.minx(), bbox.miny() }; + return point{ bbox.minx(), bbox.miny() }; } point centroid; if (!mapnik::geometry::centroid(polygon, centroid)) { auto center = bbox.center(); - return { center.x, center.y }; + return point{ center.x, center.y }; } fitness_functor fitness_func(centroid, size); @@ -220,15 +230,21 @@ point polylabel(const polygon& polygon, T precision = 1) } // namespace detail template -point interior(polygon const& polygon, double scale_factor) +bool interior(polygon const& polygon, double scale_factor, point & pt) { // This precision has been chosen to work well in the map (viewport) coordinates. double precision = 10.0 * scale_factor; - return detail::polylabel(polygon, precision); + if (boost::optional> opt = detail::polylabel(polygon, precision)) + { + pt = *opt; + return true; + } + + return false; } template -point interior(polygon const& polygon, double scale_factor); +bool interior(polygon const& polygon, double scale_factor, point & pt); } } diff --git a/src/text/symbolizer_helpers.cpp b/src/text/symbolizer_helpers.cpp index f777075473..4bac8a4fe2 100644 --- a/src/text/symbolizer_helpers.cpp +++ b/src/text/symbolizer_helpers.cpp @@ -300,8 +300,11 @@ void base_symbolizer_helper::initialize_points() const using transform_group_type = geometry::strategy_group; transform_group_type transform_group(ps, vs); geometry::polygon tranformed_poly(geometry::transform(poly, transform_group)); - geometry::point pt = geometry::interior(tranformed_poly, scale_factor_); - points_.emplace_back(pt.x, pt.y); + geometry::point pt; + if (geometry::interior(tranformed_poly, scale_factor_, pt)) + { + points_.emplace_back(pt.x, pt.y); + } continue; } else diff --git a/test/unit/geometry/interior.cpp b/test/unit/geometry/interior.cpp new file mode 100644 index 0000000000..646bb0d3db --- /dev/null +++ b/test/unit/geometry/interior.cpp @@ -0,0 +1,31 @@ +#include "catch.hpp" + +#include + +TEST_CASE("polygon interior") { + +SECTION("empty polygon") { + + mapnik::geometry::polygon poly; + mapnik::geometry::point pt; + + CHECK(!mapnik::geometry::interior(poly, 1.0, pt)); +} + +SECTION("interior of a square") { + + mapnik::geometry::polygon poly; + poly.exterior_ring.emplace_back(-1, -1); + poly.exterior_ring.emplace_back( 1, -1); + poly.exterior_ring.emplace_back( 1, 1); + poly.exterior_ring.emplace_back(-1, 1); + poly.exterior_ring.emplace_back(-1, -1); + + mapnik::geometry::point pt{ -3, -3 }; + + CHECK(mapnik::geometry::interior(poly, 1.0, pt)); + CHECK(pt.x == Approx(0)); + CHECK(pt.y == Approx(0)); +} + +} From 2e478ddd3768353bcfd700354e2b480768a9b927 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Wed, 24 Jan 2018 11:07:05 +0000 Subject: [PATCH 4/6] Update visual tests --- test/data-visual | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/data-visual b/test/data-visual index 2749835d32..1a9784770b 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit 2749835d3265d95b951c2c90c099fac35e326f39 +Subproject commit 1a9784770b378d5bdc05020b5f27d79189c7af4a From f8c6ad1e29d8a4924b02a65b87f433820a407533 Mon Sep 17 00:00:00 2001 From: talaj Date: Thu, 14 Sep 2017 15:21:30 +0200 Subject: [PATCH 5/6] visual tests: allow to ignore particular renderer (#3768) * visual tests: refactor parsing parameters from the style * visual tests: allow to ignore particular renderer * update visual tests --- test/visual/config.hpp | 9 ++++- test/visual/runner.cpp | 87 +++++++++++++++++++++++++++--------------- test/visual/runner.hpp | 1 + 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/test/visual/config.hpp b/test/visual/config.hpp index 0dac1d9830..2f5f6acdee 100644 --- a/test/visual/config.hpp +++ b/test/visual/config.hpp @@ -25,12 +25,15 @@ // stl #include +#include #include #include // boost #include +#include + namespace visual_tests { @@ -47,12 +50,16 @@ struct config config() : status(true), scales({ 1.0, 2.0 }), sizes({ { 500, 100 } }), - tiles({ { 1, 1 } }) { } + tiles({ { 1, 1 } }), + bbox(), + ignored_renderers() { } bool status; std::vector scales; std::vector sizes; std::vector tiles; + mapnik::box2d bbox; + std::set ignored_renderers; }; enum result_state : std::uint8_t diff --git a/test/visual/runner.cpp b/test/visual/runner.cpp index d0c7222fb3..44787e97d1 100644 --- a/test/visual/runner.cpp +++ b/test/visual/runner.cpp @@ -32,6 +32,15 @@ namespace visual_tests { +struct renderer_name_visitor +{ + template + std::string operator()(Renderer const&) const + { + return Renderer::renderer_type::name; + } +}; + class renderer_visitor { public: @@ -247,6 +256,44 @@ result_list runner::test_range(files_iterator begin, return results; } +void runner::parse_params(mapnik::parameters const & params, config & cfg) const +{ + cfg.status = *params.get("status", cfg.status); + + boost::optional sizes = params.get("sizes"); + + if (sizes) + { + cfg.sizes.clear(); + parse_map_sizes(*sizes, cfg.sizes); + } + + boost::optional tiles = params.get("tiles"); + + if (tiles) + { + cfg.tiles.clear(); + parse_map_sizes(*tiles, cfg.tiles); + } + + boost::optional bbox_string = params.get("bbox"); + + if (bbox_string) + { + cfg.bbox.from_string(*bbox_string); + } + + for (auto const & renderer : renderers_) + { + std::string renderer_name = mapnik::util::apply_visitor(renderer_name_visitor(), renderer); + boost::optional enabled = params.get(renderer_name); + if (enabled && !*enabled) + { + cfg.ignored_renderers.insert(renderer_name); + } + } +} + result_list runner::test_one(runner::path_type const& style_path, report_type & report, std::atomic & fail_count) const @@ -270,39 +317,13 @@ result_list runner::test_one(runner::path_type const& style_path, throw; } - mapnik::parameters const & params = map.get_extra_parameters(); + parse_params(map.get_extra_parameters(), cfg); - boost::optional status = params.get("status", cfg.status); - - if (!*status) + if (!cfg.status) { return results; } - boost::optional sizes = params.get("sizes"); - - if (sizes) - { - cfg.sizes.clear(); - parse_map_sizes(*sizes, cfg.sizes); - } - - boost::optional tiles = params.get("tiles"); - - if (tiles) - { - cfg.tiles.clear(); - parse_map_sizes(*tiles, cfg.tiles); - } - - boost::optional bbox_string = params.get("bbox"); - mapnik::box2d box; - - if (bbox_string) - { - box.from_string(*bbox_string); - } - std::string name(style_path.stem().string()); for (auto const & size : cfg.sizes) @@ -322,10 +343,16 @@ result_list runner::test_one(runner::path_type const& style_path, for (auto const & ren : renderers_) { + std::string renderer_name = mapnik::util::apply_visitor(renderer_name_visitor(), ren); + if (cfg.ignored_renderers.count(renderer_name)) + { + continue; + } + map.resize(size.width, size.height); - if (box.valid()) + if (cfg.bbox.valid()) { - map.zoom_to_box(box); + map.zoom_to_box(cfg.bbox); } else { diff --git a/test/visual/runner.hpp b/test/visual/runner.hpp index e38e394228..bf068ec220 100644 --- a/test/visual/runner.hpp +++ b/test/visual/runner.hpp @@ -59,6 +59,7 @@ class runner report_type & report, std::atomic & fail_limit) const; void parse_map_sizes(std::string const & str, std::vector & sizes) const; + void parse_params(mapnik::parameters const & params, config & cfg) const; const map_sizes_grammar map_sizes_parser_; const path_type styles_dir_; From cfd9668fdb27a567dd7cc17eb6f8c311d9f668e6 Mon Sep 17 00:00:00 2001 From: talaj Date: Wed, 13 Sep 2017 11:13:51 +0200 Subject: [PATCH 6/6] visual tests: report failed tests (#3765) --- test/visual/report.cpp | 60 +++++++++++++++++++++++++++++++----------- test/visual/report.hpp | 3 +++ 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/test/visual/report.cpp b/test/visual/report.cpp index c4cfc13207..57a8d32771 100644 --- a/test/visual/report.cpp +++ b/test/visual/report.cpp @@ -39,22 +39,7 @@ void console_report::report(result const & r) s << '-' << r.tiles.width << 'x' << r.tiles.height; } s << '-' << std::fixed << std::setprecision(1) << r.scale_factor << "\" with " << r.renderer_name << "... "; - - switch (r.state) - { - case STATE_OK: - s << "OK"; - break; - case STATE_FAIL: - s << "FAILED (" << r.diff << " different pixels)"; - break; - case STATE_OVERWRITE: - s << "OVERWRITTEN (" << r.diff << " different pixels)"; - break; - case STATE_ERROR: - s << "ERROR (" << r.error_message << ")"; - break; - } + report_state(r); if (show_duration) { @@ -115,9 +100,52 @@ unsigned console_report::summary(result_list const & results) s << "total: \t" << duration_cast(total).count() << " milliseconds" << std::endl; } + s << std::endl; + report_failures(results); + s << std::endl; + return fail + error; } +void console_report::report_state(result const & r) +{ + switch (r.state) + { + case STATE_OK: + s << "OK"; + break; + case STATE_FAIL: + s << "FAILED (" << r.diff << " different pixels)"; + break; + case STATE_OVERWRITE: + s << "OVERWRITTEN (" << r.diff << " different pixels)"; + break; + case STATE_ERROR: + s << "ERROR (" << r.error_message << ")"; + break; + } +} + +void console_report::report_failures(result_list const & results) +{ + for (auto const & r : results) + { + if (r.state == STATE_OK) + { + continue; + } + + s << r.name << " "; + report_state(r); + s << std::endl; + if (!r.actual_image_path.empty()) + { + s << " " << r.actual_image_path << " (actual)" << std::endl; + s << " " << r.reference_image_path << " (reference)" << std::endl; + } + } +} + void console_short_report::report(result const & r) { switch (r.state) diff --git a/test/visual/report.hpp b/test/visual/report.hpp index bb21bafd08..c65eddf1e7 100644 --- a/test/visual/report.hpp +++ b/test/visual/report.hpp @@ -48,6 +48,9 @@ class console_report unsigned summary(result_list const & results); protected: + void report_state(result const & r); + void report_failures(result_list const & results); + std::ostream & s; bool show_duration; };