.travis.yml

@@ -11,43 +11,42 @@ env:
    - HEAVY_JOBS="2"
    - PREFIX=/tmp/mapnik
    - secure: "F6ivqDNMBQQnrDGA9+7IX+GDswuIqQQd7YPJdQqa2Ked9jddAQDeJClb05ig3JlwfOlYLGZOd43ZX0pKuMtI2Gbkwz211agGP9S3YunwlRg8iWtJlO5kYFUdKCmJNhjg4icfkGELCgwXn+zuEWFSLpkPcjqAFKFlQrIJeAJJgKM="
-addons:
-  postgresql: "9.4"
 
 cache:
   directories:
   - $HOME/.ccache
 
-dist: precise
+dist: trusty
+sudo: false
 
 matrix:
   include:
     - os: linux
-      sudo: false
-      compiler: ": clang"
+      name: Linux gcc-6
       env: JOBS=4 CXX="ccache g++-6" CC="gcc-6"
       addons:
+        postgresql: "9.4"
         apt:
           sources: [ 'ubuntu-toolchain-r-test']
-          packages: [ 'libstdc++-6-dev', 'g++-6', 'xutils-dev']
+          packages: [ 'libstdc++-6-dev', 'g++-6', 'xutils-dev', 'postgresql-9.4-postgis-2.3' ]
     - os: linux
-      sudo: false
-      compiler: ": clang"
+      name: Linux clang-3.9
       env: JOBS=8 CXX="ccache clang++-3.9 -Qunused-arguments" CC="clang-3.9" ENABLE_GLIBC_WORKAROUND=true TRIGGER=true
       addons:
+        postgresql: "9.4"
         apt:
           sources: [ 'ubuntu-toolchain-r-test' ]
-          packages: [ 'libstdc++-4.9-dev', 'xutils-dev']
+          packages: [ 'libstdc++-4.9-dev', 'xutils-dev', 'postgresql-9.4-postgis-2.3' ]
     - os: linux
-      sudo: false
-      compiler: ": clang-coverage"
+      name: Linux clang-3.9 + coverage
       env: JOBS=8 COVERAGE=true CXX="ccache clang++-3.9 -Qunused-arguments" CC="clang-3.9"
       addons:
+        postgresql: "9.4"
         apt:
           sources: [ 'ubuntu-toolchain-r-test' ]
-          packages: ['libstdc++-4.9-dev', 'xutils-dev' ]
+          packages: ['libstdc++-4.9-dev', 'xutils-dev', 'postgresql-9.4-postgis-2.3' ]
     - os: osx
-      compiler: ": clang-osx"
+      name: OSX clang
       # https://docs.travis-ci.com/user/languages/objective-c/#Supported-OS-X-iOS-SDK-versions
       osx_image: xcode7.3 # upgrades clang from 6 -> 7
       env: JOBS=4 CXX="ccache clang++ -Qunused-arguments"
@@ -63,7 +62,7 @@ before_install:
  - export PATH=${PREFIX}/bin:$(pwd)/mason_packages/.link/bin:${PATH}
  - export COVERAGE=${COVERAGE:-false}
  - export BENCH=${BENCH:-false}
- - git_submodule_update --init
+ - git_submodule_update --init deps/
 
 install:
  - on 'osx' export DATA_PATH=$(brew --prefix)/var/postgres
@@ -101,14 +100,12 @@ script:
  # to ensure that slow builds still upload their
  # ccache results and therefore should be faster
  # (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;
- - enabled ${COVERAGE} coverage
- - enabled ${BENCH} make bench
+ - DEADLINE=$(( $(date +%s) + 40 * 60 ))
+ - scripts/travis-command-wrapper.py -s "date" -i 120 --deadline="$DEADLINE" make
+ - test_ok && git_submodule_update --init test/
+ - test_ok && make test
+ - test_ok && enabled ${COVERAGE} coverage
+ - test_ok && enabled ${BENCH} make bench
  - ./scripts/check_glibcxx.sh
 
 after_success:

CHANGELOG.md

@@ -6,6 +6,19 @@ Developers: Please commit along with changes.
 
 For a complete change history, see the git log.
 
