Merge branch 'master' into boost_1_67

.travis.yml

@@ -106,10 +106,7 @@ script:
  # (and might work) for the next build
  - DURATION=2400
  - scripts/travis-command-wrapper.py -s "date" -i 120 --deadline=$(( $(date +%s) + ${DURATION} )) make
- - RESULT=0
+ - make test
- - 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
  - ./scripts/check_glibcxx.sh

INSTALL.md

@@ -6,6 +6,7 @@ First clone mapnik from github and initialize submodules
 
 ```bash
 git clone https://github.com/mapnik/mapnik.git
+cd mapnik
 git submodule update --init
 ```
 
@@ -191,6 +192,6 @@ Mapnik is great for building your own mapping applications. Visit
 https://github.com/mapnik/mapnik/wiki/LearningMapnik for basic
 tutorials on how to programmatically use Mapnik.
 
-### Contributers
+### Contributors
 
-Read docs/contributing.md for resources for getting involved with Mapnik development.
+Read [docs/contributing.md](docs/contributing.md) for resources for getting involved with Mapnik development.

SConstruct

@@ -34,6 +34,20 @@ try:
 except:
     HAS_DISTUTILS = False
 
+try:
+    # Python 3.3+
+    from shlex import quote as shquote
+except:
+    # Python 2.7
+    from pipes import quote as shquote
+
+try:
+    # Python 3.3+
+    from subprocess import DEVNULL
+except:
+    # Python 2.7
+    DEVNULL = open(os.devnull, 'w')
+
 LIBDIR_SCHEMA_DEFAULT='lib'
 severities = ['debug', 'warn', 'error', 'none']
 
@@ -157,12 +171,66 @@ def regular_print(color,text,newline=True):
     else:
         print (text)
 
-def call(cmd, silent=False):
+def shell_command(cmd, *args, **kwargs):
-    stdin, stderr = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE).communicate()
+    """ Run command through shell.
-    if not stderr:
+
-        return stdin.strip()
+    `cmd` should be a valid, properly shell-quoted command.
-    elif not silent:
+
-        color_print(1,'Problem encounted with SCons scripts, please post bug report to: https://github.com/mapnik/mapnik/issues \nError was: %s' % stderr)
+    Additional positional arguments, if provided, will each
+    be individually quoted as necessary and appended to `cmd`,
+    separated by spaces.
+
+    `logstream` optional keyword argument should be either:
+        - a file-like object, into which the command-line
+          and the command's STDERR output will be written; or
+        - None, in which case STDERR will go to DEVNULL.
+
+    Additional keyword arguments will be passed to `Popen`.
+
+    Returns a tuple `(result, output)` where:
+    `result` = True if the command completed successfully,
+               False otherwise
+    `output` = captured STDOUT with trailing whitespace removed
+    """
+    # `cmd` itself is intentionally not wrapped in `shquote` here
+    # in order to support passing user-provided commands that may
+    # include arguments. For example:
+    #
+    #   ret, out = shell_command(env['CXX'], '--version')
+    #
+    # needs to work even if `env['CXX'] == 'ccache c++'`
+    #
+    if args:
+        cmdstr = ' '.join([cmd] + [shquote(a) for a in args])
+    else:
+        cmdstr = cmd
+    # redirect STDERR to `logstream` if provided
+    try:
+        logstream = kwargs.pop('logstream')
+    except KeyError:
+        logstream = None
+    else:
+        if logstream is not None:
+            logstream.write(cmdstr + '\n')
+            kwargs['stderr'] = logstream
+        else:
+            kwargs['stderr'] = DEVNULL
+    # execute command and capture output
+    proc = Popen(cmdstr, shell=True, stdout=PIPE, **kwargs)
+    out, err = proc.communicate()
+    try:
+        outtext = out.decode(sys.stdout.encoding or 'UTF-8').rstrip()
+    except UnicodeDecodeError:
+        outtext = out.decode('UTF-8', errors='replace').rstrip()
+    if logstream is not None and outtext:
+        logstream.write('->\t' + outtext.replace('\n', '\n->\t') + '\n')
+    return proc.returncode == 0, outtext
+
+def silent_command(cmd, *args):
+    return shell_command(cmd, *args, stderr=DEVNULL)
+
+def config_command(cmd, *args):
+    return shell_command(cmd, *args, logstream=conf.logstream)
 
 def strip_first(string,find,replace=''):
     if string.startswith(find):
@@ -188,13 +256,6 @@ def create_uninstall_target(env, path, is_glob=False):
                 ])
                 env.Alias("uninstall", "uninstall-"+path)
 
-def shortest_name(libs):
-    name = '-'*200
-    for lib in libs:
-        if len(name) > len(lib):
-            name = lib
-    return name
-
 def rm_path(item,set,_env):
     for i in _env[set]:
         if i.startswith(item):
@@ -491,6 +552,12 @@ for opt in opts.options:
     if opt.key not in pickle_store:
         pickle_store.append(opt.key)
 
+def rollback_option(env, variable):
+    global opts
+    for item in opts.options:
+        if item.key == variable:
+            env[variable] = item.default
+
 # Method of adding configure behavior to Scons adapted from:
 # http://freeorion.svn.sourceforge.net/svnroot/freeorion/trunk/FreeOrion/SConstruct
 preconfigured = False
@@ -578,19 +645,22 @@ def prioritize_paths(context, silent=True):
 
 def CheckPKGConfig(context, version):
     context.Message( 'Checking for pkg-config... ' )
-    ret = context.TryAction('pkg-config --atleast-pkgconfig-version=%s' % version)[0]
+    context.sconf.cached = False
+    ret, _ = config_command('pkg-config --atleast-pkgconfig-version', version)
     context.Result( ret )
     return ret
 
 def CheckPKG(context, name):
     context.Message( 'Checking for %s... ' % name )
-    ret = context.TryAction('pkg-config --exists \'%s\'' % name)[0]
+    context.sconf.cached = False
+    ret, _ = config_command('pkg-config --exists', name)
     context.Result( ret )
     return ret
 
 def CheckPKGVersion(context, name, version):
     context.Message( 'Checking for at least version %s for %s... ' % (version,name) )
-    ret = context.TryAction('pkg-config --atleast-version=%s \'%s\'' % (version,name))[0]
+    context.sconf.cached = False
+    ret, _ = config_command('pkg-config --atleast-version', version, name)
     context.Result( ret )
     return ret
 
@@ -601,8 +671,9 @@ def parse_config(context, config, checks='--libs --cflags'):
     if config in ('GDAL_CONFIG'):
         toolname += ' %s' % checks
     context.Message( 'Checking for %s... ' % toolname)
-    cmd = '%s %s' % (env[config],checks)
+    context.sconf.cached = False
-    ret = context.TryAction(cmd)[0]
+    cmd = '%s %s' % (env[config], checks)
+    ret, value = config_command(cmd)
     parsed = False
     if ret:
         try:
@@ -613,7 +684,6 @@ def parse_config(context, config, checks='--libs --cflags'):
                 # and thus breaks knowledge below that gdal worked
                 # TODO - upgrade our scons logic to support Framework linking
                 if env['PLATFORM'] == 'Darwin':
-                    value = call(cmd,silent=True)
                     if value and '-framework GDAL' in value:
                         env['LIBS'].append('gdal')
                         if os.path.exists('/Library/Frameworks/GDAL.framework/unix/lib'):
@@ -631,7 +701,7 @@ def parse_config(context, config, checks='--libs --cflags'):
             # optional deps...
             if tool not in env['SKIPPED_DEPS']:
                 env['SKIPPED_DEPS'].append(tool)
-            conf.rollback_option(config)
+            rollback_option(env, config)
         else: # freetype and libxml2, not optional
             if tool not in env['MISSING_DEPS']:
                 env['MISSING_DEPS'].append(tool)
