From 5e30aee4e76b72b20a40ff71c73da9c19e1bd03a Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Fri, 22 Jan 2016 11:02:12 -0600 Subject: [PATCH 1/6] Added missing required header from unit test --- test/unit/imaging/image_io_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/imaging/image_io_test.cpp b/test/unit/imaging/image_io_test.cpp index 47966f42c..f2463f810 100644 --- a/test/unit/imaging/image_io_test.cpp +++ b/test/unit/imaging/image_io_test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include From 72297c943186914db8221c74ec2b33a5123a1368 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sat, 23 Jan 2016 11:06:45 +0000 Subject: [PATCH 2/6] Adapt geometry tests got changes in boost 1.60 The issues with points at NaN and infinity bring considered as valid reported in https://svn.boost.org/trac/boost/ticket/11711 has been fixed in boost 1.60 so the tests need to reflect that. Also per https://svn.boost.org/trac/boost/ticket/11710 empty polygons are now considered invalid, and hence not simple. --- test/unit/geometry/geometry_is_simple.cpp | 32 +++++++++++++++++++++++ test/unit/geometry/geometry_is_valid.cpp | 32 +++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/test/unit/geometry/geometry_is_simple.cpp b/test/unit/geometry/geometry_is_simple.cpp index dbed04a1e..1170350b9 100644 --- a/test/unit/geometry/geometry_is_simple.cpp +++ b/test/unit/geometry/geometry_is_simple.cpp @@ -163,6 +163,36 @@ SECTION("polygon 3 repeated points") { CHECK( !mapnik::geometry::is_simple(poly) ); } +#if BOOST_VERSION >= 106000 + +SECTION("polygon that is empty") { + mapnik::geometry::polygon poly; + CHECK( !mapnik::geometry::is_simple(poly) ); +} + +SECTION("polygon that has empty exterior ring") { + mapnik::geometry::polygon poly; + mapnik::geometry::linear_ring ring; + poly.set_exterior_ring(std::move(ring)); + CHECK( !mapnik::geometry::is_simple(poly) ); +} + +SECTION("polygon that has empty interior ring") { + mapnik::geometry::polygon poly; + mapnik::geometry::linear_ring ring; + ring.add_coord(0,0); + ring.add_coord(1,0); + ring.add_coord(1,1); + ring.add_coord(0,1); + ring.add_coord(0,0); + poly.set_exterior_ring(std::move(ring)); + mapnik::geometry::linear_ring ring2; + poly.add_hole(std::move(ring2)); + CHECK( !mapnik::geometry::is_simple(poly) ); +} + +#else // BOOST_VERSION >= 1.60 + SECTION("polygon that is empty") { mapnik::geometry::polygon poly; CHECK( mapnik::geometry::is_simple(poly) ); @@ -189,6 +219,8 @@ SECTION("polygon that has empty interior ring") { CHECK( mapnik::geometry::is_simple(poly) ); } +#endif // BOOST_VERSION >= 1.60 + // A polygon with a spike can still be simple SECTION("polygon with spike") { mapnik::geometry::polygon poly; diff --git a/test/unit/geometry/geometry_is_valid.cpp b/test/unit/geometry/geometry_is_valid.cpp index 6d8b922dd..6d8fcf993 100644 --- a/test/unit/geometry/geometry_is_valid.cpp +++ b/test/unit/geometry/geometry_is_valid.cpp @@ -54,6 +54,36 @@ SECTION("point unitialized") { CHECK( failure2 == boost::geometry::no_failure ); } +#if BOOST_VERSION >= 106000 + +SECTION("point NaN") { + mapnik::geometry::point pt(std::numeric_limits::quiet_NaN(),std::numeric_limits::quiet_NaN()); + CHECK( std::isnan(pt.x) ); + CHECK( std::isnan(pt.y) ); + CHECK( !mapnik::geometry::is_valid(pt) ); + std::string message; + CHECK( !mapnik::geometry::is_valid(pt, message) ); + CHECK( message == "Geometry has point(s) with invalid coordinate(s)"); + boost::geometry::validity_failure_type failure; + CHECK( !mapnik::geometry::is_valid(pt, failure) ); + CHECK( failure == boost::geometry::failure_invalid_coordinate ); +} + +SECTION("point Infinity") { + mapnik::geometry::point pt(std::numeric_limits::infinity(),std::numeric_limits::infinity()); + CHECK( std::isinf(pt.x) ); + CHECK( std::isinf(pt.y) ); + CHECK( !mapnik::geometry::is_valid(pt) ); + std::string message; + CHECK( !mapnik::geometry::is_valid(pt, message) ); + CHECK( message == "Geometry has point(s) with invalid coordinate(s)"); + boost::geometry::validity_failure_type failure; + CHECK( !mapnik::geometry::is_valid(pt, failure) ); + CHECK( failure == boost::geometry::failure_invalid_coordinate ); +} + +#else // BOOST_VERSION >= 1.60 + // This is funky that boost geometry is_valid does not check for NAN when dealing with a point // this test is here in case the logic ever changes // Bug report on this: https://svn.boost.org/trac/boost/ticket/11711 @@ -86,6 +116,8 @@ SECTION("point Infinity") { CHECK( failure == boost::geometry::no_failure ); } +#endif // BOOST_VERSION >= 1.60 + SECTION("multi point") { mapnik::geometry::multi_point mpt; mpt.add_coord(0,0); From 05b66fc35587f63889d1f11009f57632d05f678f Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 23 Jan 2016 20:21:05 +0100 Subject: [PATCH 3/6] refactor svg_parser_test - moved some boilerplate to helper struct test_parser - added REQUIRE(!parse...) to parsing error tests - changed parsing error tests to compare full error lists instead of just count and then individual messages (if count was different, you were left in the dark with no messages at all) - changed some double-quotes in errors to single-quotes (corresponding change to parser follows) --- test/data | 2 +- test/unit/svg/svg_parser_test.cpp | 246 +++++++++++++++--------------- 2 files changed, 120 insertions(+), 128 deletions(-) diff --git a/test/data b/test/data index 24844dead..e4178f3ce 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 24844dead6d23fd541aba147b69703a605fc7351 +Subproject commit e4178f3ce9798f93024a49d6cd0a87d98ec0f6b0 diff --git a/test/unit/svg/svg_parser_test.cpp b/test/unit/svg/svg_parser_test.cpp index e0bfd9ecc..3fee6a452 100644 --- a/test/unit/svg/svg_parser_test.cpp +++ b/test/unit/svg/svg_parser_test.cpp @@ -34,6 +34,42 @@ #include #include +namespace // internal +{ + struct test_parser + { + mapnik::svg_storage_type path; + mapnik::svg::vertex_stl_adapter stl_storage; + mapnik::svg::svg_path_adapter svg_path; + mapnik::svg::svg_converter_type svg; + mapnik::svg::svg_parser p; + + test_parser() + : stl_storage(path.source()) + , svg_path(stl_storage) + , svg(svg_path, path.attributes()) + , p(svg) + {} + + mapnik::svg::svg_parser* operator->() + { + return &p; + } + }; + + template + std::string join(C const& container) + { + std::string result; + for (auto const& str : container) + { + if (!result.empty()) result += "\n "; + result += str; + } + return result; + } +} + TEST_CASE("SVG parser") { SECTION("SVG i/o") @@ -49,142 +85,112 @@ TEST_CASE("SVG parser") { SECTION("SVG::parse i/o") { std::string svg_name("FAIL"); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse(svg_name)) + char const* expected_errors[] = { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Unable to open 'FAIL'"); - } + "Unable to open 'FAIL'" + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG::parse_from_string syntax error") { std::string svg_name("./test/data/svg/invalid.svg"); + char const* expected_errors[] = + { + "Unable to parse '\n\n'" + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Unable to parse '\n\n'"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG::parse_from_string syntax error") { std::string svg_name("./test/data/svg/invalid.svg"); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse(svg_name)) + char const* expected_errors[] = { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "svg_parser::parse - Unable to parse './test/data/svg/invalid.svg'"); - } + "svg_parser::parse - Unable to parse './test/data/svg/invalid.svg'" + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser color ") { std::string svg_name("./test/data/svg/color_fail.svg"); + char const* expected_errors[] = + { + "Failed to parse color: \"fail\"", + "Failed to parse SVG value: 'fail'", + "Failed to parse color: \"fail\"", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 3); - REQUIRE(errors[0] == "Failed to parse color: \"fail\""); - REQUIRE(errors[1] == "Failed to parse SVG value: \"fail\""); - REQUIRE(errors[2] == "Failed to parse color: \"fail\""); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG - cope with erroneous geometries") { std::string svg_name("./test/data/svg/errors.svg"); + char const* expected_errors[] = + { + "parse_rect: Invalid width", + "Failed to parse SVG value: 'FAIL'", + "parse_rect: Invalid height", + "parse_rect: Invalid rx", + "parse_rect: Invalid ry", + "Failed to parse SVG value: '100invalidunit', trailing garbage: 'validunit'", + "unable to parse invalid svg ", + "unable to parse invalid svg with id 'fail-path'", + "unable to parse invalid svg with id 'fail-path'", + "parse_circle: Invalid radius", + "Failed to parse 'points'", + "Failed to parse 'points'", + "parse_ellipse: Invalid rx", + "parse_ellipse: Invalid ry", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 13); - REQUIRE(errors[0] == "parse_rect: Invalid width"); - REQUIRE(errors[1] == "Failed to parse SVG value: \"FAIL\""); - REQUIRE(errors[2] == "parse_rect: Invalid height"); - REQUIRE(errors[3] == "parse_rect: Invalid rx"); - REQUIRE(errors[4] == "parse_rect: Invalid ry"); - REQUIRE(errors[5] == "unable to parse invalid svg "); - REQUIRE(errors[6] == "unable to parse invalid svg with id 'fail-path'"); - REQUIRE(errors[7] == "unable to parse invalid svg with id 'fail-path'"); - REQUIRE(errors[8] == "parse_circle: Invalid radius"); - REQUIRE(errors[9] == "Failed to parse 'points'"); - REQUIRE(errors[10] == "Failed to parse 'points'"); - REQUIRE(errors[11] == "parse_ellipse: Invalid rx"); - REQUIRE(errors[12] == "parse_ellipse: Invalid ry"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser double % ") { std::string svg_name("./test/data/svg/gradient-radial-error.svg"); + char const* expected_errors[] = + { + "Failed to parse SVG value: 'FAIL'" + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 1); - REQUIRE(errors[0] == "Failed to parse SVG value: \"FAIL\""); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG parser display=none") @@ -320,16 +326,10 @@ TEST_CASE("SVG parser") { std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - p.parse_from_string(svg_str); - auto width = svg.width(); - auto height = svg.height(); + test_parser p; + REQUIRE(p->parse_from_string(svg_str)); + auto width = p.svg.width(); + auto height = p.svg.height(); REQUIRE(width == 100); REQUIRE(height == 100); } @@ -604,41 +604,33 @@ TEST_CASE("SVG parser") { SECTION("SVG missing def") { std::string svg_name("./test/data/svg/gradient-nodef.svg"); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - REQUIRE(!p.parse(svg_name)); - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 2); - REQUIRE(errors[0] == "Failed to find gradient fill: MyGradient"); - REQUIRE(errors[1] == "Failed to find gradient stroke: MyGradient"); + char const* expected_errors[] = + { + "Failed to find gradient fill: MyGradient", + "Failed to find gradient stroke: MyGradient", + }; + + test_parser p; + REQUIRE(!p->parse(svg_name)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG missing id") { - std::string svg_name("./test/data/svg/gradient-no-id.svg"); + char const* expected_errors[] = + { + "Failed to find gradient fill: MyGradient", + "Failed to find gradient stroke: MyGradient", + }; + std::ifstream in(svg_name.c_str()); std::string svg_str((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - using namespace mapnik::svg; - mapnik::svg_storage_type path; - vertex_stl_adapter stl_storage(path.source()); - svg_path_adapter svg_path(stl_storage); - svg_converter_type svg(svg_path, path.attributes()); - svg_parser p(svg); - - if (!p.parse_from_string(svg_str)) - { - auto const& errors = p.error_messages(); - REQUIRE(errors.size() == 2); - REQUIRE(errors[0] == "Failed to find gradient fill: MyGradient"); - REQUIRE(errors[1] == "Failed to find gradient stroke: MyGradient"); - } + test_parser p; + REQUIRE(!p->parse_from_string(svg_str)); + REQUIRE(join(p->error_messages()) == join(expected_errors)); } SECTION("SVG missing inheritance") From 8d7fca3236c555b6552e028edbaf13abb2f7b01d Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sat, 23 Jan 2016 20:44:14 +0100 Subject: [PATCH 4/6] svg_parse_value - add 'px' unit, report trailing garbage - avoid operator comma in semantic action, refs #3249 --- src/svg/svg_parser.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index e979e3f7b..6613d5cde 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -137,29 +137,36 @@ double parse_double(T & error_messages, const char* str) template double parse_svg_value(T & error_messages, const char* str, bool & percent) { - using namespace boost::spirit::qi; using skip_type = boost::spirit::ascii::space_type; using boost::phoenix::ref; - double_type double_; - char_type char_; - _1_type _1; + qi::double_type double_; + qi::lit_type lit; + qi::_1_type _1; double val = 0.0; - symbols units; + qi::symbols units; units.add + ("px", 1.0) ("pt", DPI/72.0) ("pc", DPI/6.0) ("mm", DPI/25.4) ("cm", DPI/2.54) ("in", (double)DPI) ; - if (!phrase_parse(str, str + std::strlen(str), - double_[ref(val) = _1, ref(percent) = false] + const char* cur = str; // phrase_parse modifies the first iterator + const char* end = str + std::strlen(str); + if (!qi::phrase_parse(cur, end, + double_[ref(val) = _1][ref(percent) = false] > - (units[ ref(val) *= _1] | - char_('%')[ref(val) *= 0.01, ref(percent) = true]), + lit('%')[ref(val) *= 0.01][ref(percent) = true]), skip_type())) { - error_messages.emplace_back("Failed to parse SVG value: \"" + std::string(str) + "\""); + error_messages.emplace_back("Failed to parse SVG value: '" + std::string(str) + "'"); + } + else if (cur != end) + { + error_messages.emplace_back("Failed to parse SVG value: '" + std::string(str) + + "', trailing garbage: '" + cur + "'"); } return val; } From e9fbe0724f172bb0ac5f794caca9b572dea8431a Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Sun, 24 Jan 2016 18:34:28 +0100 Subject: [PATCH 5/6] travis: fetch required git submodule pull requests --- .travis.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0dcfac089..4be27fe6f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ sudo: false git: depth: 10 - submodules: true + submodules: false env: global: @@ -43,6 +43,15 @@ before_install: - export MASON_PUBLISH=${MASON_PUBLISH:-false} - if [[ ${TRAVIS_BRANCH} != 'master' ]]; then export MASON_PUBLISH=false; fi - if [[ ${TRAVIS_PULL_REQUEST} != 'false' ]]; then export MASON_PUBLISH=false; fi + - git submodule update --init --depth=10 || + git submodule foreach 'test "$sha1" = "`git rev-parse HEAD`" || + git ls-remote origin "refs/pull/*/head" | + while read hash ref; do + if test "$hash" = "$sha1"; then + git config --add remote.origin.fetch "+$ref:$ref"; + fi + done' + - git submodule update --init --depth=10 install: - if [[ $(uname -s) == 'Linux' ]]; then From db9facef9044e21521610b6299a196af2e1305fd Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 25 Jan 2016 10:04:10 +0100 Subject: [PATCH 6/6] update submodules --- deps/mapbox/variant | 2 +- test/data | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deps/mapbox/variant b/deps/mapbox/variant index 272f91c09..9afd64060 160000 --- a/deps/mapbox/variant +++ b/deps/mapbox/variant @@ -1 +1 @@ -Subproject commit 272f91c0962f001021e35811d9bf934fcb2476f4 +Subproject commit 9afd6406065440e02b6bf42db4633b815846db0d diff --git a/test/data b/test/data index e4178f3ce..94bb5e033 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit e4178f3ce9798f93024a49d6cd0a87d98ec0f6b0 +Subproject commit 94bb5e0330c3d920c9aed609058d01116917c90c