+
+## UNRELEASED
+
+#### Core
+
+- Fixed compilation with ICU >= 61 (#3963)
+- Fixed bbox reprojection (#3940)
+
+#### Plugins
+
+- GDAL: Fixed several issues with overviews (#3939)
+
+
 ## 3.0.20
 
 Released: April 12, 2018

SConstruct

@@ -42,7 +42,7 @@ ICU_LIBS_DEFAULT='/usr/'
 
 DEFAULT_CC = "cc"
 DEFAULT_CXX = "c++"
-DEFAULT_CXX11_CXXFLAGS = " -std=c++11"
+DEFAULT_CXX11_CXXFLAGS = " -std=c++11 -DU_USING_ICU_NAMESPACE=0"
 DEFAULT_CXX11_LINKFLAGS = ""
 if sys.platform == 'darwin':
     # homebrew default

debian/changelog

+mapnik (3.0.21~rc1+ds-1~exp1) experimental; urgency=medium
+
+  * New upstream release candidate.
+  * Bump Standards-Version to 4.2.1, no changes.
+  * Update watch file to limit matches to archive path.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Tue, 02 Oct 2018 16:08:16 +0200
+
+mapnik (3.0.20+ds-2) unstable; urgency=medium
+
+  * Strip trailing whitespace from changelog & rules files.
+  * Bump Standards-Version to 4.1.5, no changes.
+  * Use filter instead of findstring to prevent partial matches.
+  * Drop autopkgtest to test installability.
+  * Add lintian override for testsuite-autopkgtest-missing.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Tue, 31 Jul 2018 21:18:48 +0200
+
 mapnik (3.0.20+ds-1) unstable; urgency=medium
 
   * Move from experimental to unstable.
@@ -794,4 +812,3 @@ mapnik (0.3.0+svn424-1) unstable; urgency=low
   * Initial release (Closes: #402792)
 
  -- Dominic Hargreaves <dom@earth.li>  Sun, 14 Jan 2007 15:53:35 +0000
-

debian/control

@@ -31,7 +31,7 @@ Build-Depends: debhelper (>= 9~),
                pkg-config,
                scons,
                zlib1g-dev
-Standards-Version: 4.1.4
+Standards-Version: 4.2.1
 Vcs-Browser: https://salsa.debian.org/debian-gis-team/mapnik
 Vcs-Git: https://salsa.debian.org/debian-gis-team/mapnik.git -b experimental
 Homepage: http://www.mapnik.org/

debian/rules

@@ -12,19 +12,19 @@ ifneq (,$(filter parallel=%,$(subst $(COMMA), ,$(DEB_BUILD_OPTIONS))))
 NJOBS := -j$(subst parallel=,,$(filter parallel=%,$(subst $(COMMA), ,$(DEB_BUILD_OPTIONS))))
 endif
 
-DEB_HOST_ARCH ?= $(shell dpkg-architecture -qDEB_HOST_ARCH)
+DEB_BUILD_ARCH ?= $(shell dpkg-architecture -qDEB_BUILD_ARCH)
 
 # Disable parallel builds for problematic architectures
-NO_PARALLEL_ARCHS = "arm64 armel armhf alpha hurd-i386 kfreebsd-i386 kfreebsd-amd64"
+NO_PARALLEL_ARCHS = arm64 armel armhf alpha hurd-i386 kfreebsd-i386 kfreebsd-amd64
 
 ifeq ($(shell dpkg-vendor --derives-from Ubuntu && echo yes),yes)
 	# From mapnik (3.0.8+ds-1ubuntu1):
 	# The amd64 buildds are castrated to 4G RAM and 4G swap,
 	# the ppc64el buildds to 12G RAM and 4G swap.
-	NO_PARALLEL_ARCHS += "amd64 ppc64el"
+	NO_PARALLEL_ARCHS += amd64 ppc64el
 endif
 
-ifneq (,$(findstring $(DEB_HOST_ARCH),$(NO_PARALLEL_ARCHS)))
+ifneq (,$(filter $(DEB_BUILD_ARCH),$(NO_PARALLEL_ARCHS)))
 	NJOBS = -j1
 endif
 
@@ -87,4 +87,3 @@ override_dh_prep:
 
 override_dh_makeshlibs:
 	dh_makeshlibs -V
-

debian/source/lintian-overrides

+# Not worth the effort
+testsuite-autopkgtest-missing
+

debian/tests/control

-# Test installability
-Depends: @
-Test-Command: /bin/true

debian/watch

@@ -5,4 +5,4 @@ uversionmangle=s/(\d)[_\.\-\+]?((RC|rc|pre|dev|beta|alpha)(?:-?offset)?\d*)$/$1~
 filenamemangle=s/(?:.*?\/)?(?:rel|v|mapnik)?[\-\_]?(\d\S+)\.(tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))/mapnik-$1.$2/,\
 repacksuffix=+ds \
 https://github.com/mapnik/mapnik/releases \
-(?:.*?archive/)?(?:rel|v|mapnik)?[\-\_]?(\d\S+)\.(?:tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))
+(?:.*?/archive/)?(?:rel|v|mapnik)?[\-\_]?(\d\S+)\.(?:tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))

include/mapnik/svg/svg_parser.hpp

@@ -74,6 +74,7 @@ public:
     svg_converter_type & path_;
     bool is_defs_;
     bool strict_;
+    bool ignore_;
     std::map<std::string, gradient> gradient_map_;
     std::map<std::string, boost::property_tree::detail::rapidxml::xml_node<char> const*> node_cache_;
     agg::trans_affine viewbox_tr_{};

include/mapnik/svg/svg_points_grammar_impl.hpp

@@ -50,7 +50,6 @@ svg_points_grammar<Iterator, PathType,SkipType>::svg_points_grammar()
     // commands
     function<move_to> move_to_;
     function<line_to> line_to_;
-    function<close> close_;
 
     start = coord[move_to_(_r1, _1, false)] // move_to
         >> *(-lit(',') >> coord [ line_to_(_r1, _1,false) ] ); // *line_to

include/mapnik/text/scrptrun.hpp

@@ -37,7 +37,7 @@ struct ParenStackEntry
     UScriptCode scriptCode = USCRIPT_INVALID_CODE;
 };
 
