From 71344f78df2de0d3db9f36b47df087cd12074e55 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 3 Jul 2017 15:18:23 +0200 Subject: [PATCH 01/20] wkb_reader : pre-allocate number of polygons in `multi_polygon` and number of geometries in `geometry_collection` via `vector.reserce(...)` --- src/wkb.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wkb.cpp b/src/wkb.cpp index 644ee81ae..6414e65d7 100644 --- a/src/wkb.cpp +++ b/src/wkb.cpp @@ -351,6 +351,7 @@ private: { int num_polys = read_integer(); mapnik::geometry::multi_polygon multi_poly; + multi_poly.reserve(num_polys); for (int i = 0; i < num_polys; ++i) { pos_ += 5; @@ -363,6 +364,7 @@ private: { int num_geometries = read_integer(); mapnik::geometry::geometry_collection collection; + collection.reserve(num_geometries); for (int i = 0; i < num_geometries; ++i) { pos_ += 1; // skip byte order From d0ecd51b6337a3d0bc832816f9d758756224f298 Mon Sep 17 00:00:00 2001 From: talaj Date: Mon, 3 Jul 2017 17:15:07 +0200 Subject: [PATCH 02/20] Set premultiplied flag to the color font glyph bitmap (#3716) * color font bitmap is premultiplied * update visual tests --- src/text/renderer.cpp | 1 + test/data-visual | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/text/renderer.cpp b/src/text/renderer.cpp index 9e0833f93..692190b8e 100644 --- a/src/text/renderer.cpp +++ b/src/text/renderer.cpp @@ -157,6 +157,7 @@ void composite_color_bitmap(T & pixmap, FT_Bitmap *bitmap, int x, int y, double int scaled_height = bitmap->rows * scale; image_rgba8 scaled_image(scaled_width, scaled_height); scale_image_agg(scaled_image, image , SCALING_BILINEAR , scale, scale, 0.0, 0.0, 1.0, 0); + set_premultiplied_alpha(scaled_image, true); composite(pixmap, scaled_image, comp_op, opacity, x, y); } diff --git a/test/data-visual b/test/data-visual index fd518f1f5..674c5402e 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit fd518f1f512b8aea4ac740c2ce12c249616a291c +Subproject commit 674c5402e6275905ddb7b1fccb0376cae2ec50e0 From 53249053e42be3337518f8cd616e6c137ef3528d Mon Sep 17 00:00:00 2001 From: talaj Date: Tue, 4 Jul 2017 16:32:45 +0200 Subject: [PATCH 03/20] cairo renderer: clear map background with given color as agg renderer does (#3718) --- src/cairo/cairo_renderer.cpp | 1 + test/unit/map/background.cpp | 66 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 test/unit/map/background.cpp diff --git a/src/cairo/cairo_renderer.cpp b/src/cairo/cairo_renderer.cpp index 162fd6096..e3a8f34e9 100644 --- a/src/cairo/cairo_renderer.cpp +++ b/src/cairo/cairo_renderer.cpp @@ -149,6 +149,7 @@ void cairo_renderer::setup(Map const& map) { cairo_save_restore guard(context_); context_.set_color(*bg); + context_.set_operator(composite_mode_e::src); context_.paint(); } boost::optional const& image_filename = map.background_image(); diff --git a/test/unit/map/background.cpp b/test/unit/map/background.cpp new file mode 100644 index 000000000..18c1b4bf6 --- /dev/null +++ b/test/unit/map/background.cpp @@ -0,0 +1,66 @@ + +#include "catch.hpp" + +#include +#include + +#if defined(HAVE_CAIRO) +#include +#include +#endif + +TEST_CASE("map") { + +SECTION("set background - agg") { + + mapnik::Map map(256, 256); + mapnik::image_rgba8 image(map.width(), map.height()); + const mapnik::color c1(255, 0, 0); + mapnik::fill(image, c1); + + CHECK(image(0, 0) == c1.rgba()); + + // Fully transparent black should replace red color + const mapnik::color c2(0, 0, 0, 0); + map.set_background(c2); + mapnik::agg_renderer ren(map, image); + ren.apply(); + + CHECK(image(0, 0) == c2.rgba()); +} + +#if defined(HAVE_CAIRO) +SECTION("set background - cairo") { + + mapnik::Map map(256, 256); + mapnik::cairo_surface_ptr image_surface( + cairo_image_surface_create(CAIRO_FORMAT_ARGB32, map.width(), map.height()), + mapnik::cairo_surface_closer()); + mapnik::cairo_ptr image_context(mapnik::create_context(image_surface)); + + cairo_set_source_rgba(image_context.get(), 1.0, 0.0, 0.0, 1.0); + cairo_paint(image_context.get()); + + { + mapnik::image_rgba8 image(map.width(), map.height()); + mapnik::cairo_image_to_rgba8(image, image_surface); + const mapnik::color c1(255, 0, 0); + CHECK(image(0, 0) == c1.rgba()); + } + + // Fully transparent black should replace red color + const mapnik::color c2(0, 0, 0, 0); + map.set_background(c2); + mapnik::cairo_renderer ren(map, image_context, 1.0); + ren.apply(); + + { + mapnik::image_rgba8 image(map.width(), map.height()); + mapnik::cairo_image_to_rgba8(image, image_surface); + CHECK(image(0, 0) == c2.rgba()); + } +} +#endif + +} + From 4c9322bc0df1fc480fc99f82ea9a0d99ea10bdd2 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 10:59:42 +0200 Subject: [PATCH 04/20] re-use `lamdas` definitions and aviod duplicate symbols when building with vs2017 --- include/mapnik/json/feature_grammar_x3_def.hpp | 11 +---------- include/mapnik/json/generic_json_grammar_x3.hpp | 15 +++++++++++++++ .../mapnik/json/generic_json_grammar_x3_def.hpp | 16 ---------------- 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/include/mapnik/json/feature_grammar_x3_def.hpp b/include/mapnik/json/feature_grammar_x3_def.hpp index d6432c5d6..7769a20ff 100644 --- a/include/mapnik/json/feature_grammar_x3_def.hpp +++ b/include/mapnik/json/feature_grammar_x3_def.hpp @@ -164,15 +164,6 @@ struct geometry_type_ : x3::symbols } } geometry_type_symbols; -auto assign_name = [](auto const& ctx) -{ - std::get<0>(_val(ctx)) = std::move(_attr(ctx)); -}; -auto assign_value = [](auto const& ctx) -{ - std::get<1>(_val(ctx)) = std::move(_attr(ctx)); -}; - auto const assign_geometry_type = [] (auto const& ctx) { std::get<0>(_val(ctx)) = _attr(ctx); @@ -263,7 +254,7 @@ auto const geometry_tuple_def = (geometry_type[assign_geometry_type] (omit[geojson_string] > lit(':') > omit[value])) % lit(','); -auto const property_def = geojson_string[assign_name] > lit(':') > value[assign_value]; +auto const property_def = geojson_string[assign_key] > lit(':') > value[assign_value]; auto const properties_def = property[assign_property] % lit(','); diff --git a/include/mapnik/json/generic_json_grammar_x3.hpp b/include/mapnik/json/generic_json_grammar_x3.hpp index 802db8459..27cd92c44 100644 --- a/include/mapnik/json/generic_json_grammar_x3.hpp +++ b/include/mapnik/json/generic_json_grammar_x3.hpp @@ -43,6 +43,21 @@ BOOST_SPIRIT_DECLARE(generic_json_grammar_type); BOOST_SPIRIT_DECLARE(generic_json_key_value_type); } +auto assign = [](auto const& ctx) +{ + _val(ctx) = _attr(ctx); +}; + +auto assign_key = [](auto const& ctx) +{ + std::get<0>(_val(ctx)) = _attr(ctx); +}; + +auto assign_value = [](auto const& ctx) +{ + std::get<1>(_val(ctx)) = std::move(_attr(ctx)); +}; + grammar::generic_json_grammar_type const& generic_json_grammar(); grammar::generic_json_key_value_type const& generic_json_key_value(); diff --git a/include/mapnik/json/generic_json_grammar_x3_def.hpp b/include/mapnik/json/generic_json_grammar_x3_def.hpp index 8c30cb37d..f15334642 100644 --- a/include/mapnik/json/generic_json_grammar_x3_def.hpp +++ b/include/mapnik/json/generic_json_grammar_x3_def.hpp @@ -46,22 +46,6 @@ auto make_false = [] (auto const& ctx) _val(ctx) = false; }; -auto assign = [](auto const& ctx) -{ - _val(ctx) = _attr(ctx); -}; - -auto assign_key = [](auto const& ctx) -{ - std::get<0>(_val(ctx)) = _attr(ctx); -}; - -auto assign_value = [](auto const& ctx) -{ - std::get<1>(_val(ctx)) = _attr(ctx); -}; - - using x3::lit; using x3::string; // exported rules From 0ad1c580094c9ecbe95ea71c1c5bb8c735d853a6 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 11:11:01 +0200 Subject: [PATCH 05/20] trying to `inline` lambdas (VS2017) (!) --- include/mapnik/json/generic_json_grammar_x3.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mapnik/json/generic_json_grammar_x3.hpp b/include/mapnik/json/generic_json_grammar_x3.hpp index 27cd92c44..2da057596 100644 --- a/include/mapnik/json/generic_json_grammar_x3.hpp +++ b/include/mapnik/json/generic_json_grammar_x3.hpp @@ -43,17 +43,17 @@ BOOST_SPIRIT_DECLARE(generic_json_grammar_type); BOOST_SPIRIT_DECLARE(generic_json_key_value_type); } -auto assign = [](auto const& ctx) +inline auto assign = [](auto const& ctx) { _val(ctx) = _attr(ctx); }; -auto assign_key = [](auto const& ctx) +inline auto assign_key = [](auto const& ctx) { - std::get<0>(_val(ctx)) = _attr(ctx); + std::get<0>(_val(ctx)) = std::move(_attr(ctx)); }; -auto assign_value = [](auto const& ctx) +inline auto assign_value = [](auto const& ctx) { std::get<1>(_val(ctx)) = std::move(_attr(ctx)); }; From 631c0a59c42108a79a4e3bb4108f39f46e5312eb Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 11:14:24 +0200 Subject: [PATCH 06/20] Revert "trying to `inline` lambdas (VS2017) (!)" this is not allowed in c++14 requires c++17 This reverts commit 0ad1c580094c9ecbe95ea71c1c5bb8c735d853a6. --- include/mapnik/json/generic_json_grammar_x3.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mapnik/json/generic_json_grammar_x3.hpp b/include/mapnik/json/generic_json_grammar_x3.hpp index 2da057596..27cd92c44 100644 --- a/include/mapnik/json/generic_json_grammar_x3.hpp +++ b/include/mapnik/json/generic_json_grammar_x3.hpp @@ -43,17 +43,17 @@ BOOST_SPIRIT_DECLARE(generic_json_grammar_type); BOOST_SPIRIT_DECLARE(generic_json_key_value_type); } -inline auto assign = [](auto const& ctx) +auto assign = [](auto const& ctx) { _val(ctx) = _attr(ctx); }; -inline auto assign_key = [](auto const& ctx) +auto assign_key = [](auto const& ctx) { - std::get<0>(_val(ctx)) = std::move(_attr(ctx)); + std::get<0>(_val(ctx)) = _attr(ctx); }; -inline auto assign_value = [](auto const& ctx) +auto assign_value = [](auto const& ctx) { std::get<1>(_val(ctx)) = std::move(_attr(ctx)); }; From e08616aed3b83ffa70548cf96acccae974d67c3c Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 11:17:14 +0200 Subject: [PATCH 07/20] Didn't play well with VS2017, reverting "re-use `lamdas` definitions and aviod duplicate symbols when building with vs2017" This reverts commit 4c9322bc0df1fc480fc99f82ea9a0d99ea10bdd2. --- include/mapnik/json/feature_grammar_x3_def.hpp | 11 ++++++++++- include/mapnik/json/generic_json_grammar_x3.hpp | 15 --------------- .../mapnik/json/generic_json_grammar_x3_def.hpp | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/include/mapnik/json/feature_grammar_x3_def.hpp b/include/mapnik/json/feature_grammar_x3_def.hpp index 7769a20ff..d6432c5d6 100644 --- a/include/mapnik/json/feature_grammar_x3_def.hpp +++ b/include/mapnik/json/feature_grammar_x3_def.hpp @@ -164,6 +164,15 @@ struct geometry_type_ : x3::symbols } } geometry_type_symbols; +auto assign_name = [](auto const& ctx) +{ + std::get<0>(_val(ctx)) = std::move(_attr(ctx)); +}; +auto assign_value = [](auto const& ctx) +{ + std::get<1>(_val(ctx)) = std::move(_attr(ctx)); +}; + auto const assign_geometry_type = [] (auto const& ctx) { std::get<0>(_val(ctx)) = _attr(ctx); @@ -254,7 +263,7 @@ auto const geometry_tuple_def = (geometry_type[assign_geometry_type] (omit[geojson_string] > lit(':') > omit[value])) % lit(','); -auto const property_def = geojson_string[assign_key] > lit(':') > value[assign_value]; +auto const property_def = geojson_string[assign_name] > lit(':') > value[assign_value]; auto const properties_def = property[assign_property] % lit(','); diff --git a/include/mapnik/json/generic_json_grammar_x3.hpp b/include/mapnik/json/generic_json_grammar_x3.hpp index 27cd92c44..802db8459 100644 --- a/include/mapnik/json/generic_json_grammar_x3.hpp +++ b/include/mapnik/json/generic_json_grammar_x3.hpp @@ -43,21 +43,6 @@ BOOST_SPIRIT_DECLARE(generic_json_grammar_type); BOOST_SPIRIT_DECLARE(generic_json_key_value_type); } -auto assign = [](auto const& ctx) -{ - _val(ctx) = _attr(ctx); -}; - -auto assign_key = [](auto const& ctx) -{ - std::get<0>(_val(ctx)) = _attr(ctx); -}; - -auto assign_value = [](auto const& ctx) -{ - std::get<1>(_val(ctx)) = std::move(_attr(ctx)); -}; - grammar::generic_json_grammar_type const& generic_json_grammar(); grammar::generic_json_key_value_type const& generic_json_key_value(); diff --git a/include/mapnik/json/generic_json_grammar_x3_def.hpp b/include/mapnik/json/generic_json_grammar_x3_def.hpp index f15334642..8c30cb37d 100644 --- a/include/mapnik/json/generic_json_grammar_x3_def.hpp +++ b/include/mapnik/json/generic_json_grammar_x3_def.hpp @@ -46,6 +46,22 @@ auto make_false = [] (auto const& ctx) _val(ctx) = false; }; +auto assign = [](auto const& ctx) +{ + _val(ctx) = _attr(ctx); +}; + +auto assign_key = [](auto const& ctx) +{ + std::get<0>(_val(ctx)) = _attr(ctx); +}; + +auto assign_value = [](auto const& ctx) +{ + std::get<1>(_val(ctx)) = _attr(ctx); +}; + + using x3::lit; using x3::string; // exported rules From e653f4cc4c8891486cecf3132c504248e2c6d4f9 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 16:01:06 +0200 Subject: [PATCH 08/20] make constructor explicit to avoid C2664 error with VS2017. --- include/mapnik/transform/transform_expression.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mapnik/transform/transform_expression.hpp b/include/mapnik/transform/transform_expression.hpp index 1b6649f42..18c5ac092 100644 --- a/include/mapnik/transform/transform_expression.hpp +++ b/include/mapnik/transform/transform_expression.hpp @@ -171,7 +171,7 @@ struct transform_node : base_() {} template - transform_node(T const& val) + explicit transform_node(T const& val) : base_(val) {} template From 55776f0b8c10b9fad59f73df4528a5a236f08d55 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 16:02:17 +0200 Subject: [PATCH 09/20] add missing include directives (VS2017) --- test/unit/datasource/geojson.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/datasource/geojson.cpp b/test/unit/datasource/geojson.cpp index 460b263ac..60af00de8 100644 --- a/test/unit/datasource/geojson.cpp +++ b/test/unit/datasource/geojson.cpp @@ -33,6 +33,8 @@ #include #include #include +#include +#include #include /* From cd0c746a422c89f775352fb8f575571899ac97b4 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 16:02:47 +0200 Subject: [PATCH 10/20] split grammar into two rules (VS2017) --- test/visual/parse_map_sizes.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/visual/parse_map_sizes.cpp b/test/visual/parse_map_sizes.cpp index 56df50e99..d0df2db9d 100644 --- a/test/visual/parse_map_sizes.cpp +++ b/test/visual/parse_map_sizes.cpp @@ -38,8 +38,9 @@ namespace visual_tests { namespace x3 = boost::spirit::x3; using x3::ulong_; -auto const map_sizes_grammar = x3::rule >{} = - (ulong_ >> ',' >> ulong_) % ';' ; +auto const map_size_rule = x3::rule {} = ulong_ >> ',' >> ulong_; +auto const map_sizes_grammar = x3::rule > {} = + map_size_rule % ';' ; void parse_map_sizes(std::string const & str, std::vector & sizes) { From b632633373828ca881eaaef5a9859aee7a8f3bc6 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 16:03:28 +0200 Subject: [PATCH 11/20] convert args to mapnik::geometry::geometry (VS2017) --- test/unit/serialization/wkb_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/serialization/wkb_test.cpp b/test/unit/serialization/wkb_test.cpp index c19b8cdca..8eea6d3bf 100644 --- a/test/unit/serialization/wkb_test.cpp +++ b/test/unit/serialization/wkb_test.cpp @@ -129,8 +129,8 @@ TEST_CASE("Well-known-geometries") geom_3.emplace_back(0,0); geom_3.emplace_back(1,1); geom_3.emplace_back(2,2); - REQUIRE(mapnik::util::to_wkt(wkt0, geom_2)); - REQUIRE(mapnik::util::to_wkt(wkt1, geom_3)); + REQUIRE(mapnik::util::to_wkt(wkt0, mapnik::geometry::geometry(geom_2))); + REQUIRE(mapnik::util::to_wkt(wkt1, mapnik::geometry::geometry(geom_3))); if (!mapnik::geometry::is_empty(geom_2) && !mapnik::geometry::is_empty(geom_3)) { REQUIRE(wkt2 == wkt3); From 3ae71eea0fdadfd9e5a63f0e25f64411e857fe22 Mon Sep 17 00:00:00 2001 From: artemp Date: Thu, 6 Jul 2017 16:09:38 +0200 Subject: [PATCH 12/20] JSON - wrap lamda functions into anonymous namespace to fix linking on Windows with VS2017. --- include/mapnik/json/feature_grammar_x3_def.hpp | 4 ++++ include/mapnik/json/generic_json_grammar_x3_def.hpp | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/mapnik/json/feature_grammar_x3_def.hpp b/include/mapnik/json/feature_grammar_x3_def.hpp index d6432c5d6..d05219350 100644 --- a/include/mapnik/json/feature_grammar_x3_def.hpp +++ b/include/mapnik/json/feature_grammar_x3_def.hpp @@ -164,6 +164,8 @@ struct geometry_type_ : x3::symbols } } geometry_type_symbols; +namespace { + auto assign_name = [](auto const& ctx) { std::get<0>(_val(ctx)) = std::move(_attr(ctx)); @@ -173,6 +175,8 @@ auto assign_value = [](auto const& ctx) std::get<1>(_val(ctx)) = std::move(_attr(ctx)); }; +} // VS2017 + auto const assign_geometry_type = [] (auto const& ctx) { std::get<0>(_val(ctx)) = _attr(ctx); diff --git a/include/mapnik/json/generic_json_grammar_x3_def.hpp b/include/mapnik/json/generic_json_grammar_x3_def.hpp index 8c30cb37d..c16d9477a 100644 --- a/include/mapnik/json/generic_json_grammar_x3_def.hpp +++ b/include/mapnik/json/generic_json_grammar_x3_def.hpp @@ -31,6 +31,8 @@ namespace mapnik { namespace json { namespace grammar { namespace x3 = boost::spirit::x3; +namespace { + auto make_null = [] (auto const& ctx) { _val(ctx) = mapnik::value_null{}; @@ -48,19 +50,20 @@ auto make_false = [] (auto const& ctx) auto assign = [](auto const& ctx) { - _val(ctx) = _attr(ctx); + _val(ctx) = std::move(_attr(ctx)); }; auto assign_key = [](auto const& ctx) { - std::get<0>(_val(ctx)) = _attr(ctx); + std::get<0>(_val(ctx)) = std::move(_attr(ctx)); }; auto assign_value = [](auto const& ctx) { - std::get<1>(_val(ctx)) = _attr(ctx); + std::get<1>(_val(ctx)) = std::move(_attr(ctx)); }; +} // VS2017 using x3::lit; using x3::string; From 951f10791a1f2e05fcf8f657fe88a35e2c3f8f62 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Thu, 6 Jul 2017 12:30:05 -0500 Subject: [PATCH 13/20] Fix for #3714 in master, addresses RGBA tiffs that have an alpha value and no data value - and are using the no data value over the alpha. --- plugins/input/gdal/gdal_featureset.cpp | 2 +- test/data | 2 +- test/data-visual | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/input/gdal/gdal_featureset.cpp b/plugins/input/gdal/gdal_featureset.cpp index 02f2dd745..979f9e587 100644 --- a/plugins/input/gdal/gdal_featureset.cpp +++ b/plugins/input/gdal/gdal_featureset.cpp @@ -545,7 +545,7 @@ feature_ptr gdal_featureset::get_feature(mapnik::query const& q) if (alpha) { MAPNIK_LOG_DEBUG(gdal) << "gdal_featureset: processing alpha band..."; - if (!raster_has_nodata) + if (!raster_has_nodata || (red && green && blue)) { raster_io_error = alpha->RasterIO(GF_Read, x_off, y_off, width, height, image.bytes() + 3, image.width(), image.height(), GDT_Byte, 4, 4 * image.width()); diff --git a/test/data b/test/data index 99da07d5e..f95fe1c7b 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 99da07d5e76ccf5978ef0a380bf5f631f9088584 +Subproject commit f95fe1c7b56a5eeb4fa2c2bcdc403d9254ce7448 diff --git a/test/data-visual b/test/data-visual index 674c5402e..df578e343 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit 674c5402e6275905ddb7b1fccb0376cae2ec50e0 +Subproject commit df578e3436681bb9bc582c7ac55a4205e98334f4 From b164117d99ea51a8248ddf53d983fb965f306e65 Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 7 Jul 2017 10:09:08 +0200 Subject: [PATCH 14/20] disable handling unsupported attributes (leaving only well-known unsupported elements) to reduce verbosity. --- src/svg/svg_parser.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index 15e32699d..1638c3f6c 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -112,7 +112,7 @@ static std::array const unsupported_elements name_to_int("a")} }; - +#if 0 // disable to reduce verbosity static std::array const unsupported_attributes { {name_to_int("alignment-baseline"), name_to_int("baseline-shift"), @@ -158,6 +158,8 @@ static std::array const unsupported_attributes name_to_int("writing-mode")} }; +#endif + template void handle_unsupported(svg_parser& parser, T const& ar, char const* name) { @@ -697,7 +699,8 @@ void parse_attr(svg_parser & parser, char const* name, char const* value ) } break; default: - handle_unsupported(parser, unsupported_attributes, name); + //handle_unsupported(parser, unsupported_attributes, name); + // disable for now to reduce verbosity break; } } From a3247c8dce50be1028fdfb9836f7a63471dc6542 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Fri, 7 Jul 2017 20:44:15 +0000 Subject: [PATCH 15/20] remove superfluous line --- src/load_map.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/load_map.cpp b/src/load_map.cpp index 6db669709..4d6066755 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -1380,7 +1380,6 @@ void map_parser::parse_raster_symbolizer(rule & rule, xml_node const & node) { found_colorizer = true; raster_colorizer_ptr colorizer = std::make_shared(); - put(raster_sym, keys::colorizer, colorizer); if (parse_raster_colorizer(colorizer, css)) put(raster_sym, keys::colorizer, colorizer); } From 24d07e7792072aec67318c11e75e44a9b410203e Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 18 Jul 2017 10:08:16 +0200 Subject: [PATCH 16/20] Update CHANGELOG with missing #3688 [skip-ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e1ad8c26..f42e90da2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Released: June 5, 2017 - shapeindex - return error code when no features can read from shapefile (#3198) - Upgrade Scons to `2.5.1` - Fixed bug (typo) in `raster_featureset.cpp` (#3696) +- Made `freetype_engine` singleton again. This allows for better control of its life-time. Original interface is preserved via adding static methods (#3688) ## 3.0.13 From 9e58c890430db6f0b6f1f7a1690877c9d913d92a Mon Sep 17 00:00:00 2001 From: artemp Date: Wed, 19 Jul 2017 16:07:39 +0200 Subject: [PATCH 17/20] Add support for U_ICU_VERSION_MAJOR_NUM >= 59 (#3729) --- include/mapnik/text/harfbuzz_shaper.hpp | 4 +++- include/mapnik/unicode.hpp | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/mapnik/text/harfbuzz_shaper.hpp b/include/mapnik/text/harfbuzz_shaper.hpp index 534feb633..a62071800 100644 --- a/include/mapnik/text/harfbuzz_shaper.hpp +++ b/include/mapnik/text/harfbuzz_shaper.hpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #pragma GCC diagnostic pop @@ -54,7 +55,8 @@ static inline hb_script_t _icu_script_to_script(UScriptCode script) static inline const uint16_t * uchar_to_utf16(const UChar* src) { static_assert(sizeof(UChar) == sizeof(uint16_t),"UChar is eq size to uint16_t"); -#if defined(_MSC_VER) +#if defined(_MSC_VER) || (U_ICU_VERSION_MAJOR_NUM >= 59) + // ^^ http://site.icu-project.org/download/59#TOC-ICU4C-char16_t1 return reinterpret_cast(src); #else return src; diff --git a/include/mapnik/unicode.hpp b/include/mapnik/unicode.hpp index 18034d262..695ca4586 100644 --- a/include/mapnik/unicode.hpp +++ b/include/mapnik/unicode.hpp @@ -31,6 +31,8 @@ // std #include #include +// icu +#include struct UConverter; From 093fcee6d1ba1fd360718ceade83894aeffc2548 Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 21 Jul 2017 10:11:29 +0200 Subject: [PATCH 18/20] only include if ICU >= 59 (attempting to fix current `coverage` build on travis). --- include/mapnik/unicode.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/mapnik/unicode.hpp b/include/mapnik/unicode.hpp index 695ca4586..798bfcb4e 100644 --- a/include/mapnik/unicode.hpp +++ b/include/mapnik/unicode.hpp @@ -32,7 +32,12 @@ #include #include // icu +#if (U_ICU_VERSION_MAJOR_NUM >= 59) +#pragma GCC diagnostic push +#include #include +#pragma GCC diagnostic pop +#endif struct UConverter; From a26723b4a5cee3b2a94182af6122955723720755 Mon Sep 17 00:00:00 2001 From: artemp Date: Wed, 2 Aug 2017 10:30:37 +0100 Subject: [PATCH 19/20] add missing ' in error message. --- src/svg/svg_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index 1638c3f6c..dc054ac55 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -168,7 +168,7 @@ void handle_unsupported(svg_parser& parser, T const& ar, char const* name) { if (e == element) { - parser.err_handler().on_error(std::string("Unsupported:\"") + name); + parser.err_handler().on_error(std::string("Unsupported:'") + name + "'"); } } } From 6f73181e9be93327ff7378c904ec1cf1f70a976c Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 31 Jul 2017 09:18:12 +0100 Subject: [PATCH 20/20] Enforce consistent error handling policy - always throw on fatal errors (both strict and non-strict). In `strict` mode throw on first parsing error. Remove return values from `parse`,`parse_from_string` and `traverse_tree` methods. Update unit tests. --- include/mapnik/svg/svg_parser.hpp | 4 +- src/marker_cache.cpp | 9 +- src/svg/svg_parser.cpp | 20 ++--- test/unit/svg/svg_parser_test.cpp | 134 ++++++++++++++++++++++++------ 4 files changed, 122 insertions(+), 45 deletions(-) diff --git a/include/mapnik/svg/svg_parser.hpp b/include/mapnik/svg/svg_parser.hpp index 7a8e88f91..fab0b8e19 100644 --- a/include/mapnik/svg/svg_parser.hpp +++ b/include/mapnik/svg/svg_parser.hpp @@ -69,8 +69,8 @@ public: explicit svg_parser(svg_converter_type & path, bool strict = false); ~svg_parser(); error_handler & err_handler(); - bool parse(std::string const& filename); - bool parse_from_string(std::string const& svg); + void parse(std::string const& filename); + void parse_from_string(std::string const& svg); svg_converter_type & path_; bool is_defs_; bool strict_; diff --git a/src/marker_cache.cpp b/src/marker_cache.cpp index 2e223ec90..d7ea7facb 100644 --- a/src/marker_cache.cpp +++ b/src/marker_cache.cpp @@ -175,14 +175,14 @@ std::shared_ptr marker_cache::find(std::string const& uri, svg_path_adapter svg_path(stl_storage); svg_converter_type svg(svg_path, marker_path->attributes()); svg_parser p(svg, strict); + p.parse_from_string(known_svg_string); - if (!p.parse_from_string(known_svg_string) && !strict) + if (!strict) { for (auto const& msg : p.err_handler().error_messages()) { MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; } - //return std::make_shared(mapnik::marker_null()); } //svg.arrange_orientations(); double lox,loy,hix,hiy; @@ -215,15 +215,14 @@ std::shared_ptr marker_cache::find(std::string const& uri, svg_path_adapter svg_path(stl_storage); svg_converter_type svg(svg_path, marker_path->attributes()); svg_parser p(svg, strict); + p.parse(uri); - - if (!p.parse(uri) && !strict) + if (!strict) { for (auto const& msg : p.err_handler().error_messages()) { MAPNIK_LOG_ERROR(marker_cache) << "SVG PARSING ERROR:\"" << msg << "\""; } - //return std::make_shared(mapnik::marker_null()); } //svg.arrange_orientations(); double lox,loy,hix,hiy; diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index dc054ac55..0dc14c440 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -80,7 +80,7 @@ namespace mapnik { namespace svg { namespace rapidxml = boost::property_tree::detail::rapidxml; -bool traverse_tree(svg_parser& parser, rapidxml::xml_node const* node); +void traverse_tree(svg_parser& parser, rapidxml::xml_node const* node); void end_element(svg_parser& parser, rapidxml::xml_node const* node); void parse_path(svg_parser& parser, rapidxml::xml_node const* node); void parse_element(svg_parser& parser, char const* name, rapidxml::xml_node const* node); @@ -370,7 +370,7 @@ bool parse_id_from_url (char const* str, std::string & id) x3::space); } -bool traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) +void traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) { auto const* name = node->name(); switch (node->type()) @@ -466,7 +466,6 @@ bool traverse_tree(svg_parser & parser, rapidxml::xml_node const* node) default: break; } - return true; } @@ -1406,7 +1405,7 @@ svg_parser::svg_parser(svg_converter stream(mapnik::utf8_to_utf16(filename)); @@ -1417,8 +1416,7 @@ bool svg_parser::parse(std::string const& filename) { std::stringstream ss; ss << "Unable to open '" << filename << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } stream.unsetf(std::ios::skipws); @@ -1436,8 +1434,7 @@ bool svg_parser::parse(std::string const& filename) { std::stringstream ss; ss << "svg_parser::parse - Unable to parse '" << filename << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } for (rapidxml::xml_node const* child = doc.first_node(); @@ -1445,10 +1442,9 @@ bool svg_parser::parse(std::string const& filename) { traverse_tree(*this, child); } - return err_handler_.error_messages().empty() ? true : false; } -bool svg_parser::parse_from_string(std::string const& svg) +void svg_parser::parse_from_string(std::string const& svg) { const int flags = rapidxml::parse_trim_whitespace | rapidxml::parse_validate_closing_tags; rapidxml::xml_document<> doc; @@ -1462,15 +1458,13 @@ bool svg_parser::parse_from_string(std::string const& svg) { std::stringstream ss; ss << "Unable to parse '" << svg << "'"; - err_handler_.on_error(ss.str()); - return false; + throw std::runtime_error(ss.str()); } for (rapidxml::xml_node const* child = doc.first_node(); child; child = child->next_sibling()) { traverse_tree(*this, child); } - return err_handler_.error_messages().empty() ? true : false; } svg_parser::error_handler & svg_parser::err_handler() diff --git a/test/unit/svg/svg_parser_test.cpp b/test/unit/svg/svg_parser_test.cpp index d076909bb..b49e3d7d4 100644 --- a/test/unit/svg/svg_parser_test.cpp +++ b/test/unit/svg/svg_parser_test.cpp @@ -44,11 +44,11 @@ namespace // internal mapnik::svg::svg_converter_type svg; mapnik::svg::svg_parser p; - test_parser() + explicit test_parser(bool strict = false) : stl_storage(path.source()) , svg_path(stl_storage) , svg(svg_path, path.attributes()) - , p(svg) + , p(svg, strict) {} mapnik::svg::svg_parser* operator->() @@ -91,8 +91,14 @@ TEST_CASE("SVG parser") { }; test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse(svg_name); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG::parse_from_string syntax error") @@ -108,8 +114,14 @@ TEST_CASE("SVG parser") { std::istreambuf_iterator()); test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG::parse_from_string syntax error") @@ -121,8 +133,14 @@ TEST_CASE("SVG parser") { }; test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + try + { + p->parse(svg_name); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == join(expected_errors)); + } } SECTION("SVG parser color ") @@ -140,9 +158,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse_from_string(svg_str); + REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + } + { + test_parser p(true); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == std::string(expected_errors[0])); + } + } } SECTION("SVG - cope with erroneous geometries") @@ -170,9 +201,24 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse_from_string(svg_str); + REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + } + + { + // strict + test_parser p(true); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == std::string(expected_errors[0])); + } + } } SECTION("SVG parser double % ") @@ -188,9 +234,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse_from_string(svg_str); + REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + } + { + test_parser p(true); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == std::string(expected_errors[0])); + } + } } SECTION("SVG parser display=none") @@ -327,7 +386,7 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); test_parser p; - REQUIRE(p->parse_from_string(svg_str)); + p->parse_from_string(svg_str); auto width = p.svg.width(); auto height = p.svg.height(); REQUIRE(width == 100); @@ -610,10 +669,22 @@ TEST_CASE("SVG parser") { "Failed to find gradient fill: MyGradient", "Failed to find gradient stroke: MyGradient", }; - - test_parser p; - REQUIRE(!p->parse(svg_name)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse(svg_name); + REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + } + { + test_parser p(true); + try + { + p->parse(svg_name); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == std::string(expected_errors[0])); + } + } } SECTION("SVG missing id") @@ -629,9 +700,22 @@ TEST_CASE("SVG parser") { std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - test_parser p; - REQUIRE(!p->parse_from_string(svg_str)); - REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + { + test_parser p; + p->parse_from_string(svg_str); + REQUIRE(join(p->err_handler().error_messages()) == join(expected_errors)); + } + { + test_parser p(true); + try + { + p->parse_from_string(svg_str); + } + catch (std::exception const& ex) + { + REQUIRE(ex.what() == std::string(expected_errors[0])); + } + } } SECTION("SVG missing inheritance")