From 770d93b7cc1178d705a23997a087e884fc74374b Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Tue, 30 Jun 2015 22:23:44 -0500 Subject: [PATCH] Fix situation where offset_converter might start off with an SEG_END from the vertex, therefore starting invalid processing. Tests added to confirm fix. Ref #2937 --- include/mapnik/offset_converter.hpp | 5 ++ test/unit/vertex_adapter/offset_converter.cpp | 75 ++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/include/mapnik/offset_converter.hpp b/include/mapnik/offset_converter.hpp index b1f2476c8..2badce8b5 100644 --- a/include/mapnik/offset_converter.hpp +++ b/include/mapnik/offset_converter.hpp @@ -310,6 +310,11 @@ private: v1.cmd = v0.cmd; // PUSH INITIAL points.push_back(vertex2d(v0.x, v0.y, v0.cmd)); + if (v0.cmd == SEG_END) // not enough vertices in source + { + return status_ = process; + } + while ((v0.cmd = geom_.vertex(&v0.x, &v0.y)) != SEG_END) { points.push_back(vertex2d(v0.x, v0.y, v0.cmd)); diff --git a/test/unit/vertex_adapter/offset_converter.cpp b/test/unit/vertex_adapter/offset_converter.cpp index 38c91b5dc..b4b7d73c7 100644 --- a/test/unit/vertex_adapter/offset_converter.cpp +++ b/test/unit/vertex_adapter/offset_converter.cpp @@ -27,14 +27,22 @@ struct fake_path : fake_path(l.begin(), l.size()) { } - fake_path(std::vector const &v) - : fake_path(v.begin(), v.size()) { + fake_path(std::vector const &v, bool make_invalid = false) + : fake_path(v.begin(), v.size(), make_invalid) { } template - fake_path(Itr itr, size_t sz) { + fake_path(Itr itr, size_t sz, bool make_invalid = false) { size_t num_coords = sz >> 1; + if (make_invalid) + { + num_coords++; + } vertices_.reserve(num_coords); + if (make_invalid) + { + vertices_.push_back(std::make_tuple(0,0,mapnik::SEG_END)); + } for (size_t i = 0; i < num_coords; ++i) { double x = *itr++; @@ -70,6 +78,32 @@ double dist(double x0, double y0, double x1, double y1) return std::sqrt(dx*dx + dy*dy); } +void test_null_segment(double const &offset) +{ + fake_path path = {}; + mapnik::offset_converter off_path_new(path); + off_path_new.set_offset(offset); + double x0 = 0; + double y0 = 0; + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); +} + +void test_invalid_segment(double const &offset) +{ + std::vector v_path = {1, 1, 1, 2}; + fake_path path(v_path, true); + mapnik::offset_converter off_path_new(path); + off_path_new.set_offset(offset); + double x0 = 0; + double y0 = 0; + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); + REQUIRE(off_path_new.vertex(&x0, &y0) == mapnik::SEG_END); +} + + void test_simple_segment(double const &offset) { fake_path path = {0, 0, 1, 0}, off_path = {0, offset, 1, offset}; @@ -263,6 +297,41 @@ void test_s_shaped_curve(double const &offset) { TEST_CASE("offset converter") { +SECTION("null segment") { + try { + + std::vector offsets = { 1, -1 }; + for (double offset : offsets) { + // test simple straight line segment - should be easy to + // find the correspondance here. + offset_test::test_null_segment(offset); + } + } + catch (std::exception const& ex) + { + std::cerr << ex.what() << "\n"; + REQUIRE(false); + } +} + +SECTION("invalid segment") { + try { + + std::vector offsets = { 1, -1 }; + for (double offset : offsets) { + // test simple straight line segment - should be easy to + // find the correspondance here. + offset_test::test_invalid_segment(offset); + } + } + catch (std::exception const& ex) + { + std::cerr << ex.what() << "\n"; + REQUIRE(false); + } +} + + SECTION("simple segment") { try {