-class ScriptRun : public UObject {
+class ScriptRun : public icu::UObject {
 public:
     ScriptRun();
 

plugins/input/gdal/gdal_featureset.cpp

@@ -124,6 +124,33 @@ feature_ptr gdal_featureset::next()
     return feature_ptr();
 }
 
+void gdal_featureset::find_best_overview(int bandNumber,
+                                         int ideal_width,
+                                         int ideal_height,
+                                         int & current_width,
+                                         int & current_height) const
+{
+    GDALRasterBand * band = dataset_.GetRasterBand(bandNumber);
+    int band_overviews = band->GetOverviewCount();
+    if (band_overviews > 0)
+    {
+        for (int b = 0; b < band_overviews; b++)
+        {
+            GDALRasterBand * overview = band->GetOverview(b);
+            int overview_width = overview->GetXSize();
+            int overview_height = overview->GetYSize();
+            if ((overview_width < current_width ||
+                 overview_height < current_height) &&
+                ideal_width <= overview_width &&
+                ideal_height <= overview_height)
+            {
+                current_width = overview_width;
+                current_height = overview_height;
+            }
+        }
+    }
+}
+
 feature_ptr gdal_featureset::get_feature(mapnik::query const& q)
 {
     feature_ptr feature = feature_factory::create(ctx_,1);
@@ -208,77 +235,55 @@ feature_ptr gdal_featureset::get_feature(mapnik::query const& q)
     int im_width = width;
     double im_offset_x = x_off;
     double im_offset_y = y_off;
-    int current_width = (int)raster_width_;
-    int current_height = (int)raster_height_;
-
-    // loop through overviews -- snap up in resolution to closest overview if necessary
-    // we find an image size that most resembles the resolution of our output image.
-    double width_res = std::get<0>(q.resolution());
-    double height_res = std::get<1>(q.resolution());
-    int res_adjusted_raster_width = static_cast<int>(std::floor(((double)raster_width_ * width_res) + .5));
-    int res_adjusted_raster_height = static_cast<int>(std::floor(((double)raster_height_ * height_res) + .5));
+    int current_width = static_cast<int>(raster_width_);
+    int current_height = static_cast<int>(raster_height_);
+
+    // loop through overviews -- snap up in resolution to closest overview
+    // if necessary we find an image size that most resembles
+    // the resolution of our output image.
+    const double width_res = std::get<0>(q.resolution());
+    const double height_res = std::get<1>(q.resolution());
+    const int ideal_raster_width = static_cast<int>(
+        std::floor(raster_extent_.width() *
+            width_res * filter_factor) + .5);
+    const int ideal_raster_height = static_cast<int>(
+        std::floor(raster_extent_.height() *
+            height_res * filter_factor) + .5);
+
     if (band_ > 0 && band_ < nbands_)
     {
-        GDALRasterBand * band = dataset_.GetRasterBand(band_);
-        int band_overviews = band->GetOverviewCount();
-        if (band_overviews > 0)
-        {
-            for (int b = 0; b < band_overviews; b++)
-            {
-                GDALRasterBand * overview = band->GetOverview(b);
-                int overview_width = overview->GetXSize();
-                int overview_height = overview->GetYSize();
-                if ((overview_width < current_width || overview_height < current_height) &&
-                    res_adjusted_raster_width <= overview_width &&
-                    res_adjusted_raster_height <= overview_height)
-                {
-                    current_width = overview_width;
-                    current_height = overview_height;
-                }
-            }
-        }
+        find_best_overview(band_,
+                           ideal_raster_width,
+                           ideal_raster_height,
+                           current_width,
+                           current_height);
     }
     else
     {
         for (int i = 0; i < nbands_; ++i)
         {
-            GDALRasterBand * band = dataset_.GetRasterBand(i + 1);
-            int band_overviews = band->GetOverviewCount();
-            if (band_overviews > 0)
-            {
-                for (int b = 0; b < band_overviews; b++)
-                {
-                    GDALRasterBand * overview = band->GetOverview(b);
-                    int overview_width = overview->GetXSize();
-                    int overview_height = overview->GetYSize();
-                    if ((overview_width < current_width || overview_height < current_height) &&
-                        res_adjusted_raster_width <= overview_width &&
-                        res_adjusted_raster_height <= overview_height)
-                    {
-                        current_width = overview_width;
-                        current_height = overview_height;
+            find_best_overview(i + 1,
+                               ideal_raster_width,
+                               ideal_raster_height,
+                               current_width,
+                               current_height);
         }
     }
-            }
-        }
-    }
-    if (current_width != (int)raster_width_ || current_height != (int)raster_height_)
+
+    if (current_width != (int)raster_width_ ||
+        current_height != (int)raster_height_)
     {
         if (current_width != (int)raster_width_)
         {
             double ratio = (double)current_width / (double)raster_width_;
-            int adjusted_width = static_cast<int>(std::floor((ratio * im_width) + 0.5));
-            double adjusted_ratio = (double)adjusted_width / (double)im_width;
-            im_offset_x = adjusted_ratio * im_offset_x;
-            im_width = adjusted_width;
+            im_offset_x = std::floor(ratio * im_offset_x);
+            im_width = static_cast<int>(std::ceil(ratio * im_width));
         }
         if (current_height != (int)raster_height_)
         {
             double ratio = (double)current_height / (double)raster_height_;
-            int adjusted_height = static_cast<int>(std::floor((ratio * im_height) + 0.5));
-            double adjusted_ratio = (double)adjusted_height / (double)im_height;
-            im_offset_y = adjusted_ratio * im_offset_y;
-            im_height = adjusted_height;
+            im_offset_y = std::floor(ratio * im_offset_y);
+            im_height = static_cast<int>(std::ceil(ratio * im_height));
         }
     }
     

plugins/input/gdal/gdal_featureset.hpp

@@ -72,6 +72,12 @@ public:
     mapnik::feature_ptr next();
 
 private:
+    void find_best_overview(int bandNumber,
+                            int ideal_width,
+                            int ideal_height,
+                            int & current_width,
+                            int & current_height) const;
+
     mapnik::feature_ptr get_feature(mapnik::query const& q);
     mapnik::feature_ptr get_feature_at_point(mapnik::coord2d const& p);
     GDALDataset & dataset_;

scripts/travis-common.sh

@@ -28,6 +28,10 @@ on () {
     esac
 }
 
+test_ok () {
+    return $TRAVIS_TEST_RESULT
+}
+
 git_submodule_update () {
     git submodule update "$@" && return
     # failed, search branch and pull request heads for matching commit

src/proj_transform.cpp

@@ -21,13 +21,17 @@
  *****************************************************************************/
 
 // mapnik
-#include <mapnik/global.hpp>
 #include <mapnik/box2d.hpp>
+#include <mapnik/geometry.hpp>
+#include <mapnik/geometry_adapters.hpp>
 #include <mapnik/projection.hpp>
 #include <mapnik/proj_transform.hpp>
 #include <mapnik/coord.hpp>
 #include <mapnik/util/is_clockwise.hpp>
 
+// boost
+#include <boost/geometry/algorithms/envelope.hpp>
+
 #ifdef MAPNIK_USE_PROJ4
 // proj4
 #include <proj_api.h>
@@ -39,6 +43,56 @@
 
 namespace mapnik {
 
+namespace { // (local)
+
+// Returns points in clockwise order. This allows us to do anti-meridian checks.
+template <typename T>
+auto envelope_points(box2d<T> const& env, std::size_t num_points)
+    -> geometry::multi_point<T>
+{
+    auto width = env.width();
+    auto height = env.height();
+
+    geometry::multi_point<T> 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<double> & box) const
     return true;
 }
 
-// Returns points in clockwise order. This allows us to do anti-meridian checks.
-void envelope_points(std::vector< coord<double,2> > & coords, box2d<double>& env, int points)
-{
-    double width = env.width();
-    double height = env.height();
-
-    int steps;
-
-    if (points <= 4) {
-        steps = 0;
-    } else {
-        steps = static_cast<int>(std::ceil((points - 4) / 4.0));
-    }
-
-    steps += 1;
-    double xstep = width / steps;
-    double ystep = height / steps;
-
-    coords.resize(points);
-    for (int i=0; i<steps; i++) {
-        // top: left>right
-        coords[i] = coord<double, 2>(env.minx() + i * xstep, env.maxy());
-        // right: top>bottom
-        coords[i + steps] = coord<double, 2>(env.maxx(), env.maxy() - i * ystep);
-        // bottom: right>left
-        coords[i + steps * 2] = coord<double, 2>(env.maxx() - i * xstep, env.miny());
-        // left: bottom>top
-        coords[i + steps * 3] = coord<double, 2>(env.minx(), env.miny() + i * ystep);
-    }
-}
-
-box2d<double> calculate_bbox(std::vector<coord<double,2> > & points) {
-    std::vector<coord<double,2> >::iterator it = points.begin();
-    std::vector<coord<double,2> >::iterator it_end = points.end();
-
-    box2d<double> 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<double>& env, int points) const
         return backward(env);
     }
 
-    std::vector<coord<double,2> > 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<coord<double,2> >::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<double> result = calculate_bbox(coords);
+    box2d<double> 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<double>& env, int points) const
         return forward(env);
     }
 
-    std::vector<coord<double,2> > 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<coord<double,2> >::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<double> result = calculate_bbox(coords);
+    box2d<double> result;
+    boost::geometry::envelope(coords, result);
 
     if (is_dest_longlat_ && !util::is_clockwise(coords))
     {

src/svg/svg_parser.cpp

@@ -103,17 +103,17 @@ void parse_attr(svg_parser& parser, char const* name, char const* value);
 
 namespace {
 
-static std::array<unsigned, 7> const unsupported_elements
+static std::array<unsigned, 8> const unsupported_elements
 { {name_to_int("symbol"),
    name_to_int("marker"),
    name_to_int("view"),
    name_to_int("text"),
    name_to_int("switch"),
    name_to_int("image"),
-   name_to_int("a")}
+   name_to_int("a"),
+   name_to_int("clipPath")}
 };
 
-#if 0 // disable to reduce verbosity
 static std::array<unsigned, 43> const unsupported_attributes
 { {name_to_int("alignment-baseline"),
    name_to_int("baseline-shift"),
@@ -159,17 +159,19 @@ static std::array<unsigned, 43> const unsupported_attributes
    name_to_int("writing-mode")}
 };
 
-#endif
-
 template <typename T>
-void handle_unsupported(svg_parser& parser, T const& ar, char const* name)
+void handle_unsupported(svg_parser& parser, T const& ar, char const* name, char const* type)
 {
     unsigned element = name_to_int(name);
     for (auto const& e : ar)
     {
         if (e == element)
         {
-            parser.err_handler().on_error(std::string("SVG support error: <" + std::string(name) + "> element is not supported"));
+            parser.err_handler().on_error(std::string("SVG support error: <"
+                                                      + std::string(name)
+                                                      + "> "
+                                                      + std::string(type)
+                                                      +" is not supported"));
         }
     }
 }
@@ -383,6 +385,7 @@ std::pair<unsigned,bool> parse_preserve_aspect_ratio(T & err_handler, char const
 
 void traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node)
 {
+    if (parser.ignore_) return;
     auto const* name = node->name();
     switch (node->type())
     {
@@ -398,6 +401,11 @@ void traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node)
             }
             break;
         }
+        case name_to_int("clipPath"):
+        {
+            parser.ignore_ = true;
+            break;
+        }
         // the gradient tags *should* be in defs, but illustrator seems not to put them in there so
         // accept them anywhere
         case name_to_int("linearGradient"):
@@ -407,8 +415,7 @@ void traverse_tree(svg_parser & parser, rapidxml::xml_node<char> const* node)
             parse_radial_gradient(parser, node);
             break;
         case name_to_int("symbol"):
-            parse_id(parser, node);
-            //parse_dimensions(parser, node);
+            parser.ignore_ = true;
             break;
         }
 
