CHANGELOG.md

@@ -8,12 +8,15 @@ For a complete change history, see the git log.
 
 ## 3.0.18
 
-Released: xxxxxx XX, 201X
+Released: January 26, 2018
 
-(Packaged from xxxxxxxxx)
+(Packaged from 44ef46c81)
 
     - SVG parser - fixed logic for calculating dimensions when `width` and `height` expressed in
       percentage units (#3812)
+    - New improved `interior` placement algorithm (#3839)
+    - Fixed handling of an empty interior rings in `polygon_vertex_processor` (#3833)
+    - Fixed handling of an empty interior rings in `vertex_adapter' (#3842)(#3838)
 
 ## 3.0.17
 

debian/changelog

-mapnik (3.0.18~rc1+ds-1~exp2) UNRELEASED; urgency=medium
+mapnik (3.0.18+ds-1~exp1) experimental; urgency=medium
 
+  * New upstream release.
   * Update copyright-format URL to use HTTPS.
+  * Bump Standards-Version to 4.1.3, no changes.
 
- -- Bas Couwenberg <sebastic@debian.org>  Sat, 20 Jan 2018 22:15:30 +0100
+ -- Bas Couwenberg <sebastic@debian.org>  Fri, 26 Jan 2018 17:13:45 +0100
 
 mapnik (3.0.18~rc1+ds-1~exp1) experimental; urgency=medium
 

debian/control

@@ -31,7 +31,7 @@ Build-Depends: debhelper (>= 9~),
                pkg-config,
                scons,
                zlib1g-dev
-Standards-Version: 4.1.2
+Standards-Version: 4.1.3
 Vcs-Browser: https://anonscm.debian.org/cgit/pkg-grass/mapnik.git
 Vcs-Git: https://anonscm.debian.org/git/pkg-grass/mapnik.git
 Homepage: http://www.mapnik.org/

include/mapnik/geom_util.hpp

@@ -518,80 +518,6 @@ bool hit_test(PathType & path, double x, double y, double tol)
     return inside;
 }
 
-template <typename PathType>
-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<double> intersections; // only need to store the X as we know the y
-    geometry::point<double> 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

include/mapnik/geometry/interior.hpp

+/*****************************************************************************
+ *
+ * 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 <mapnik/geometry.hpp>
+#include <mapnik/config.hpp> // for MAPNIK_DECL
+
+namespace mapnik { namespace geometry {
+
+template <class T>
+MAPNIK_DECL bool interior(polygon<T> const& polygon,
+                          double scale_factor,
+                          point<T> & pt);
+
+} }
+
+#endif // MAPNIK_GEOMETRY_INTERIOR_HPP

include/mapnik/geometry/polygon_vertex_processor.hpp

+/*****************************************************************************
+ *
+ * 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 <mapnik/geometry.hpp>
+
+namespace mapnik { namespace geometry {
+
+template <typename T>
+struct polygon_vertex_processor
+{
+    template <typename Path>
+    void add_path(Path & path)
+    {
+        point<T> p;
+        unsigned cmd;
+        linear_ring<T> 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:
+                    if (!ring.empty())
+                    {
+                        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<T>();
+                    break;
+            }
+        }
+    }
+
+    polygon<T> polygon_;
+};
+
+} }
+
+#endif // MAPNIK_GEOMETRY_POLYGON_VERTEX_PROCESSOR_HPP

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

include/mapnik/markers_placements/interior.hpp

@@ -26,6 +26,8 @@
 #include <mapnik/markers_placements/point.hpp>
 #include <mapnik/geom_util.hpp>
 #include <mapnik/geometry_types.hpp>
+#include <mapnik/geometry/interior.hpp>
+#include <mapnik/geometry/polygon_vertex_processor.hpp>
 
 namespace mapnik {
 
@@ -58,11 +60,19 @@ public:
         }
         else
         {
-            if (!label::interior_position(this->locator_, x, y))
+            geometry::polygon_vertex_processor<double> vertex_processor;
+            vertex_processor.add_path(this->locator_);
+            geometry::point<double> placement;
+            if (!geometry::interior(vertex_processor.polygon_,
+                                   this->params_.scale_factor,
+                                   placement))
             {
                 this->done_ = true;
                 return false;
             }
+
+            x = placement.x;
+            y = placement.y;
         }
 
         angle = 0;

include/mapnik/renderer_common/process_point_symbolizer.hpp

@@ -32,6 +32,7 @@
 #include <mapnik/geometry_centroid.hpp>
 #include <mapnik/geometry_type.hpp>
 #include <mapnik/geometry_types.hpp>
+#include <mapnik/geometry/interior.hpp>
 #include <mapnik/vertex_adapters.hpp>
 #include <mapnik/geom_util.hpp>
 
@@ -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::polygon<double> >(geometry);
-            geometry::polygon_vertex_adapter<double> va(poly);
-            if (!label::interior_position(va ,pt.x, pt.y))
-                return;
+            if (!geometry::interior(poly, common.scale_factor_, pt)) return;
         }
         else
         {

src/build.py

@@ -169,6 +169,7 @@ source = Split(
     datasource_cache_static.cpp
     debug.cpp
     geometry_reprojection.cpp
+    geometry/interior.cpp
     expression_node.cpp
     expression_string.cpp
     expression.cpp

src/geometry/interior.cpp

+/*****************************************************************************
+ *
+ * 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 <mapnik/geometry/interior.hpp>
+#include <mapnik/geometry_envelope.hpp>
+#include <mapnik/box2d.hpp>
+#include <mapnik/geometry_centroid.hpp>
+
+#include <algorithm>
+#include <cmath>
+#include <iostream>
+#include <queue>
+
+#pragma GCC diagnostic push
+#include <mapnik/warning_ignore.hpp>
+#include <boost/optional.hpp>
+#pragma GCC diagnostic pop
+
+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 <class T>
+T segment_dist_sq(const point<T>& p,
+                  const point<T>& a,
+                  const point<T>& 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 <class T>
+void point_to_ring_dist(point<T> const& point, linear_ring<T> 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 <class T>
+double point_to_polygon_dist(const point<T>& point, const polygon<T>& polygon)
+{
+    bool inside = false;
+    double min_dist_sq = std::numeric_limits<double>::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 <class T>
+struct fitness_functor
+{
+    fitness_functor(point<T> const& centroid, point<T> const& polygon_size)
+        : centroid(centroid),
+          max_size(std::max(polygon_size.x, polygon_size.y))
+        {}
+
+    T operator()(point<T> const& cell_center, T distance_polygon) const
+    {
+        if (distance_polygon <= 0)
+        {
+            return distance_polygon;
+        }
+        point<T> 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<T> centroid;
+    T max_size;
+};
+
+template <class T>
+struct cell
+{
+    template <class FitnessFunc>
+    cell(const point<T>& c_, T h_,
+         const polygon<T>& 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<T> 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 <class T>
+boost::optional<point<T>> polylabel(polygon<T> const& polygon, T precision = 1)
+{
+    if (polygon.exterior_ring.empty())
+    {
+        return boost::none;
+    }
+
+    // find the bounding box of the outer ring
+    const box2d<T> bbox = envelope(polygon.exterior_ring);
+    const point<T> 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<T>& a, const cell<T>& b)
+    {
+        return a.max_fitness < b.max_fitness;
+    };
+    using Queue = std::priority_queue<cell<T>, std::vector<cell<T>>, decltype(compare_func)>;
+    Queue queue(compare_func);
+
+    if (cell_size == 0)
+    {
+        return point<T>{ bbox.minx(), bbox.miny() };
+    }
+
+    point<T> centroid;
+    if (!mapnik::geometry::centroid(polygon, centroid))
+    {
+        auto center = bbox.center();
+        return point<T>{ center.x, center.y };
+    }
+
+    fitness_functor<T> 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<T>({x + h, y + h}, h, polygon, fitness_func));
+        }
+    }
+
+    // take centroid as the first best guess
+    auto best_cell = cell<T>(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<T>({current_cell.c.x - h, current_cell.c.y - h}, h, polygon, fitness_func));
+        queue.push(cell<T>({current_cell.c.x + h, current_cell.c.y - h}, h, polygon, fitness_func));
+        queue.push(cell<T>({current_cell.c.x - h, current_cell.c.y + h}, h, polygon, fitness_func));
+        queue.push(cell<T>({current_cell.c.x + h, current_cell.c.y + h}, h, polygon, fitness_func));
+    }
+
+    return best_cell.c;
+}
+
+} // namespace detail
+
+template <class T>
+bool interior(polygon<T> const& polygon, double scale_factor, point<T> & pt)
+{
+    // This precision has been chosen to work well in the map (viewport) coordinates.
+    double precision = 10.0 * scale_factor;
+    if (boost::optional<point<T>> opt = detail::polylabel(polygon, precision))
+    {
+        pt = *opt;
+        return true;
+    }
+
+    return false;
+}
+
+template
+bool interior(polygon<double> const& polygon, double scale_factor, point<double> & pt);
+
+} }
+

src/renderer_common/render_markers_symbolizer.cpp

@@ -228,7 +228,8 @@ markers_dispatch_params::markers_dispatch_params(box2d<double> const& size,
         get<value_double, keys::max_error>(sym, feature, vars),
         get<value_bool, keys::allow_overlap>(sym, feature, vars),
         get<value_bool, keys::avoid_edges>(sym, feature, vars),
-        get<direction_enum, keys::direction>(sym, feature, vars)}
+        get<direction_enum, keys::direction>(sym, feature, vars),
+        scale}
     , placement_method(get<marker_placement_enum, keys::markers_placement_type>(sym, feature, vars))
     , ignore_placement(get<value_bool, keys::ignore_placement>(sym, feature, vars))
     , snap_to_pixels(snap)

src/text/symbolizer_helpers.cpp

@@ -31,6 +31,7 @@
 #include <mapnik/geometry.hpp>
 #include <mapnik/geometry_type.hpp>
 #include <mapnik/geometry_centroid.hpp>
+#include <mapnik/geometry/interior.hpp>
 #include <mapnik/vertex_processor.hpp>
 #include <mapnik/geom_util.hpp>
 #include <mapnik/parse_path.hpp>
@@ -40,6 +41,10 @@
 #include <mapnik/text/placement_finder_impl.hpp>
 #include <mapnik/text/placements/base.hpp>
 #include <mapnik/text/placements/dummy.hpp>
+#include <mapnik/geometry_transform.hpp>
+#include <mapnik/geometry_strategy.hpp>
+#include <mapnik/proj_strategy.hpp>
+#include <mapnik/view_strategy.hpp>
 
 namespace mapnik {
 namespace geometry {
@@ -288,9 +293,19 @@ void base_symbolizer_helper::initialize_points() const
             }
             else if (how_placed == INTERIOR_PLACEMENT && type == geometry::geometry_types::Polygon)
             {
-                auto const& poly = mapnik::util::get<geometry::polygon<double> >(geom);
-                geometry::polygon_vertex_adapter<double> va(poly);
-                success = label::interior_position(va, label_x, label_y);
+                auto const& poly = util::get<geometry::polygon<double>>(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<proj_strategy, view_strategy>;
+                transform_group_type transform_group(ps, vs);
+                geometry::polygon<double> tranformed_poly(geometry::transform<double>(poly, transform_group));
+                geometry::point<double> pt;
+                if (geometry::interior(tranformed_poly, scale_factor_, pt))
+                {
+                    points_.emplace_back(pt.x, pt.y);
+                }
+                continue;
             }
             else
             {

src/vertex_adapters.cpp

@@ -146,6 +146,12 @@ unsigned polygon_vertex_adapter<T>::vertex(coord_type * x, coord_type * y) const
     {
         current_index_ = 0;
         end_index_ = poly_.interior_rings[rings_itr_ - 1].size();
+        if (end_index_ == 0)
+        {
+            *x = 0;
+            *y = 0;
+            return mapnik::SEG_CLOSE;
+        }
         point<T> const& coord = poly_.interior_rings[rings_itr_ - 1][current_index_++];
         *x = coord.x;
         *y = coord.y;

test/unit/geometry/interior.cpp

+#include "catch.hpp"
+
+#include <mapnik/geometry/interior.hpp>
+
+TEST_CASE("polygon interior") {
+
+SECTION("empty polygon") {
+
+    mapnik::geometry::polygon<double> poly;
+    mapnik::geometry::point<double> pt;
+
+    CHECK(!mapnik::geometry::interior(poly, 1.0, pt));
+}
+
+SECTION("interior of a square") {
+
+    mapnik::geometry::polygon<double> 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<double> pt{ -3, -3 };
+
+    CHECK(mapnik::geometry::interior(poly, 1.0, pt));
+    CHECK(pt.x == Approx(0));
+    CHECK(pt.y == Approx(0));
+}
+
+}

test/unit/geometry/polygon_vertex_processor.cpp

+#include "catch.hpp"
+#include "unit/vertex_adapter/fake_path.hpp"
+
+#include <mapnik/geometry/polygon_vertex_processor.hpp>
+
+TEST_CASE("polygon_vertex_processor") {
+
+SECTION("empty polygon") {
+
+    fake_path path = {};
+    mapnik::geometry::polygon_vertex_processor<double> 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<double> 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<double> 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));
+}
+
+}

test/unit/vertex_adapter/fake_path.hpp

+/*****************************************************************************
+ *
+ * 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 <mapnik/vertex.hpp>
+
+// stl
+#include <vector>
+#include <tuple>
+
+namespace detail
+{
+
+template <typename T>
+struct fake_path
+{
+    using coord_type = std::tuple<T, T, unsigned>;
+    using cont_type = std::vector<coord_type>;
+    cont_type vertices_;
+    typename cont_type::iterator itr_;
+
+    fake_path(std::initializer_list<T> l)
+        : fake_path(l.begin(), l.size())
+    {
+    }
+
+    fake_path(std::vector<T> const &v, bool make_invalid = false)
+        : fake_path(v.begin(), v.size(), make_invalid)
+    {
+    }
+
+    template <typename Itr>
+    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<double>;
+

test/unit/vertex_adapter/vertex_adapter.cpp

-
 #include "catch.hpp"
 
 #include <mapnik/vertex_adapters.hpp>
@@ -267,4 +266,56 @@ SECTION("polygon with hole") {
     REQUIRE( y == 0 );
 }
 
+SECTION("polygon with empty interior ring") {
+    mapnik::geometry::polygon<double> g;
+    g.exterior_ring.add_coord(-1, -1);
+    g.exterior_ring.add_coord( 1, -1);
+    g.exterior_ring.add_coord( 1,  1);
+    g.exterior_ring.add_coord(-1,  1);
+    g.exterior_ring.add_coord(-1, -1);
+
+    // Emplace empty interior ring
+    mapnik::geometry::linear_ring<double> empty_ring;
+    g.add_hole(std::move(empty_ring));
+
+    mapnik::geometry::polygon_vertex_adapter<double> va(g);
+    double x,y;
+    unsigned cmd;
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_MOVETO );
+    CHECK( x == Approx(-1) );
+    CHECK( y == Approx(-1) );
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_LINETO );
+    CHECK( x == Approx(1) );
+    CHECK( y == Approx(-1) );
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_LINETO );
+    CHECK( x == Approx(1) );
+    CHECK( y == Approx(1) );
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_LINETO );
+    CHECK( x == Approx(-1) );
+    CHECK( y == Approx(1) );
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_CLOSE );
+    CHECK( x == Approx(0) );
+    CHECK( y == Approx(0) );
+
+    cmd = va.vertex(&x,&y);
+    CHECK( cmd == mapnik::SEG_CLOSE );
+    CHECK( x == Approx(0) );
+    CHECK( y == Approx(0) );
+
+    cmd = va.vertex(&x,&y);
+    REQUIRE( cmd == mapnik::SEG_END );
+    REQUIRE( x == Approx(0) );
+    REQUIRE( y == Approx(0) );
+}
+
 }

test/visual/config.hpp

@@ -25,12 +25,15 @@
 
 // stl
 #include <vector>
+#include <set>
 #include <string>
 #include <chrono>
 
 // boost
 #include <boost/filesystem.hpp>
 
+#include <mapnik/box2d.hpp>
+
 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<double> scales;
     std::vector<map_size> sizes;
     std::vector<map_size> tiles;
+    mapnik::box2d<double> bbox;
+    std::set<std::string> ignored_renderers;
 };
 
 enum result_state : std::uint8_t

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<milliseconds>(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)