@@ -643,12 +713,11 @@ def get_pkg_lib(context, config, lib):
     libname = None
     env = context.env
     context.Message( 'Checking for name of %s library... ' % lib)
-    cmd = '%s --libs' % env[config]
+    context.sconf.cached = False
-    ret = context.TryAction(cmd)[0]
+    ret, value = config_command(env[config], '--libs')
     parsed = False
     if ret:
         try:
-            value = call(cmd, silent=True).decode("utf8")
             if ' ' in value:
                 parts = value.split(' ')
                 if len(parts) > 1:
@@ -671,37 +740,33 @@ def parse_pg_config(context, config):
     env = context.env
     tool = config.lower()
     context.Message( 'Checking for %s... ' % tool)
-    ret = context.TryAction(env[config])[0]
+    context.sconf.cached = False
+    ret, lib_path = config_command(env[config], '--libdir')
+    ret, inc_path = config_command(env[config], '--includedir')
     if ret:
-        lib_path = call('%s --libdir' % env[config]).decode("utf8")
-        inc_path = call('%s --includedir' % env[config]).decode("utf8")
         env.AppendUnique(CPPPATH = fix_path(inc_path))
         env.AppendUnique(LIBPATH = fix_path(lib_path))
         lpq = env['PLUGINS']['postgis']['lib']
         env.Append(LIBS = lpq)
     else:
         env['SKIPPED_DEPS'].append(tool)
-        conf.rollback_option(config)
+        rollback_option(env, config)
     context.Result( ret )
     return ret
 
 def ogr_enabled(context):
     env = context.env
     context.Message( 'Checking if gdal is ogr enabled... ')
-    ret = context.TryAction('%s --ogr-enabled' % env['GDAL_CONFIG'])[0]
+    context.sconf.cached = False
+    ret, out = config_command(env['GDAL_CONFIG'], '--ogr-enabled')
+    if ret and out:
+        ret = (out == 'yes')
     if not ret:
         if 'ogr' not in env['SKIPPED_DEPS']:
             env['SKIPPED_DEPS'].append('ogr')
     context.Result( ret )
     return ret
 
-def rollback_option(context,variable):
-    global opts
-    env = context.env
-    for item in opts.options:
-        if item.key == variable:
-            env[variable] = item.default
-
 def FindBoost(context, prefixes, thread_flag):
     """Routine to auto-find boost header dir, lib dir, and library naming structure.
 
@@ -727,7 +792,7 @@ def FindBoost(context, prefixes, thread_flag):
         if len(libItems) >= 1 and len(incItems) >= 1:
             BOOST_LIB_DIR = os.path.dirname(libItems[0])
             BOOST_INCLUDE_DIR = incItems[0].rstrip('boost/')
-            shortest_lib_name = shortest_name(libItems)
+            shortest_lib_name = min(libItems, key=len)
             match = re.search(r'%s(.*)\..*' % search_lib, shortest_lib_name)
             if hasattr(match,'groups'):
                 BOOST_APPEND = match.groups()[0]
@@ -812,7 +877,7 @@ def CheckIcuData(context, silent=False):
 
     if not silent:
         context.Message('Checking for ICU data directory...')
-    ret = context.TryRun("""
+    ret, out = context.TryRun("""
 
 #include <unicode/putil.h>
 #include <iostream>
@@ -829,14 +894,15 @@ int main() {
 """, '.cpp')
     if silent:
         context.did_show_result=1
-    if ret[0]:
-        context.Result('u_getDataDirectory returned %s' % ret[1])
-        return ret[1].strip()
-    else:
-        ret = call("icu-config --icudatadir", silent=True)
     if ret:
-            context.Result('icu-config returned %s' % ret.decode("utf8"))
+        value = out.strip()
-            return ret.decode('utf8')
+        context.Result('u_getDataDirectory returned %s' % value)
+        return value
+    else:
+        ret, value = config_command('icu-config --icudatadir')
+        if ret:
+            context.Result('icu-config returned %s' % value)
+            return value
         else:
             context.Result('Failed to detect (mapnik-config will have null value)')
             return ''