@@ -497,6 +504,14 @@ void end_element(svg_parser & parser, rapidxml::xml_node<char> const* node)
             parser.is_defs_ = false;
         }
     }
+    else if(std::strcmp(name, "clipPath") == 0)
+    {
+        parser.ignore_ = false;
+    }
+    else if(std::strcmp(name,"symbol") == 0)
+    {
+        parser.ignore_ = false;
+}
 }
 
 void parse_element(svg_parser & parser, char const* name, rapidxml::xml_node<char> const* node)
@@ -535,7 +550,7 @@ void parse_element(svg_parser & parser, char const* name, rapidxml::xml_node<cha
         parse_dimensions(parser, node);
         break;
     default:
-        handle_unsupported(parser, unsupported_elements, name);
+        handle_unsupported(parser, unsupported_elements, name, "element");
         break;
     }
 }
@@ -709,8 +724,7 @@ void parse_attr(svg_parser & parser, char const* name, char const* value )
         }
         break;
     default:
-        //handle_unsupported(parser, unsupported_attributes, name);
-        // disable for now to reduce verbosity
+        handle_unsupported(parser, unsupported_attributes, name, "attribute");
         break;
     }
 }
@@ -1429,6 +1443,7 @@ svg_parser::svg_parser(svg_converter<svg_path_adapter,
                        agg::pod_bvector<mapnik::svg::path_attributes> > & path, bool strict)
     : path_(path),
       is_defs_(false),
