.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,24 @@ Developers: Please commit along with changes.
 
 For a complete change history, see the git log.
 
+
+## 3.0.21
+
+Released: October 8, 2018
+
+(Packaged from 1dbb1d2c1)
+
+#### Core
+
+- Fixed compilation with ICU >= 61 (#3963)
+- Fixed bbox reprojection (#3940)
+- SVG: enabled unsupported attributes handling
+
+#### 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.20+ds-3) UNRELEASED; urgency=medium
+mapnik (3.0.21+ds-2) unstable; urgency=medium
 
+  * Add upstream patch to use pkg-config for freetype2.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Thu, 25 Oct 2018 15:24:53 +0200
+
+mapnik (3.0.21+ds-1) unstable; urgency=medium
+
+  * Move from experimental to unstable.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Tue, 09 Oct 2018 18:23:50 +0200
+
+mapnik (3.0.21+ds-1~exp1) experimental; urgency=medium
+
+  * New upstream release.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Mon, 08 Oct 2018 16:54:28 +0200
+
+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>  Sun, 05 Aug 2018 20:28:06 +0200
+ -- Bas Couwenberg <sebastic@debian.org>  Tue, 02 Oct 2018 16:08:16 +0200
 
 mapnik (3.0.20+ds-2) unstable; urgency=medium
 

debian/patches/freetype-pkgconfig.patch → debian/patches/0001-Use-pkg-config-to-find-FreeType2-if-available.patch

-Description: Use pkg-config when freetype-config is not available.
-Author: Bas Couwenberg <sebastic@debian.org>
+Description: Use pkg-config to find FreeType2 if available
+Author: Raul Marin <rmrodriguez@cartodb.com>
+Origin: https://github.com/mapnik/mapnik/pull/3892/commits/23755a527a5e0f24a7591fcc41dece7ce5f080b7
 Bug: https://github.com/mapnik/mapnik/issues/3870
 Bug-Debian: https://bugs.debian.org/892451
 
 --- a/SConstruct
 +++ b/SConstruct
-@@ -1393,6 +1393,8 @@ if not preconfigured:
+@@ -1393,6 +1393,7 @@ if not preconfigured:
          ['harfbuzz', 'harfbuzz/hb.h',True,'C++']
      ]
  
 +    CHECK_PKG_CONFIG = conf.CheckPKGConfig('0.15.0')
-+
      if env.get('FREETYPE_LIBS') or env.get('FREETYPE_INCLUDES'):
          REQUIRED_LIBSHEADERS.insert(0,['freetype','ft2build.h',True,'C'])
          if env.get('FREETYPE_INCLUDES'):
-@@ -1413,6 +1415,15 @@ if not preconfigured:
-                     env['EXTRA_FREETYPE_LIBS'].append('bz2')
-             except OSError as e:
-                 pass
-+    elif env['RUNTIME_LINK'] == 'static':
-+        temp_env = env.Clone()
-+        temp_env['LIBS'] = []
+@@ -1401,6 +1402,21 @@ if not preconfigured:
+         if env.get('FREETYPE_LIBS'):
+             lib_path = env['FREETYPE_LIBS']
+             env.AppendUnique(LIBPATH = fix_path(lib_path))
++    elif CHECK_PKG_CONFIG and conf.CheckPKG('freetype2'):
++        # Freetype 2.9+ doesn't use freetype-config and uses pkg-config instead
++        cmd = 'pkg-config freetype2 --libs --cflags'
++        if env['RUNTIME_LINK'] == 'static':
++            cmd += ' --static'
++
++        temp_env = Environment(ENV=os.environ)
 +        try:
-+            temp_env.ParseConfig('pkg-config freetype2 --libs')
-+            if 'bz2' in temp_env['LIBS']:
-+                env['EXTRA_FREETYPE_LIBS'].append('bz2')
++            temp_env.ParseConfig(cmd)
++            for lib in temp_env['LIBS']:
++                env.AppendUnique(LIBPATH = fix_path(lib))
++            for inc in temp_env['CPPPATH']:
++                env.AppendUnique(CPPPATH = fix_path(inc))
 +        except OSError as e:
 +            pass
- 
-     # libxml2 should be optional but is currently not
-     # https://github.com/mapnik/mapnik/issues/913
-@@ -1635,8 +1646,6 @@ if not preconfigured:
+     elif conf.parse_config('FREETYPE_CONFIG'):
+         # check if freetype links to bz2
+         if env['RUNTIME_LINK'] == 'static':
+@@ -1634,9 +1650,6 @@ if not preconfigured:
+             color_print(1,'%s not detected on your system' % env['QUERIED_ICU_DATA'] )
              env['MISSING_DEPS'].append('ICU_DATA')
  
- 
+-
 -    CHECK_PKG_CONFIG = conf.CheckPKGConfig('0.15.0')
 -
      if len(env['REQUESTED_PLUGINS']):

debian/patches/series

-freetype-pkgconfig.patch
+0001-Use-pkg-config-to-find-FreeType2-if-available.patch

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

include/mapnik/version.hpp

@@ -27,7 +27,7 @@
 
 #define MAPNIK_MAJOR_VERSION 3
 #define MAPNIK_MINOR_VERSION 0
-#define MAPNIK_PATCH_VERSION 20
+#define MAPNIK_PATCH_VERSION 21
 
 #define MAPNIK_VERSION (MAPNIK_MAJOR_VERSION*100000) + (MAPNIK_MINOR_VERSION*100) + (MAPNIK_PATCH_VERSION)
 

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 {

test/unit/datasource/geojson.cpp

@@ -834,7 +834,7 @@ TEST_CASE("geojson") {
                     std::initializer_list<attr> attrs = {
                         attr{"name", tr.transcode("Test")},
                         attr{"NOM_FR", tr.transcode("Québec")},
-                        attr{"boolean", mapnik::value_bool("true")},
+                        attr{"boolean", mapnik::value_bool(true)},
                         attr{"description", tr.transcode("Test: \u005C")},
                         attr{"double", mapnik::value_double(1.1)},
                         attr{"int", mapnik::value_integer(1)},

test/unit/datasource/topojson.cpp

@@ -92,7 +92,7 @@ TEST_CASE("topojson")
             std::initializer_list<attr> attrs = {
                 attr{"name", tr.transcode("Test")},
                 attr{"NOM_FR", tr.transcode("Québec")},
-                attr{"boolean", mapnik::value_bool("true")},
+                attr{"boolean", mapnik::value_bool(true)},
                 attr{"description", tr.transcode("Test: \u005C")},
                 attr{"double", mapnik::value_double(1.1)},
                 attr{"int", mapnik::value_integer(1)},