@@ -845,8 +911,8 @@ int main() {
 def CheckGdalData(context, silent=False):
 
     if not silent:
-        context.Message('Checking for GDAL data directory...')
+        context.Message('Checking for GDAL data directory... ')
-    ret = context.TryRun("""
+    ret, out = context.TryRun("""
 
 #include "cpl_config.h"
 #include <iostream>
@@ -857,19 +923,20 @@ int main() {
 }
 
 """, '.cpp')
+    value = out.strip()
     if silent:
         context.did_show_result=1
-    if ret[0]:
+    if ret:
-        context.Result('GDAL_PREFIX returned %s' % ret[1])
+        context.Result('GDAL_PREFIX returned %s' % value)
     else:
         context.Result('Failed to detect (mapnik-config will have null value)')
-    return ret[1].strip()
+    return value
 
 def CheckProjData(context, silent=False):
 
     if not silent:
         context.Message('Checking for PROJ_LIB directory...')
-    ret = context.TryRun("""
+    ret, out = context.TryRun("""
 
 // This is narly, could eventually be replaced using https://github.com/OSGeo/proj.4/pull/551]
 #include <proj_api.h>
@@ -919,20 +986,21 @@ int main() {
 }
 
 """, '.cpp')
+    value = out.strip()
     if silent:
         context.did_show_result=1
-    if ret[0]:
+    if ret:
-        context.Result('pj_open_lib returned %s' % ret[1])
+        context.Result('pj_open_lib returned %s' % value)
     else:
         context.Result('Failed to detect (mapnik-config will have null value)')
-    return ret[1].strip()
+    return value
 
 def CheckCairoHasFreetype(context, silent=False):
     if not silent:
         context.Message('Checking for cairo freetype font support ... ')
     context.env.AppendUnique(CPPPATH=copy(env['CAIRO_CPPPATHS']))
 
-    ret = context.TryRun("""
+    ret, out = context.TryRun("""
 
 #include <cairo-features.h>
 
@@ -945,7 +1013,7 @@ int main()
     #endif
 }
 
-""", '.cpp')[0]
+""", '.cpp')
     if silent:
         context.did_show_result=1
     context.Result(ret)
@@ -972,7 +1040,7 @@ int main()
     return ret
 
 def GetBoostLibVersion(context):
-    ret = context.TryRun("""
+    ret, out = context.TryRun("""
 
 #include <boost/version.hpp>
 #include <iostream>
@@ -987,8 +1055,8 @@ return 0;
 """, '.cpp')
     # hack to avoid printed output
     context.did_show_result=1
-    context.Result(ret[0])
+    context.Result(ret)
-    return ret[1].strip()
+    return out.strip()
 
 def CheckBoostScopedEnum(context, silent=False):
     if not silent:
@@ -1008,8 +1076,9 @@ int main()
     context.Result(ret)
     return ret
 
-def icu_at_least_four_two(context):
+def icu_at_least(context, min_version_str):
-    ret = context.TryRun("""
+    context.Message('Checking for ICU version >= %s... ' % min_version_str)
+    ret, out = context.TryRun("""
 
 #include <unicode/uversion.h>
 #include <iostream>
@@ -1021,28 +1090,32 @@ int main()
 }
 
 """, '.cpp')
-    # hack to avoid printed output
+    try:
-    context.Message('Checking for ICU version >= 4.2... ')
+        found_version_str = out.strip()
-    context.did_show_result=1
+        found_version = tuple(map(int, found_version_str.split('.')))
-    result = ret[1].strip()
+        min_version = tuple(map(int, min_version_str.split('.')))
-    if not result:
+    except:
-        context.Result('error, could not get major and minor version from unicode/uversion.h')
+        context.Result('error (could not get version from unicode/uversion.h)')
         return False
 
-    major, minor = map(int,result.split('.'))
+    if found_version >= min_version:
-    if major >= 4 and minor >= 0:
+        context.Result('yes (found ICU %s)' % found_version_str)
-        color_print(4,'found: icu %s' % result)
         return True
 
-    color_print(1,'\nFound insufficient icu version... %s' % result)
+    context.Result('no (found ICU %s)' % found_version_str)
     return False
 
 def harfbuzz_version(context):
-    ret = context.TryRun("""
+    context.Message('Checking for HarfBuzz version >= %s... ' % HARFBUZZ_MIN_VERSION_STRING)
+    ret, out = context.TryRun("""
 
 #include "harfbuzz/hb.h"
 #include <iostream>
 
+#ifndef HB_VERSION_ATLEAST
+#define HB_VERSION_ATLEAST(...) 0
+#endif
+
 int main()
 {
     std::cout << HB_VERSION_ATLEAST(%s, %s, %s) << ";" << HB_VERSION_STRING;
@@ -1050,24 +1123,20 @@ int main()
 }
 
 """ % HARFBUZZ_MIN_VERSION, '.cpp')
-    # hack to avoid printed output
+    if not ret:
-    context.Message('Checking for HarfBuzz version >= %s... ' % HARFBUZZ_MIN_VERSION_STRING)
+        context.Result('error (could not get version from hb.h)')
-    context.did_show_result=1
+    else:
-    result = ret[1].strip()
+        ok_str, found_version_str = out.strip().split(';', 1)
-    if not result:
+        ret = int(ok_str)
-        context.Result('error, could not get version from hb.h')
+        if ret:
-        return False
+            context.Result('yes (found HarfBuzz %s)' % found_version_str)
-
+        else:
-    items = result.split(';')
+            context.Result('no (found HarfBuzz %s)' % found_version_str)
-    if items[0] == '1':
+    return ret
-        color_print(4,'found: HarfBuzz %s' % items[1])
-        return True
-
-    color_print(1,'\nHarfbuzz >= %s required but found ... %s' % (HARFBUZZ_MIN_VERSION_STRING,items[1]))
-    return False
 
 def harfbuzz_with_freetype_support(context):
-    ret = context.TryRun("""
+    context.Message('Checking for HarfBuzz with freetype support... ')
+    ret, out = context.TryRun("""
 
 #include "harfbuzz/hb-ft.h"
 #include <iostream>
@@ -1078,11 +1147,8 @@ int main()
 }
 
 """, '.cpp')
-    context.Message('Checking for HarfBuzz with freetype support\n')
+    context.Result(ret)
-    context.Result(ret[0])
+    return ret
-    if ret[0]:
-        return True
-    return False
 
 def boost_regex_has_icu(context):
     if env['RUNTIME_LINK'] == 'static':
@@ -1091,7 +1157,8 @@ def boost_regex_has_icu(context):
             if lib_name in context.env['LIBS']:
                 context.env['LIBS'].remove(lib_name)
             context.env.Append(LIBS=lib_name)
-    ret = context.TryRun("""
+    context.Message('Checking if boost_regex was built with ICU unicode support... ')
+    ret, out = context.TryRun("""
 
 #include <boost/regex/icu.hpp>
 #include <unicode/unistr.h>
@@ -1111,11 +1178,8 @@ int main()
 }
 
 """, '.cpp')
-    context.Message('Checking if boost_regex was built with ICU unicode support... ')
+    context.Result(ret)
-    context.Result(ret[0])
+    return ret
-    if ret[0]:
-        return True
-    return False
 
 def sqlite_has_rtree(context, silent=False):
     """ check an sqlite3 install has rtree support.
@@ -1124,7 +1188,9 @@ def sqlite_has_rtree(context, silent=False):
     http://www.sqlite.org/c3ref/compileoption_get.html
     """
 
-    ret = context.TryRun("""
+    if not silent:
+        context.Message('Checking if SQLite supports RTREE... ')
+    ret, out = context.TryRun("""
 
 #include <sqlite3.h>
 #include <stdio.h>
@@ -1156,17 +1222,15 @@ int main()
 }
 
 """, '.c')
-    if not silent:
-        context.Message('Checking if SQLite supports RTREE... ')
     if silent:
         context.did_show_result=1
-    context.Result(ret[0])
+    context.Result(ret)
-    if ret[0]:
+    return ret
-        return True
-    return False
 
 def supports_cxx14(context,silent=False):
-    ret = context.TryRun("""
+    if not silent:
+        context.Message('Checking if compiler (%s) supports -std=c++14 flag... ' % context.env.get('CXX','CXX'))
+    ret, out = context.TryRun("""
 
 int main()
 {
@@ -1178,14 +1242,10 @@ int main()
 }
 
 """, '.cpp')
-    if not silent:
-        context.Message('Checking if compiler (%s) supports -std=c++14 flag... ' % context.env.get('CXX','CXX'))
     if silent:
         context.did_show_result=1
-    context.Result(ret[0])
+    context.Result(ret)
-    if ret[0]:
+    return ret
-        return True
-    return False
 
 
 
@@ -1205,8 +1265,7 @@ conf_tests = { 'prioritize_paths'      : prioritize_paths,
                'parse_pg_config'       : parse_pg_config,
                'ogr_enabled'           : ogr_enabled,
                'get_pkg_lib'           : get_pkg_lib,
-               'rollback_option'       : rollback_option,
+               'icu_at_least'          : icu_at_least,
-               'icu_at_least_four_two' : icu_at_least_four_two,
                'harfbuzz_version'      : harfbuzz_version,
                'harfbuzz_with_freetype_support': harfbuzz_with_freetype_support,
                'boost_regex_has_icu'   : boost_regex_has_icu,
@@ -1274,11 +1333,11 @@ if not preconfigured:
     env['PLATFORM'] = platform.uname()[0]
     color_print(4,"Configuring on %s in *%s*..." % (env['PLATFORM'],mode))
 
-    cxx_version = call("%s --version" % env["CXX"] ,silent=True)
+    ret, cxx_version = config_command(env['CXX'], '--version')
-    if cxx_version:
+    if ret:
-        color_print(5, "CXX %s" % cxx_version.decode("utf8"))
+        color_print(5, "C++ compiler: %s" % cxx_version)
     else:
-        color_print(5, "Could not detect CXX compiler")
+        color_print(5, "Could not detect C++ compiler")
 
     env['MISSING_DEPS'] = []
     env['SKIPPED_DEPS'] = []
@@ -1528,7 +1587,7 @@ if not preconfigured:
             else:
                 if libname == env['ICU_LIB_NAME']:
                     if env['ICU_LIB_NAME'] not in env['MISSING_DEPS']:
-                        if not conf.icu_at_least_four_two():
+                        if not conf.icu_at_least("4.0"):
                             # expression_string.cpp and map.cpp use fromUTF* function only available in >= ICU 4.2
                             env['MISSING_DEPS'].append(env['ICU_LIB_NAME'])
                 elif libname == 'harfbuzz':
@@ -1597,8 +1656,8 @@ if not preconfigured:
         # around. See https://svn.boost.org/trac/boost/ticket/6779 for more
         # details.
         if not env['HOST']:
-            boost_version = [int(x) for x in env.get('BOOST_LIB_VERSION_FROM_HEADER').split('_')]
             if not conf.CheckBoostScopedEnum():
+                boost_version = [int(x) for x in env.get('BOOST_LIB_VERSION_FROM_HEADER').split('_') if x]
                 if boost_version < [1, 51]:
                     env.Append(CXXFLAGS = '-DBOOST_NO_SCOPED_ENUMS')
                 elif boost_version < [1, 57]:

benchmark/build.py

@@ -18,40 +18,19 @@ test_env.Append(CPPDEFINES = env['LIBMAPNIK_DEFINES'])
 if test_env['HAS_CAIRO']:
     test_env.PrependUnique(CPPPATH=test_env['CAIRO_CPPPATHS'])
     test_env.Append(CPPDEFINES = '-DHAVE_CAIRO')
+test_env.PrependUnique(CPPPATH='include', delete_existing=True)
 test_env['LINKFLAGS'] = copy(test_env['LIBMAPNIK_LINKFLAGS'])
 if env['PLATFORM'] == 'Darwin':
     test_env.Append(LINKFLAGS='-F/ -framework CoreFoundation')
 
 test_env_local = test_env.Clone()
 
-#benchmarks = glob.glob('test*cpp')
+benchmarks = glob.glob("src/*.cpp")
-benchmarks = [
+
-    #"test_array_allocation.cpp",
+for src in benchmarks:
-    #"test_png_encoding1.cpp",
+    name, ext = os.path.splitext(os.path.basename(src))
-    #"test_png_encoding2.cpp",
+    out = os.path.join("out", name)
-    #"test_to_string1.cpp",
+    test_program = test_env_local.Program(out, source=[src])
-    #"test_to_string2.cpp",
-    #"test_to_bool.cpp",
-    #"test_to_double.cpp",
-    #"test_to_int.cpp",
-    #"test_utf_encoding.cpp"
-    "test_polygon_clipping.cpp",
-    #"test_polygon_clipping_rendering.cpp",
-    "test_proj_transform1.cpp",
-    "test_expression_parse.cpp",
-    "test_face_ptr_creation.cpp",
-    "test_font_registration.cpp",
-    "test_rendering.cpp",
-    "test_rendering_shared_map.cpp",
-    "test_offset_converter.cpp",
-    "test_marker_cache.cpp",
-    "test_quad_tree.cpp",
-    "test_noop_rendering.cpp",
-    "test_getline.cpp",
-#    "test_numeric_cast_vs_static_cast.cpp",
-]
-for cpp_test in benchmarks:
-    test_program = test_env_local.Program('out/'+cpp_test.replace('.cpp',''), source=[cpp_test])
     if 'install' in COMMAND_LINE_TARGETS:
         env.Alias('install',test_program)
     #Depends(test_program, env.subst('../src/%s' % env['MAPNIK_LIB_NAME']))

benchmark/include/bench_framework.hpp

@@ -12,6 +12,7 @@
 #include <chrono>
 #include <cmath> // log10, round
 #include <cstdio> // snprintf
+#include <iomanip>
 #include <iostream>
 #include <set>
 #include <sstream>
@@ -239,14 +240,19 @@ int run(T const& test_runner, std::string const& name)
         big_number_fmt itersf(4, total_iters);
         big_number_fmt ips(5, total_iters / seconds<double>(elapsed_nonzero).count());
 
+        std::clog << std::left << std::setw(43) << name;
+        std::clog << std::resetiosflags(std::ios::adjustfield);
+        if (num_threads > 0) {
+            std::clog << ' ' << std::setw(3) << num_threads
+                << " worker" << (num_threads > 1 ? "s" : " ");
+        }
+        else {
+            std::clog << " main thread";
+        }
         std::snprintf(msg, sizeof(msg),
-                "%-43s %3zu thread(s) %*.0f%s iters %6.0f milliseconds %*.0f%s i/s\n",
+                " %*.0f%s iters %6.0f milliseconds %*.0f%s i/t/s\n",
-                name.c_str(),
+                itersf.w, itersf.v, itersf.u, dur_total,
-                num_threads,
+                ips.w, ips.v, ips.u);
-                itersf.w, itersf.v, itersf.u,
-                dur_total,
-                ips.w, ips.v, ips.u
-                );
         std::clog << msg;
         return 0;
     }

benchmark/run

@@ -10,7 +10,7 @@ function run {
     local threads="$2"
     local iters="$3"
     shift 3
-    $runner --threads 1 --iterations $iters "$@"
+    $runner --threads 0 --iterations $iters "$@"
     if test $threads -gt 0; then
         $runner --threads $threads --iterations $((iters/threads)) "$@"
     fi
@@ -28,6 +28,7 @@ run test_expression_parse 10 10000
 run test_face_ptr_creation 10 1000
 run test_font_registration 10 100
 run test_offset_converter 10 1000
+#run normalize_angle 0 1000000 --min-duration=0.2
 
 # commented since this is really slow on travis
 : '

benchmark/src/normalize_angle.cpp

@@ -0,0 +1,69 @@
+#include "bench_framework.hpp"
+
+#include <mapnik/util/math.hpp>
+
+template <typename T>
+struct bench_func : benchmark::test_case
+{
+    T (* const func_)(T);
+    T const value_;
+
+    bench_func(mapnik::parameters const& params, T (*func)(T), T value)
+        : test_case(params), func_(func), value_(value) {}
+
+    bool validate() const { return true; }
+
+    bool operator() () const
+    {
+        for (auto i = this->iterations_; i-- > 0; )
+        {
+            func_(value_);
+        }
+        return true;
+    }
+};
+
+#define BENCH_FUNC1(func, value) \
+        run<bench_func<double>>(#func "(" #value ")", func, value)
+
+int main(int argc, char** argv)
+{
+    return benchmark::sequencer(argc, argv)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +3)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +6)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +9)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +12)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +15)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +20)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +30)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +40)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +50)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +70)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +90)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +110)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +130)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +157)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +209)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +314)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +628)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, +942)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -3)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -6)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -9)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -12)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -15)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -20)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -30)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -40)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -50)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -70)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -90)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -110)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -130)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -157)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -209)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -314)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -628)
+        .BENCH_FUNC1(mapnik::util::normalize_angle, -942)
+        .done();
+}

deps/mapbox/geometry

@@ -1 +1 @@
-Subproject commit b0e41cc5635ff8d50e7e1edb73cadf1d2a7ddc83
+Subproject commit cfcb983f3571269653467f0a679bd956366c101e

deps/mapbox/protozero

@@ -1 +1 @@
-Subproject commit f5154595f8488dd3016a17d434202e46f2feef25
+Subproject commit a44efc34e5a86e93c06390aa19c89f8e115971f6

deps/mapbox/variant

@@ -1 +1 @@
-Subproject commit 859a8c933a0c2ab18941acb9dcf834799c0de46c
+Subproject commit 256ddd55582bb7c06c342315dbacc6a42fee4b34

include/mapnik/expression_grammar_x3_def.hpp

@@ -315,14 +315,14 @@ namespace mapnik { namespace grammar {
     auto const single_quoted_string = x3::rule<class single_quoted_string, std::string> {} = lit('\'')
         >> no_skip[*(unesc_char[append]
                      |
-                     //(lit('\\') > escaped_unicode[append]) // FIXME (!)
+                     (lit('\\') >> escaped_unicode[append])
-                     //|
+                     |
                      (~char_('\''))[append])] > lit('\'');
 
     auto const double_quoted_string = x3::rule<class double_quoted_string, std::string> {} = lit('"')
         >> no_skip[*(unesc_char[append]
                      |
-                     (lit('\\') > escaped_unicode[append])
+                     (lit('\\') >> escaped_unicode[append])
                      |
                      (~char_('"'))[append])] > lit('"');
 

include/mapnik/image_view_impl.hpp

@@ -37,7 +37,7 @@ image_view<T>::image_view(std::size_t x, std::size_t y, std::size_t width, std::
       data_(data)
 {
     if (x_ >= data_.width() && data_.width() > 0) x_ = data_.width() - 1;
-    if (y_ >= data_.height() && data.height() > 0) y_ = data_.height() - 1;
+    if (y_ >= data_.height() && data_.height() > 0) y_ = data_.height() - 1;
     if (x_ + width_ > data_.width()) width_ = data_.width() - x_;
     if (y_ + height_ > data_.height()) height_ = data_.height() - y_;
 }

include/mapnik/offset_converter.hpp

@@ -40,6 +40,8 @@
 namespace mapnik
 {
 
+static constexpr double offset_converter_default_threshold = 5.0;
+
 template <typename Geometry>
 struct offset_converter
 {
@@ -48,7 +50,7 @@ struct offset_converter
     offset_converter(Geometry & geom)
         : geom_(geom)
         , offset_(0.0)
-        , threshold_(5.0)
+        , threshold_(offset_converter_default_threshold)
         , half_turn_segments_(16)
         , status_(initial)
         , pre_first_(vertex2d::no_init)

include/mapnik/util/math.hpp

@@ -27,9 +27,11 @@
 
 namespace mapnik { namespace util {
 
+constexpr double pi = 3.1415926535897932384626433832795;
+constexpr double tau = 6.283185307179586476925286766559;
+
 MAPNIK_DECL double normalize_angle(double angle);
 
 }}
 
 #endif
-

include/mapnik/version.hpp

@@ -29,10 +29,18 @@
 #define MAPNIK_MINOR_VERSION 1
 #define MAPNIK_PATCH_VERSION 0
 
-#define MAPNIK_VERSION (MAPNIK_MAJOR_VERSION*100000) + (MAPNIK_MINOR_VERSION*100) + (MAPNIK_PATCH_VERSION)
+#define MAPNIK_VERSION          MAPNIK_MAKE_VERSION(MAPNIK_MAJOR_VERSION, \
+                                                    MAPNIK_MINOR_VERSION, \
+                                                    MAPNIK_PATCH_VERSION)
 
 #define MAPNIK_VERSION_STRING   MAPNIK_STRINGIFY(MAPNIK_MAJOR_VERSION) "." \
                                 MAPNIK_STRINGIFY(MAPNIK_MINOR_VERSION) "." \
                                 MAPNIK_STRINGIFY(MAPNIK_PATCH_VERSION)
 
+#define MAPNIK_VERSION_AT_LEAST(major, minor, patch) \
+        (MAPNIK_VERSION >= MAPNIK_MAKE_VERSION(major, minor, patch))
+
+#define MAPNIK_MAKE_VERSION(major, minor, patch) \
+        ((major) * 100000 + (minor) * 100 + (patch))
+
 #endif // MAPNIK_VERSION_HPP

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);
@@ -206,77 +233,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_width = static_cast<int>(raster_width_);
-    int current_height = (int)raster_height_;
+    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);
 
-    // 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));
     if (band_ > 0 && band_ < nbands_)
     {
-        GDALRasterBand * band = dataset_.GetRasterBand(band_);
+        find_best_overview(band_,
-        int band_overviews = band->GetOverviewCount();
+                           ideal_raster_width,
-        if (band_overviews > 0)
+                           ideal_raster_height,
-        {
+                           current_width,
-            for (int b = 0; b < band_overviews; b++)
+                           current_height);
-            {
-                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;
-                }
-            }
-        }
     }
     else
     {
         for (int i = 0; i < nbands_; ++i)
         {
-            GDALRasterBand * band = dataset_.GetRasterBand(i + 1);
+            find_best_overview(i + 1,
-            int band_overviews = band->GetOverviewCount();
+                               ideal_raster_width,
-            if (band_overviews > 0)
+                               ideal_raster_height,
-            {
+                               current_width,
-                for (int b = 0; b < band_overviews; b++)
+                               current_height);
-                {
-                    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;
         }
     }
-            }
+
-        }
+    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));
+            im_offset_x = std::floor(ratio * im_offset_x);
-            double adjusted_ratio = (double)adjusted_width / (double)im_width;
+            im_width = static_cast<int>(std::ceil(ratio * im_width));
-            im_offset_x = adjusted_ratio * im_offset_x;
-            im_width = adjusted_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));
+            im_offset_y = std::floor(ratio * im_offset_y);
-            double adjusted_ratio = (double)adjusted_height / (double)im_height;
+            im_height = static_cast<int>(std::ceil(ratio * im_height));
-            im_offset_y = adjusted_ratio * im_offset_y;
-            im_height = adjusted_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_;

plugins/input/pgraster/pgraster_datasource.cpp

@@ -172,7 +172,7 @@ pgraster_datasource::pgraster_datasource(parameters const& params)
                 (raster_table_, parsed_schema_, parsed_table_);
         }
 
-        // If we do not know either the geometry_field or the srid or we
+        // If we do not know either the raster_field or the srid or we
         // want to use overviews but do not know about schema, or
         // no extent was specified, then attempt to fetch the missing
         // information from a raster_columns entry.
@@ -180,7 +180,7 @@ pgraster_datasource::pgraster_datasource(parameters const& params)
         // This will return no records if we are querying a bogus table returned
         // from the simplistic table parsing in table_from_sql() or if
         // the table parameter references a table, view, or subselect not
-        // registered in the geometry columns.
+        // registered in the raster_columns.
         //
         geometryColumn_ = mapnik::sql_utils::unquote_copy('"', raster_field_);
         if (!parsed_table_.empty() && (
@@ -265,7 +265,7 @@ pgraster_datasource::pgraster_datasource(parameters const& params)
 
             // If we still do not know the srid then we can try to fetch
             // it from the 'table_' parameter, which should work even if it is
-            // a subselect as long as we know the geometry_field to query
+            // a subselect as long as we know the raster_field to query
             if (! geometryColumn_.empty() && srid_ <= 0)
             {
                 s.str("");
@@ -443,7 +443,7 @@ pgraster_datasource::pgraster_datasource(parameters const& params)
             MAPNIK_LOG_DEBUG(pgraster) << "pgraster_datasource: Table " << table_ << " is using SRID=" << srid_;
         }
 
-        // At this point the geometry_field may still not be known
+        // At this point the raster_field may still not be known
         // but we'll catch that where more useful...
         MAPNIK_LOG_DEBUG(pgraster) << "pgraster_datasource: Using SRID=" << srid_;
         MAPNIK_LOG_DEBUG(pgraster) << "pgraster_datasource: Using geometry_column=" << geometryColumn_;
@@ -835,7 +835,7 @@ featureset_ptr pgraster_datasource::features_with_context(query const& q,process
                 s_error << parsed_schema_ << ".";
             }
             s_error << parsed_table_
-                    << "'. Please manually provide the 'geometry_field' parameter or add an entry "
+                    << "'. Please manually provide the 'raster_field' parameter or add an entry "
                     << "in the geometry_columns for '";
 
             if (!parsed_schema_.empty())
@@ -998,7 +998,7 @@ featureset_ptr pgraster_datasource::features_at_point(coord2d const& pt, double
                     s_error << parsed_schema_ << ".";
                 }
                 s_error << parsed_table_
-                        << "'. Please manually provide the 'geometry_field' parameter or add an entry "
+                        << "'. Please manually provide the 'raster_field' parameter or add an entry "
                         << "in the geometry_columns for '";
 
                 if (!parsed_schema_.empty())

scripts/markdown-hyperlinks.pl

@@ -0,0 +1,56 @@
+#! /usr/bin/env perl
+#
+#   Re-generate hyperlinks in place:
+#
+#       perl -i scripts/markdown-hyperlinks.pl CHANGELOG.md
+#
+#   Generate to another file:
+#
+#       scripts/markdown-hyperlinks.pl CHANGELOG.md > CHANGELOG-autolink.md
+
+use strict;
+use warnings;
+
+my $user = qr/ [a-zA-Z] [a-zA-Z0-9]* /x;
+my $repo = qr/ [a-zA-Z] [a-zA-Z0-9.-]* /x;
+
+while (<>) {
+
+    # make links from @username references
+    # (except when escaped like \@foobar or in code like `where !@barman! = 'Moe'`)
+    s"(?: (`++) .*? \g{-1} (*SKIP) (*FAIL) )?   # skip over code spans
+      (?<! \[ | \\ )                            # no match after [ or \
+      \@ ($user) \b
+      (?! \] )                                  # no match before ]
+     "[\@$2](https://github.com/$2)"xg;
+
+    # make links from #1234 references (except when escaped like \#5)
+    # we can't tell whether the number refers to an issue or a pull request,
+    # luckily link to issues/1234 works in either case
+    s;(?<! \[ | \\ ) \# ([0-9]+) \b (?! \] )
+     ;[\#$1](https://github.com/mapnik/mapnik/issues/$1);xg;
+
+    # make shortcut links from raw URIs (which GFM turns into proper links,
+    # but doesn't contract even though it could)
+    # - issues
+    s;(?<! \] \( ) (https://github\.com/mapnik/mapnik/(?:issues|pull)/([0-9]+))
+     ;[\#$2]($1);xg;
+    s;(?<! \] \( ) (https://github\.com/($user/$repo)/(?:issues|pull)/([0-9]+))
+     ;[$2\#$3]($1);xg;
+    # - commit hashes
+    s;(?<! \] \( ) (https://github\.com/mapnik/mapnik/commit/([0-9a-f]{7})[0-9a-f]{0,33}) \b
+     ;[$2]($1);xg;
+    s;(?<! \] \( ) (https://github\.com/($user/$repo)/commit/([0-9a-f]{7})[0-9a-f]{0,33}) \b
+     ;[$2\@$3]($1);xg;
+
+    # make links from commit hashes
+    # (accept 7 or 9-40 hex digits, but not 8 which could be a date)
+    s"(?: (`++) .*? \g{-1} (*SKIP) (*FAIL) )?   # skip over code spans
+      (?: \[.*?\] \(.*?\)  (*SKIP) (*FAIL) )?   # skip over links
+      (?<! / )                                  # no match after /
+      \b (([0-9a-f]{7})                         # 7 digits for link text
+          ([0-9a-f]{2,33})?) \b                 # maybe 2-33 more digits
+     "[$3](https://github.com/mapnik/mapnik/commit/$2)"xg;
+
+    print;
+}

src/agg/process_line_pattern_symbolizer.cpp

@@ -132,12 +132,11 @@ private:
         box2d<double> clip_box = clipping_extent(common_);
         if (clip)
         {
-            double padding = (double)(common_.query_extent_.width() / common_.width_);
+            double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-            if (half_stroke > 1)
+            double pixels = std::ceil(std::max(width / 2.0 + std::fabs(offset),
-                padding *= half_stroke;
+                                              (std::fabs(offset) * offset_converter_default_threshold)));
-            if (std::fabs(offset) > 0)
+            double padding = pad_per_pixel * pixels * common_.scale_factor_;
-                padding *= std::fabs(offset) * 1.2;
+
-            padding *= common_.scale_factor_;
             clip_box.pad(padding);
         }
         using vertex_converter_type = vertex_converter<clip_line_tag, transform_tag,

src/agg/process_line_symbolizer.cpp

@@ -140,23 +140,12 @@ void agg_renderer<T0,T1>::process(line_symbolizer const& sym,
     line_rasterizer_enum rasterizer_e = get<line_rasterizer_enum, keys::line_rasterizer>(sym, feature, common_.vars_);
     if (clip)
     {
-        double padding = static_cast<double>(common_.query_extent_.width() / common_.width_);
+        double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-        double half_stroke = 0.5 * width;
+        double pixels = std::ceil(std::max(width / 2.0 + std::fabs(offset),
-        if (half_stroke > 1)
+                                          (std::fabs(offset) * offset_converter_default_threshold)));
-        {
+        double padding = pad_per_pixel * pixels * common_.scale_factor_;
-            padding *= half_stroke;
-        }
-        if (std::fabs(offset) > 0)
-        {
-            padding *= std::fabs(offset) * 1.2;
-        }
 
-        padding *= common_.scale_factor_;
         clip_box.pad(padding);
-        // debugging
-        //box2d<double> inverse = query_extent_;
-        //inverse.pad(-padding);
-        //draw_geo_extent(inverse,mapnik::color("red"));
     }
 
     if (rasterizer_e == RASTERIZER_FAST)

src/cairo/process_line_pattern_symbolizer.cpp

@@ -133,13 +133,11 @@ void cairo_renderer<T>::process(line_pattern_symbolizer const& sym,
     box2d<double> clipping_extent = common_.query_extent_;
     if (clip)
     {
-        double padding = (double)(common_.query_extent_.width()/common_.width_);
+        double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-        double half_stroke = width/2.0;
+        double pixels = std::ceil(std::max(width / 2.0 + std::fabs(offset),
-        if (half_stroke > 1)
+                                          (std::fabs(offset) * offset_converter_default_threshold)));
-            padding *= half_stroke;
+        double padding = pad_per_pixel * pixels * common_.scale_factor_;
-        if (std::fabs(offset) > 0)
+
-            padding *= std::fabs(offset) * 1.2;
-        padding *= common_.scale_factor_;
         clipping_extent.pad(padding);
     }
 

src/cairo/process_line_symbolizer.cpp

@@ -73,13 +73,11 @@ void cairo_renderer<T>::process(line_symbolizer const& sym,
     box2d<double> clipping_extent = common_.query_extent_;
     if (clip)
     {
-        double padding = (double)(common_.query_extent_.width()/common_.width_);
+        double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-        double half_stroke = width/2.0;
+        double pixels = std::ceil(std::max(width / 2.0 + std::fabs(offset),
-        if (half_stroke > 1)
+                                          (std::fabs(offset) * offset_converter_default_threshold)));
-            padding *= half_stroke;
+        double padding = pad_per_pixel * pixels * common_.scale_factor_;
-        if (std::fabs(offset) > 0)
+
-            padding *= std::fabs(offset) * 1.2;
-        padding *= common_.scale_factor_;
         clipping_extent.pad(padding);
     }
     using vertex_converter_type =  vertex_converter<clip_line_tag,

src/grid/process_line_pattern_symbolizer.cpp

@@ -97,13 +97,11 @@ void grid_renderer<T>::process(line_pattern_symbolizer const& sym,
     box2d<double> clipping_extent = common_.query_extent_;
     if (clip)
     {
-        double padding = (double)(common_.query_extent_.width()/pixmap_.width());
+        double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-        double half_stroke = stroke_width/2.0;
+        double pixels = std::ceil(std::max(stroke_width / 2.0 + std::fabs(offset),
-        if (half_stroke > 1)
+                                          (std::fabs(offset) * offset_converter_default_threshold)));
-            padding *= half_stroke;
+        double padding = pad_per_pixel * pixels * common_.scale_factor_;
-        if (std::fabs(offset) > 0)
+
-            padding *= std::fabs(offset) * 1.2;
-        padding *= common_.scale_factor_;
         clipping_extent.pad(padding);
     }
 

src/grid/process_line_symbolizer.cpp

@@ -84,13 +84,11 @@ void grid_renderer<T>::process(line_symbolizer const& sym,
 
     if (clip)
     {
-        double padding = (double)(common_.query_extent_.width()/pixmap_.width());
+        double pad_per_pixel = static_cast<double>(common_.query_extent_.width()/common_.width_);
-        double half_stroke = width/2.0;
+        double pixels = std::ceil(std::max(width / 2.0 + std::fabs(offset),
-        if (half_stroke > 1)
+                                          (std::fabs(offset) * offset_converter_default_threshold)));
-            padding *= half_stroke;
+        double padding = pad_per_pixel * pixels * common_.scale_factor_;
-        if (std::fabs(offset) > 0)
+
-            padding *= std::fabs(offset) * 1.2;
-        padding *= common_.scale_factor_;
         clipping_extent.pad(padding);
     }
     using vertex_converter_type = vertex_converter<clip_line_tag, clip_poly_tag, transform_tag,

src/load_map.cpp

@@ -380,14 +380,11 @@ void map_parser::parse_map(Map & map, xml_node const& node, std::string const& b
                 }
                 if (success)
                 {
-                    int min_version = (n[0] * 100000) + (n[1] * 100) + (n[2]);
+                    if (!MAPNIK_VERSION_AT_LEAST(n[0], n[1], n[2]))
-                    if (min_version > MAPNIK_VERSION)
                     {
                         throw config_error(std::string("This map uses features only present in Mapnik version ") + *min_version_string + " and newer");
                     }
-
                 }
-
             }
         }
         catch (config_error const& ex)
@@ -545,12 +542,26 @@ void map_parser::parse_style(Map & map, xml_node const& node)
 
         if (!map.insert_style(name, std::move(style)))
         {
-            if (map.find_style(name))
+            boost::optional<const feature_type_style &> dupe = map.find_style(name);
+            if (strict_)
+            {
+                if (dupe)
                 {
                     throw config_error("duplicate style name");
                 }
                 throw config_error("failed to insert style to the map");
             }
+            else
+            {
+                std::string s_err("failed to insert style '");
+                s_err += name + "' to the map";
+                if (dupe)
+                {
+                    s_err += " since it was already added";
+                }
+                MAPNIK_LOG_ERROR(load_map) << "map_parser: " << s_err;
+            }
+        }
     }
     catch (config_error const& ex)
     {

src/math.cpp

@@ -22,7 +22,6 @@
 
 // mapnik
 #include <mapnik/util/math.hpp>
-#include <mapnik/global.hpp>
 
 // stl
 #include <cmath>
@@ -33,13 +32,26 @@ namespace util {
 
     double normalize_angle(double angle)
     {
-        while (angle >= M_PI)
+        if (angle > pi)
         {
-            angle -= 2.0 * M_PI;
+            if (angle > 16 * tau)
+            {
+                // the angle is too large; better compute the remainder
+                // directly to avoid subtracting circles ad infinitum
+                return std::remainder(angle, tau);
             }
-        while (angle < -M_PI)
+            // std::remainder would take longer than a few subtractions
+            while ((angle -= tau) > pi)
+                ;
+        }
+        else if (angle < -pi)
         {
-            angle += 2.0 * M_PI;
+            if (angle < -16 * tau)
+            {
+                return std::remainder(angle, tau);
+            }
+            while ((angle += tau) < -pi)
+                ;
         }
         return angle;
     }

src/proj_transform.cpp

@@ -21,13 +21,17 @@
  *****************************************************************************/
 
 // mapnik
-#include <mapnik/global.hpp>
+#include <mapnik/geometry/boost_adapters.hpp>
 #include <mapnik/geometry/box2d.hpp>
+#include <mapnik/geometry/multi_point.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;
+    auto coords = envelope_points(env, points);  // this is always clockwise
-    envelope_points(coords, env, points);  // this is always clockwise
 
-    double z;
+    for (auto & p : coords)
-    for (std::vector<coord<double,2> >::iterator it = coords.begin(); it!=coords.end(); ++it) {
+    {
-        z = 0;
+        double z = 0;
-        if (!backward(it->x, it->y, z)) {
+        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;
+    auto coords = envelope_points(env, points);  // this is always clockwise
-    envelope_points(coords, env, points);  // this is always clockwise
 
-    double z;
+    for (auto & p : coords)
-    for (std::vector<coord<double,2> >::iterator it = coords.begin(); it!=coords.end(); ++it) {
+    {
-        z = 0;
+        double z = 0;
-        if (!forward(it->x, it->y, z)) {
+        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))
     {

test/data-visual

@@ -1 +1 @@
-Subproject commit c113ce13267124332cc2ecd049d7d2d7397f9a51
+Subproject commit 23034ae27fb0b00d202688865268a80d05065fcc

test/standalone/map_xml_test.cpp

@@ -156,6 +156,17 @@ TEST_CASE("map xml I/O") {
         }
     } // END SECTION
 
+    SECTION("duplicate styles only throw in strict mode") {
+        std::string duplicate_stylename("test/data/broken_maps/duplicate_stylename.xml");
+        CAPTURE(duplicate_stylename);
+        mapnik::Map m(256, 256);
+        REQUIRE(m.register_fonts("fonts", true));
+        REQUIRE_NOTHROW(mapnik::load_map(m, duplicate_stylename, false));
+        mapnik::Map m2(256, 256);
+        REQUIRE(m2.register_fonts("fonts", true));
+        REQUIRE_THROWS(mapnik::load_map(m2, duplicate_stylename, true));
+    } // END SECTION
+
     SECTION("broken maps") {
         std::vector<bfs::path> broken_maps;
         add_xml_files("test/data/broken_maps", broken_maps);

test/unit/core/expressions_test.cpp

@@ -9,7 +9,7 @@
 #include <mapnik/unicode.hpp>
 
 #include <functional>
-#include <vector>
+#include <map>
 
 namespace {
 
@@ -57,7 +57,7 @@ std::string parse_and_dump(std::string const& str)
 TEST_CASE("expressions")
 {
     using namespace std::placeholders;
-    using properties_type = std::vector<std::pair<std::string, mapnik::value> > ;
+    using properties_type = std::map<std::string, mapnik::value>;
     mapnik::transcoder tr("utf8");
 
     properties_type prop = {{ "foo"   , tr.transcode("bar") },
@@ -65,6 +65,7 @@ TEST_CASE("expressions")
                             { "grass" , tr.transcode("grow")},
                             { "wind"  , tr.transcode("blow")},
                             { "sky"   , tr.transcode("is blue")},
+                            { "τ"     , mapnik::value_double(6.2831853)},
                             { "double", mapnik::value_double(1.23456)},
                             { "int"   , mapnik::value_integer(123)},
                             { "bool"  , mapnik::value_bool(true)},
@@ -74,8 +75,6 @@ TEST_CASE("expressions")
     auto eval = std::bind(evaluate_string, feature, _1);
     auto approx = Approx::custom().epsilon(1e-6);
 
-    TRY_CHECK(eval(" [foo]='bar' ") == true);
-
     // primary expressions
     // null
     TRY_CHECK(parse_and_dump("null") == "null");
@@ -98,6 +97,17 @@ TEST_CASE("expressions")
     TRY_CHECK(parse_and_dump("deg_to_rad") == "0.0174533");
     TRY_CHECK(parse_and_dump("rad_to_deg") == "57.2958");
 
+    // ascii attribute name
+    TRY_CHECK(eval(" [foo]='bar' ") == true);
+
+    // unicode attribute name
+    TRY_CHECK(eval("[τ]") == prop.at("τ"));
+    TRY_CHECK(eval("[τ]") == eval(u8"[\u03C4]"));
+
+    // change to TRY_CHECK once \u1234 escape sequence in attribute name
+    // is implemented in expression grammar
+    CHECK_NOFAIL(eval("[τ]") == eval("[\\u03C3]"));
+
     // unary functions
     // sin / cos
     TRY_CHECK(eval(" sin(0.25 * pi) / cos(0.25 * pi) ").to_double() == approx(1.0));
@@ -174,7 +184,8 @@ TEST_CASE("expressions")
 
     // regex
     // replace
-    TRY_CHECK(eval(" [foo].replace('(\\B)|( )','$1 ') ") == tr.transcode("b a r"));
+    TRY_CHECK(eval(" [foo].replace('(\\B)|( )','$1 ') ") == tr.transcode("b a r")); // single quotes
+    TRY_CHECK(eval(" [foo].replace(\"(\\B)|( )\",\"$1 \") ") == tr.transcode("b a r")); // double quotes
 
     // https://en.wikipedia.org/wiki/Chess_symbols_in_Unicode
     //'\u265C\u265E\u265D\u265B\u265A\u265D\u265E\u265C' - black chess figures
@@ -185,14 +196,26 @@ TEST_CASE("expressions")
     TRY_CHECK(val0.to_string() == val1.to_string()); // UTF-8
     TRY_CHECK(val0.to_unicode() == val1.to_unicode()); // Unicode
     // \u+NNNN \U+NNNNNNNN \xNN\xNN
-    auto val3 = eval(u8"'\u262f\xF0\x9F\x8D\xB7'");
+    // single quotes
-    auto val4 = eval(u8"'\U0000262f\U0001F377'");
+    auto val3 = eval("'\\u262f\\xF0\\x9F\\x8D\\xB7'");
+    auto val4 = eval("'\\U0000262f\\U0001F377'");
+    // double quotes
+    auto val5 = eval("\"\\u262f\\xF0\\x9F\\x8D\\xB7\"");
+    auto val6 = eval("\"\\U0000262f\\U0001F377\"");
     // UTF16 surrogate pairs work also ;)
-    // \ud83d\udd7a\ud83c\udffc => \U0001F57A\U0001F3FC works also
+    auto val7 = eval("'\\ud83d\\udd7a\\ud83c\\udffc'");
-    // TODO: find a way to enter UTF16 pairs
+    auto val8 = eval("'\\U0001F57A\\U0001F3FC'");
+
     TRY_CHECK(val3 == val4);
+    TRY_CHECK(val5 == val6);
     TRY_CHECK(val3.to_string() == val4.to_string()); // UTF-8
     TRY_CHECK(val3.to_unicode() == val4.to_unicode()); // Unicode
+    TRY_CHECK(val5.to_string() == val6.to_string()); // UTF-8
+    TRY_CHECK(val5.to_unicode() == val6.to_unicode()); // Unicode
+    TRY_CHECK(val7 == val8);
+    TRY_CHECK(val7.to_string() == val8.to_string()); // UTF-8
+    TRY_CHECK(val7.to_unicode() == val8.to_unicode()); // Unicode
+
 
     // following test will fail if boost_regex is built without ICU support (unpaired surrogates in output)
     TRY_CHECK(eval("[name].replace('(\\B)|( )',' ') ") == tr.transcode("Q u é b e c"));

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_longitude", mapnik::value_double(-70.0) }
-                        , attr { "geo_latitude", mapnik::value_integer(40) }
+                        , attr { "geo_latitude", mapnik::value_double(40.0) }
                     };
-                require_attributes(feature, expected_attr);
+                REQUIRE_ATTRIBUTES(feature, expected_attr);
-                require_attributes(feature2, 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) \
+#define REQUIRE_ATTRIBUTES(feature, ...) \
-    REQUIRE(bool(feature)); \
+    do { \
-    for (auto const &kv : attrs) { \
+        auto const& _feat = (feature); /* evaluate feature only once */ \
-        REQUIRE(feature->has_key(std::get<0>(kv))); \
+        REQUIRE(_feat != nullptr); \
-        CHECK(feature->get(std::get<0>(kv)) == std::get<1>(kv)); \
+        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()); \
             } \
-
+        } \
-
+    } while (0)
-inline void require_attributes(mapnik::feature_ptr feature,
-                        std::initializer_list<attr> const &attrs) {
-    REQUIRE_ATTRIBUTES(feature, attrs);
-}
 
 namespace detail {
 

test/unit/datasource/geojson.cpp

@@ -824,7 +824,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

@@ -100,7 +100,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)},

test/unit/projection/proj_transform.cpp

@@ -119,4 +119,80 @@ SECTION("test pj_transform failure behavior")
 
 #endif
 
+// Github Issue https://github.com/mapnik/mapnik/issues/2648
+SECTION("Test proj antimeridian bbox")
+{
+    mapnik::projection prj_geog("+init=epsg:4326");
+    mapnik::projection prj_proj("+init=epsg:2193");
+
+    mapnik::proj_transform prj_trans_fwd(prj_proj, prj_geog);
+    mapnik::proj_transform prj_trans_rev(prj_geog, prj_proj);
+
+    // reference values taken from proj4 command line tool:
+    // (non-corner points assume PROJ_ENVELOPE_POINTS == 20)
+    //
+    //  cs2cs -Ef %.10f +init=epsg:2193 +to +init=epsg:4326 <<END
+    //        2105800 3087000 # left-most
+    //        1495200 3087000 # bottom-most
+    //        2105800 7173000 # right-most
+    //        3327000 7173000 # top-most
+    //  END
+    //
+    // wrong = mapnik.Box2d(-177.3145325044, -62.3337481525,
+    //                       178.0277836332, -24.5845974912)
+    const mapnik::box2d<double> better(-180.0, -62.3337481525,
+                                        180.0, -24.5845974912);
+
+    {
+        mapnik::box2d<double> ext(274000, 3087000, 3327000, 7173000);
+        prj_trans_fwd.forward(ext, PROJ_ENVELOPE_POINTS);
+        CHECK(ext.minx() == Approx(better.minx()));
+        CHECK(ext.miny() == Approx(better.miny()));
+        CHECK(ext.maxx() == Approx(better.maxx()));
+        CHECK(ext.maxy() == Approx(better.maxy()));
+    }
+
+    {
+        // check the same logic works for .backward()
+        mapnik::box2d<double> ext(274000, 3087000, 3327000, 7173000);
+        prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS);
+        CHECK(ext.minx() == Approx(better.minx()));
+        CHECK(ext.miny() == Approx(better.miny()));
+        CHECK(ext.maxx() == Approx(better.maxx()));
+        CHECK(ext.maxy() == Approx(better.maxy()));
+    }
+
+    // reference values taken from proj4 command line tool:
+    //
+    //  cs2cs -Ef %.10f +init=epsg:2193 +to +init=epsg:4326 <<END
+    //        274000 3087000 # left-most
+    //        276000 3087000 # bottom-most
+    //        276000 7173000 # right-most
+    //        274000 7173000 # top-most
+    //  END
+    //
+    const mapnik::box2d<double> normal(148.7667597489, -60.1222810241,
+                                       159.9548489296, -24.9771195155);
+
+    {
+        // checks for not being snapped (ie. not antimeridian)
+        mapnik::box2d<double> ext(274000, 3087000, 276000, 7173000);
+        prj_trans_fwd.forward(ext, PROJ_ENVELOPE_POINTS);
+        CHECK(ext.minx() == Approx(normal.minx()));
+        CHECK(ext.miny() == Approx(normal.miny()));
+        CHECK(ext.maxx() == Approx(normal.maxx()));
+        CHECK(ext.maxy() == Approx(normal.maxy()));
+    }
+
+    {
+        // check the same logic works for .backward()
+        mapnik::box2d<double> ext(274000, 3087000, 276000, 7173000);
+        prj_trans_rev.backward(ext, PROJ_ENVELOPE_POINTS);
+        CHECK(ext.minx() == Approx(normal.minx()));
+        CHECK(ext.miny() == Approx(normal.miny()));
+        CHECK(ext.maxx() == Approx(normal.maxx()));
+        CHECK(ext.maxy() == Approx(normal.maxy()));
+    }
+}
+
 }