+      ignore_(false),
       err_handler_(strict) {}
 
 svg_parser::~svg_parser() {}

src/text/text_layout.cpp

@@ -213,6 +213,7 @@ void text_layout::layout()
 // At the end everything that is left over is added as the final line.
 void text_layout::break_line_icu(std::pair<unsigned, unsigned> && line_limits)
 {
+    using BreakIterator = icu::BreakIterator;
     text_line line(line_limits.first, line_limits.second);
     shape_text(line);
 
@@ -234,7 +235,7 @@ void text_layout::break_line_icu(std::pair<unsigned, unsigned> && line_limits)
     }
 
     mapnik::value_unicode_string const& text = itemizer_.text();
-    Locale locale; // TODO: Is the default constructor correct?
+    icu::Locale locale; // TODO: Is the default constructor correct?
     UErrorCode status = U_ZERO_ERROR;
     std::unique_ptr<BreakIterator> breakitr(BreakIterator::createLineInstance(locale, status));
 
@@ -342,6 +343,7 @@ inline int adjust_last_break_position (int pos, bool repeat_wrap_char)
 
 void text_layout::break_line(std::pair<unsigned, unsigned> && line_limits)
 {
+    using BreakIterator = icu::BreakIterator;
     text_line line(line_limits.first, line_limits.second);
     shape_text(line);
     double scaled_wrap_width = wrap_width_ * scale_factor_;

test/unit/datasource/csv.cpp

@@ -241,7 +241,7 @@ TEST_CASE("csv") {
                     auto features = ds->features(query);
                     auto feature = features->next();
 
-                    require_attributes(feature, {
+                    REQUIRE_ATTRIBUTES(feature, {
                             attr { lon_name, mapnik::value_integer(0) },
                                 attr { "lat", mapnik::value_integer(0) }
                         });
@@ -295,11 +295,11 @@ TEST_CASE("csv") {
                         , attr { "Phone", mapnik::value_unicode_string("(212) 334-0711") }
                         , attr { "Address", mapnik::value_unicode_string("19 Elizabeth Street") }
                         , attr { "Precinct", mapnik::value_unicode_string("5th Precinct") }
-                        , attr { "geo_longitude", mapnik::value_integer(-70) }
-                        , attr { "geo_latitude", mapnik::value_integer(40) }
+                        , attr { "geo_longitude", mapnik::value_double(-70.0) }
+                        , attr { "geo_latitude", mapnik::value_double(40.0) }
                     };
-                require_attributes(feature, expected_attr);
-                require_attributes(feature2, expected_attr);
+                REQUIRE_ATTRIBUTES(feature, expected_attr);
+                REQUIRE_ATTRIBUTES(feature2, expected_attr);
                 if (mapnik::util::exists(filepath + ".index"))
                 {
                     mapnik::util::remove(filepath + ".index");
@@ -367,7 +367,7 @@ TEST_CASE("csv") {
 
                 auto featureset = all_features(ds);
                 auto feature = featureset->next();
-                require_attributes(feature, {
+                REQUIRE_ATTRIBUTES(feature, {
                         attr { "x", mapnik::value_integer(0) }
                         , attr { "empty_column", mapnik::value_unicode_string("") }
                         , attr { "text", mapnik::value_unicode_string("a b") }
@@ -416,15 +416,15 @@ TEST_CASE("csv") {
                 require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String});
 
                 auto featureset = all_features(ds);
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}
                         , attr{"y", 0}
                         , attr{"name", mapnik::value_unicode_string("a/a") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 1}
                         , attr{"y", 4}
                         , attr{"name", mapnik::value_unicode_string("b/b") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 10}
                         , attr{"y", 2.5}
                         , attr{"name", mapnik::value_unicode_string("c/c") } });
@@ -531,7 +531,7 @@ TEST_CASE("csv") {
                 auto fields = ds->get_descriptor().get_descriptors();
                 require_field_names(fields, {"x", "y", "1990", "1991", "1992"});
                 auto feature = all_features(ds)->next();
-                require_attributes(feature, {
+                REQUIRE_ATTRIBUTES(feature, {
                         attr{"x", 0}
                         , attr{"y", 0}
                         , attr{"1990", 1}
@@ -575,15 +575,15 @@ TEST_CASE("csv") {
                 require_field_names(fields, {"x", "y", "label"});
                 auto featureset = all_features(ds);
 
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"label", ustring("0,0") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 5}, attr{"y", 5}, attr{"label", ustring("5,5") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 5}, attr{"label", ustring("0,5") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 5}, attr{"y", 0}, attr{"label", ustring("5,0") } });
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 2.5}, attr{"y", 2.5}, attr{"label", ustring("2.5,2.5") } });
                 if (mapnik::util::exists(filename + ".index"))
                 {
@@ -615,7 +615,7 @@ TEST_CASE("csv") {
                     auto ds = get_csv_ds(filename);
                     auto fields = ds->get_descriptor().get_descriptors();
                     require_field_names(fields, {"x", "y", "z"});
-                    require_attributes(all_features(ds)->next(), {
+                    REQUIRE_ATTRIBUTES(all_features(ds)->next(), {
                             attr{"x", 1}, attr{"y", 10}, attr{"z", 9999.9999} });
                     if (mapnik::util::exists(filename + ".index"))
                     {
@@ -653,7 +653,7 @@ TEST_CASE("csv") {
                     auto ds = get_csv_ds(filename);
                     auto fields = ds->get_descriptor().get_descriptors();
                     require_field_names(fields, {"x", "y", "line"});
-                    require_attributes(all_features(ds)->next(), {
+                    REQUIRE_ATTRIBUTES(all_features(ds)->next(), {
                             attr{"x", 0}, attr{"y", 0}
                             , attr{"line", ustring("many\n  lines\n  of text\n  with unix newlines")} });
                     if (mapnik::util::exists(filename + ".index"))
@@ -684,7 +684,7 @@ TEST_CASE("csv") {
                 auto ds = get_csv_ds(filename);
                 auto fields = ds->get_descriptor().get_descriptors();
                 require_field_names(fields, {"x", "y", "z"});
-                require_attributes(all_features(ds)->next(), {
+                REQUIRE_ATTRIBUTES(all_features(ds)->next(), {
                         attr{"x", -122}, attr{"y", 48}, attr{"z", 0} });
                 if (mapnik::util::exists(filename + ".index"))
                 {
@@ -719,7 +719,7 @@ TEST_CASE("csv") {
                     auto ds = get_csv_ds(filename);
                     auto fields = ds->get_descriptor().get_descriptors();
                     require_field_names(fields, {"x", "y", "z"});
-                    require_attributes(all_features(ds)->next(), {
+                    REQUIRE_ATTRIBUTES(all_features(ds)->next(), {
                             attr{"x", 0}, attr{"y", 0}, attr{"z", ustring("hello")} });
                     if (mapnik::util::exists(filename + ".index"))
                     {
@@ -754,9 +754,9 @@ TEST_CASE("csv") {
                 require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String, mapnik::Boolean});
 
                 auto featureset = all_features(ds);
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"null", ustring("null")}, attr{"boolean", true}});
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"null", ustring("")}, attr{"boolean", false}});
 
                 if (mapnik::util::exists(filename + ".index"))
@@ -829,11 +829,11 @@ TEST_CASE("csv") {
                 require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String});
 
                 auto featureset = all_features(ds);
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("001")}});
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("003")}});
-                require_attributes(featureset->next(), {
+                REQUIRE_ATTRIBUTES(featureset->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"fips", ustring("005")}});
                 if (mapnik::util::exists(filename + ".index"))
                 {
@@ -990,7 +990,7 @@ TEST_CASE("csv") {
                 auto fields = ds->get_descriptor().get_descriptors();
                 require_field_names(fields, {"x", "y", "name"});
                 require_field_types(fields, {mapnik::Integer, mapnik::Integer, mapnik::String});
-                require_attributes(all_features(ds)->next(), {
+                REQUIRE_ATTRIBUTES(all_features(ds)->next(), {
                         attr{"x", 0}, attr{"y", 0}, attr{"name", ustring("data_name")} });
                 REQUIRE(count_features(all_features(ds)) == r.second);
                 CHECK(ds->get_geometry_type() == mapnik::datasource_geometry_t::Point);
@@ -1007,13 +1007,13 @@ TEST_CASE("csv") {
 
             auto fs = all_features(ds);
             auto feature = fs->next();
-            require_attributes(feature, {
+            REQUIRE_ATTRIBUTES(feature, {
                     attr{"x", 0}, attr{"y", 0}, attr{"bigint", 2147483648} });
 
             feature = fs->next();
-            require_attributes(feature, {
+            REQUIRE_ATTRIBUTES(feature, {
                     attr{"x", 0}, attr{"y", 0}, attr{"bigint", 9223372036854775807ll} });
-            require_attributes(feature, {
+            REQUIRE_ATTRIBUTES(feature, {
                     attr{"x", 0}, attr{"y", 0}, attr{"bigint", 0x7FFFFFFFFFFFFFFFll} });
         } // END SECTION
 #pragma GCC diagnostic pop

test/unit/datasource/ds_test_util.hpp

@@ -107,18 +107,20 @@ inline std::size_t count_features(mapnik::featureset_ptr features) {
 
 using attr = std::tuple<std::string, mapnik::value>;
 
-#define REQUIRE_ATTRIBUTES(feature, attrs) \
-    REQUIRE(bool(feature)); \
-    for (auto const &kv : attrs) { \
-        REQUIRE(feature->has_key(std::get<0>(kv))); \
-        CHECK(feature->get(std::get<0>(kv)) == std::get<1>(kv)); \
+#define REQUIRE_ATTRIBUTES(feature, ...) \
+    do { \
+        auto const& _feat = (feature); /* evaluate feature only once */ \
+        REQUIRE(_feat != nullptr); \
+        for (auto const& kv : __VA_ARGS__) { \
+            auto& key = std::get<0>(kv); \
+            auto& val = std::get<1>(kv); \
+            CAPTURE(key); \
+            CHECKED_IF(_feat->has_key(key)) { \
+                CHECK(_feat->get(key) == val); \
+                CHECK(_feat->get(key).which() == val.which()); \
             } \
-
-
-inline void require_attributes(mapnik::feature_ptr feature,
-                        std::initializer_list<attr> const &attrs) {
-    REQUIRE_ATTRIBUTES(feature, attrs);
-}
+        } \
+    } while (0)
 
 namespace detail {
 struct feature_count {