Merge pull request #2898 from mapnik/offset

Offset Fix

benchmark/build.py

@@ -43,6 +43,7 @@ benchmarks = [
     "test_font_registration.cpp",
     "test_rendering.cpp",
     "test_rendering_shared_map.cpp",
+    "test_offset_converter.cpp",
 #    "test_numeric_cast_vs_static_cast.cpp",
 ]
 for cpp_test in benchmarks:

benchmark/run

@@ -18,9 +18,10 @@ function run {
 #run test_polygon_clipping 10 1000
 #run test_polygon_clipping_rendering 10 100
 run test_proj_transform1 10 100
-run test_expression_parse 10 10000
+run test_expression_parse 10 1000
-run test_face_ptr_creation 10 10000
+run test_face_ptr_creation 10 1000
-run test_font_registration 10 1000
+run test_font_registration 10 100
+run test_offset_converter 10 1000
 
 ./benchmark/out/test_rendering \
   --name "text rendering" \

benchmark/test_offset_converter.cpp

@@ -0,0 +1,101 @@
+#include "bench_framework.hpp"
+
+// mapnik
+#include <mapnik/global.hpp>
+#include <mapnik/coord.hpp>
+#include <mapnik/vertex.hpp>
+#include <mapnik/offset_converter.hpp>
+
+struct fake_path
+{
+    using coord_type = std::tuple<double, double, unsigned>;
+    using cont_type = std::vector<coord_type>;
+    cont_type vertices_;
+    cont_type::iterator itr_;
+
+    fake_path(std::initializer_list<double> l)
+        : fake_path(l.begin(), l.size()) {
+    }
+
+    fake_path(std::vector<double> const &v)
+        : fake_path(v.begin(), v.size()) {
+    }
+
+    template <typename Itr>
+    fake_path(Itr itr, size_t sz) {
+        size_t num_coords = sz >> 1;
+        vertices_.reserve(num_coords);
+
+        for (size_t i = 0; i < num_coords; ++i) {
+            double x = *itr++;
+            double y = *itr++;
+            unsigned cmd = (i == 0) ? mapnik::SEG_MOVETO : mapnik::SEG_LINETO;
+            vertices_.push_back(std::make_tuple(x, y, cmd));
+            if (i == num_coords - 1) cmd = mapnik::SEG_END;
+            vertices_.push_back(std::make_tuple(x, y, cmd));
+        }
+        itr_ = vertices_.begin();
+    }
+
+    unsigned vertex(double *x, double *y) {
+        if (itr_ == vertices_.end()) {
+            return mapnik::SEG_END;
+        }
+        *x = std::get<0>(*itr_);
+        *y = std::get<1>(*itr_);
+        unsigned cmd = std::get<2>(*itr_);
+        ++itr_;
+        return cmd;
+    }
+
+    void rewind(unsigned) {
+        itr_ = vertices_.begin();
+    }
+};
+
+class test_offset : public benchmark::test_case
+{
+public:
+    test_offset(mapnik::parameters const& params)
+     : test_case(params) {}
+    bool validate() const
+    {
+        return true;
+    }
+    bool operator()() const
+    {
+        std::vector<double> path;
+        int mysize = 2500;
+        int x1 = 0;
+        path.reserve(mysize*2);
+        for( int i = 0; i < mysize; i++ )
+        {
+            path.push_back( i );
+            path.push_back( 0 );
+        }
+        fake_path fpath(path);
+        for (std::size_t i=0;i<iterations_;++i) {
+            mapnik::offset_converter<fake_path> off_path(fpath);
+            off_path.set_offset(10);
+            unsigned cmd;
+            double x, y;
+            while ((cmd = off_path.vertex(&x, &y)) != mapnik::SEG_END)
+            {
+                x1++;
+            }
+        }
+        return x1 > 0;
+    }
+};
+
+
+int main(int argc, char** argv)
+{
+    mapnik::parameters params;
+    benchmark::handle_args(argc,argv,params);
+    {
+        test_offset test_runner(params);
+        run(test_runner,"offset_test");
+    }
+    return 0;
+}

include/mapnik/offset_converter.hpp

@@ -29,6 +29,7 @@
 #include <mapnik/global.hpp>
 #include <mapnik/config.hpp>
 #include <mapnik/vertex.hpp>
+#include <mapnik/vertex_cache.hpp>
 
 // stl
 #include <cmath>
@@ -46,7 +47,7 @@ struct MAPNIK_DECL offset_converter
     offset_converter(Geometry & geom)
         : geom_(geom)
         , offset_(0.0)
-        , threshold_(8.0)
+        , threshold_(5.0)
         , half_turn_segments_(16)
         , status_(initial)
         , pre_first_(vertex2d::no_init)
@@ -237,8 +238,8 @@ private:
      */
     static void displace(vertex2d & v, double dx, double dy, double a)
     {
-        v.x += dx * std::cos(a) - dy * std::sin(a);
+        v.x += dx * std::cos(a) + dy * std::sin(a);
-        v.y += dx * std::sin(a) + dy * std::cos(a);
+        v.y += dx * std::sin(a) - dy * std::cos(a);
     }
 
     /**
@@ -275,13 +276,41 @@ private:
         {
             return status_;
         }
-
+        vertex2d v0(vertex2d::no_init);
         vertex2d v1(vertex2d::no_init);
         vertex2d v2(vertex2d::no_init);
         vertex2d w(vertex2d::no_init);
-
+        vertex2d start_v2(vertex2d::no_init);
-        v1.cmd = geom_.vertex(&v1.x, &v1.y);
+        std::vector<vertex2d> points;
-        v2.cmd = geom_.vertex(&v2.x, &v2.y);
+        std::vector<vertex2d> close_points;
+        bool is_polygon = false;
+        int cpt = 0;
+        v0.cmd = geom_.vertex(&v0.x, &v0.y);
+        v1.x = v0.x;
+        v1.y = v0.y;
+        v1.cmd = v0.cmd;
+        // PUSH INITIAL
+        points.push_back(vertex2d(v0.x, v0.y, v0.cmd));
+        while ((v0.cmd = geom_.vertex(&v0.x, &v0.y)) != SEG_END)
+        {
+            points.push_back(vertex2d(v0.x, v0.y, v0.cmd));
+            if (v0.cmd == SEG_CLOSE)
+            {
+                is_polygon = true;
+                close_points.push_back(vertex2d(v1.x, v1.y, v1.cmd));
+            }
+            v1.x = v0.x;
+            v1.y = v0.y;
+            v1.cmd = v0.cmd;
+        }
+        // Push SEG_END
+        points.push_back(vertex2d(v0.x, v0.y, v0.cmd));
+        std::size_t i = 0;
+        v1 = points[i++];
+        v2 = points[i++];
+        v0.cmd = v1.cmd;
+        v0.x = v1.x;
+        v0.y = v1.y;
 
         if (v2.cmd == SEG_END) // not enough vertices in source
         {
@@ -289,25 +318,119 @@ private:
         }
 
         double angle_a = 0;
+        if (is_polygon) 
+        {
+            double x = v1.x - close_points[cpt].x;
+            double y = v1.y - close_points[cpt].y;
+            cpt++;
+            x = std::abs(x) < std::numeric_limits<double>::epsilon() ? 0 : x; 
+            y = std::abs(y) < std::numeric_limits<double>::epsilon() ? 0 : y; 
+            angle_a = std::atan2(y, x);
+        }
         double angle_b = std::atan2((v2.y - v1.y), (v2.x - v1.x));
         double joint_angle;
 
-        // first vertex
+        if (!is_polygon)
-        displace(v1, angle_b);
+        {
-        push_vertex(v1);
+            // first vertex
+            displace(v1, angle_b);
+            push_vertex(v1);
+        }
+        else
+        {
+            joint_angle = explement_reflex_angle(angle_b - angle_a);
+
+            double half_turns = half_turn_segments_ * std::fabs(joint_angle);
+            int bulge_steps = 0;
+
+            if (offset_ < 0.0)
+            {
+                if (joint_angle > 0.0)
+                {
+                    joint_angle = joint_angle - 2 * M_PI;
+                }
+                else
+                {
+                    bulge_steps = 1 + static_cast<int>(std::floor(half_turns / M_PI));
+                }
+            }
+            else
+            {
+                if (joint_angle < 0.0)
+                {
+                    joint_angle = joint_angle + 2 * M_PI;
+                }
+                else
+                {
+                    bulge_steps = 1 + static_cast<int>(std::floor(half_turns / M_PI));
+                }
+            }
+            if (bulge_steps == 0)
+            {
+                displace2(v1, angle_a, angle_b);
+                push_vertex(v1);
+            }
+            else
+            {
+                displace(v1, angle_b);
+                push_vertex(v1);
+            }
+        }
 
         // Sometimes when the first segment is too short, it causes ugly
         // curls at the beginning of the line. To avoid this, we make up
         // a fake vertex two offset-lengths before the first, and expect
         // intersection detection smoothes it out.
-        pre_first_ = v1;
+        if (!is_polygon)
-        displace(pre_first_, -2 * std::fabs(offset_), 0, angle_b);
-        start_ = pre_first_;
-        while ((v1 = v2, v2.cmd = geom_.vertex(&v2.x, &v2.y)) != SEG_END)
         {
+            pre_first_ = v1;
+            displace(pre_first_, -2 * std::fabs(offset_), 0, angle_b);
+            start_ = pre_first_;
+        }
+        else
+        {
+            pre_first_ = v0;
+            start_ = pre_first_;
+        }
+        start_v2.x = v2.x;
+        start_v2.y = v2.y;
+        bool continue_loop = true;
+        while (i < points.size())
+        {
+            v1 = v2;
+            v2 = points[i++];
+            if (v1.cmd == SEG_MOVETO)
+            {
+                if (is_polygon)
+                {
+                    v1.x = start_.x;
+                    v1.y = start_.y;
+                    if (cpt < close_points.size())
+                    {
+                        double x = v1.x - close_points[cpt].x;
+                        double y = v1.y - close_points[cpt].y;
+                        x = std::abs(x) < std::numeric_limits<double>::epsilon() ? 0.0 : x; 
+                        y = std::abs(y) < std::numeric_limits<double>::epsilon() ? 0.0 : y; 
+                        angle_b = std::atan2(y,x);
+                        cpt++;
+                    }
+                }
+                start_v2.x = v2.x;
+                start_v2.y = v2.y;
+            }
             if (v2.cmd == SEG_MOVETO)
             {
-                start_ = v2;
+                start_.x = v2.x;
+                start_.y = v2.y;
+                v2.x = start_v2.x;
+                v2.y = start_v2.y;
+            }
+            else if (v2.cmd == SEG_END)
+            {
+                if (!is_polygon) break;
+                continue_loop = false;
+                v2.x = start_v2.x;
+                v2.y = start_v2.y;
             }
             else if (v2.cmd == SEG_CLOSE)
             {
@@ -360,24 +483,50 @@ private:
                     << " degrees ((< with " << bulge_steps << " segments";
             }
             #endif
-
+            if (v1.cmd == SEG_MOVETO)
-            displace(w, v1, angle_a);
-            push_vertex(w);
-
-            for (int s = 0; ++s < bulge_steps;)
             {
-                displace(w, v1, angle_a + (joint_angle * s) / bulge_steps);
+                if (bulge_steps == 0)
-                push_vertex(w);
+                {
+                    displace2(v1, angle_a, angle_b);
+                    push_vertex(v1);
+                }
+                else
+                {
+                    displace(v1, angle_b);
+                    push_vertex(v1);
+                }
             }
+            else
+            {
+                if (bulge_steps == 0)
+                {
+                    displace2(v1, angle_a, angle_b);
+                    push_vertex(v1);
+                }
+                else
+                {
+                    displace(w, v1, angle_a);
+                    w.cmd = SEG_LINETO;
+                    push_vertex(w);
 
-            displace(v1, angle_b);
+                    for (int s = 0; ++s < bulge_steps;)
-            push_vertex(v1);
+                    {
+                        displace(w, v1, angle_a + (joint_angle * s) / bulge_steps);
+                        w.cmd = SEG_LINETO;
+                        push_vertex(w);
+                    }
+                    displace(v1, angle_b);
+                    push_vertex(v1);
+                }
+            }
         }
 
         // last vertex
-        displace(v1, angle_b);
+        if (!is_polygon)
-        push_vertex(v1);
+        {
-
+            displace(v1, angle_b);
+            push_vertex(v1);
+        }
         // initialization finished
         return status_ = process;
     }

test/data-visual

@@ -1 +1 @@
-Subproject commit 5489590d3aeaba749aec8b38cbfa9cf6318209a6
+Subproject commit cdb6afca7b517c1fdda824fcee4abb60ed525331

test/unit/vertex_adapter/offset_converter.cpp

@@ -0,0 +1,329 @@
+#include "catch.hpp"
+
+// mapnik
+#include <mapnik/global.hpp>
+#include <mapnik/coord.hpp>
+#include <mapnik/vertex.hpp>
+#include <mapnik/offset_converter.hpp>
+
+// stl
+#include <stdexcept>
+#include <iostream>
+#include <fstream>
+#include <vector>
+#include <tuple>
+#include <algorithm>
+
+namespace offset_test {
+
+struct fake_path
+{
+    using coord_type = std::tuple<double, double, unsigned>;
+    using cont_type = std::vector<coord_type>;
+    cont_type vertices_;
+    cont_type::iterator itr_;
+
+    fake_path(std::initializer_list<double> l)
+        : fake_path(l.begin(), l.size()) {
+    }
+
+    fake_path(std::vector<double> const &v)
+        : fake_path(v.begin(), v.size()) {
+    }
+
+    template <typename Itr>
+    fake_path(Itr itr, size_t sz) {
+        size_t num_coords = sz >> 1;
+        vertices_.reserve(num_coords);
+
+        for (size_t i = 0; i < num_coords; ++i) {
+            double x = *itr++;
+            double y = *itr++;
+            unsigned cmd = (i == 0) ? mapnik::SEG_MOVETO : mapnik::SEG_LINETO;
+            vertices_.push_back(std::make_tuple(x, y, cmd));
+        }
+        itr_ = vertices_.begin();
+    }
+
+    unsigned vertex(double *x, double *y) {
+        if (itr_ == vertices_.end()) {
+            return mapnik::SEG_END;
+        }
+        *x = std::get<0>(*itr_);
+        *y = std::get<1>(*itr_);
+        unsigned cmd = std::get<2>(*itr_);
+        ++itr_;
+        return cmd;
+    }
+
+    void rewind(unsigned) {
+        itr_ = vertices_.begin();
+    }
+};
+
+static double DELTA_BUFF = 0.5;
+
+double dist(double x0, double y0, double x1, double y1)
+{
+    double dx = x0 - x1;
+    double dy = y0 - y1;
+    return std::sqrt(dx*dx + dy*dy);
+}
+
+void test_simple_segment(double const &offset)
+{
+    fake_path path = {0, 0, 1, 0}, off_path = {0, offset, 1, offset};
+    mapnik::offset_converter<fake_path> off_path_new(path);
+    off_path_new.set_offset(-offset);
+
+    double x0, y0, x1, y1;
+    unsigned cmd0 = off_path_new.vertex(&x0, &y0);
+    unsigned cmd1 = off_path.vertex(&x1,&y1);
+    double d = dist(x0, y0, x1, y1);
+    while (true) {
+        if (d > (std::abs(offset) + DELTA_BUFF))
+        {
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            REQUIRE(cmd0 != mapnik::SEG_END);
+            d = dist(x0, y0, x1, y1);
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+        }
+        else
+        {
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+            
+        }
+        
+        cmd1 = off_path.vertex(&x1,&y1);
+        if (cmd1 == mapnik::SEG_END) break;
+        d = dist(x0, y0, x1, y1);
+        bool done = false;
+        while (d <= (std::abs(offset) + DELTA_BUFF))
+        {
+            CHECK(true);
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            if (cmd0 == mapnik::SEG_END)
+            {
+                done = true;
+                break;
+            }
+        }
+        if (done) break;
+    }
+}
+
+void test_straight_line(double const &offset) {
+    fake_path path = {0, 0, 1, 0, 9, 0, 10, 0},
+        off_path = {0, offset, 1, offset, 9, offset, 10, offset};
+    mapnik::offset_converter<fake_path> off_path_new(path);
+    off_path_new.set_offset(-offset);
+
+    double x0, y0, x1, y1;
+    unsigned cmd0 = off_path_new.vertex(&x0, &y0);
+    unsigned cmd1 = off_path.vertex(&x1,&y1);
+    double d = dist(x0, y0, x1, y1);
+    while (true) {
+        if (d > (std::abs(offset) + DELTA_BUFF))
+        {
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            REQUIRE(cmd0 != mapnik::SEG_END);
+            d = dist(x0, y0, x1, y1);
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+        }
+        else
+        {
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+            
+        }
+        
+        cmd1 = off_path.vertex(&x1,&y1);
+        d = dist(x0, y0, x1, y1);
+        bool done = false;
+        while (d <= (std::abs(offset) + DELTA_BUFF))
+        {
+            CHECK(true);
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            if (cmd0 == mapnik::SEG_END)
+            {
+                done = true;
+                break;
+            }
+        }
+        if (done) break;
+    }
+}
+
+void test_offset_curve(double const &offset) {
+    const double r = (1.0 + offset);
+
+    std::vector<double> pos, off_pos;
+    const size_t max_i = 1000;
+    for (size_t i = 0; i <= max_i; ++i) {
+        double x = M_PI * double(i) / max_i;
+        pos.push_back(-std::cos(x)); pos.push_back(std::sin(x));
+        off_pos.push_back(-r * std::cos(x)); off_pos.push_back(r * std::sin(x));
+    }
+
+    fake_path path(pos), off_path(off_pos);
+    mapnik::offset_converter<fake_path> off_path_new(path);
+    off_path_new.set_offset(-offset);
+
+    double x0, y0, x1, y1;
+    unsigned cmd0 = off_path_new.vertex(&x0, &y0);
+    unsigned cmd1 = off_path.vertex(&x1,&y1);
+    double d = dist(x0, y0, x1, y1);
+    while (true) {
+        if (d > (std::abs(offset) + DELTA_BUFF))
+        {
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            REQUIRE(cmd0 != mapnik::SEG_END);
+            d = dist(x0, y0, x1, y1);
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+        }
+        else
+        {
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+            
+        }
+        
+        cmd1 = off_path.vertex(&x1,&y1);
+        d = dist(x0, y0, x1, y1);
+        bool done = false;
+        while (d <= (std::abs(offset) + DELTA_BUFF))
+        {
+            CHECK(true);
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            if (cmd0 == mapnik::SEG_END)
+            {
+                done = true;
+                break;
+            }
+        }
+        if (done) break;
+    }
+}
+
+void test_s_shaped_curve(double const &offset) {
+    const double r = (1.0 + offset);
+    const double r2 = (1.0 - offset);
+
+    std::vector<double> pos, off_pos;
+    const size_t max_i = 1000;
+    for (size_t i = 0; i <= max_i; ++i) {
+        double x = M_PI * double(i) / max_i;
+        pos.push_back(-std::cos(x) - 1); pos.push_back(std::sin(x));
+        off_pos.push_back(-r * std::cos(x) - 1); off_pos.push_back(r * std::sin(x));
+    }
+    for (size_t i = 0; i <= max_i; ++i) {
+        double x = M_PI * double(i) / max_i;
+        pos.push_back(-std::cos(x) + 1); pos.push_back(-std::sin(x));
+        off_pos.push_back(-r2 * std::cos(x) + 1); off_pos.push_back(-r2 * std::sin(x));
+    }
+
+    fake_path path(pos), off_path(off_pos);
+    mapnik::offset_converter<fake_path> off_path_new(path);
+    off_path_new.set_offset(-offset);
+
+    double x0, y0, x1, y1;
+    unsigned cmd0 = off_path_new.vertex(&x0, &y0);
+    unsigned cmd1 = off_path.vertex(&x1,&y1);
+    double d = dist(x0, y0, x1, y1);
+    while (true) {
+        if (d > (std::abs(offset) + DELTA_BUFF))
+        {
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            REQUIRE(cmd0 != mapnik::SEG_END);
+            d = dist(x0, y0, x1, y1);
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+        }
+        else
+        {
+            REQUIRE(d <= (std::abs(offset) + DELTA_BUFF));
+            
+        }
+        
+        cmd1 = off_path.vertex(&x1,&y1);
+        d = dist(x0, y0, x1, y1);
+        bool done = false;
+        while (d <= (std::abs(offset) + DELTA_BUFF))
+        {
+            CHECK(true);
+            cmd0 = off_path_new.vertex(&x0,&y0);
+            if (cmd0 == mapnik::SEG_END)
+            {
+                done = true;
+                break;
+            }
+        }
+        if (done) break;
+    }
+}
+
+} // END NS
+
+TEST_CASE("offset converter") {
+
+SECTION("simple segment") {
+    try {
+
+        std::vector<double> offsets = { 1, -1 };
+        for (double offset : offsets) {
+            // test simple straight line segment - should be easy to
+            // find the correspondance here.
+            offset_test::test_simple_segment(offset);
+        }
+    }
+    catch (std::exception const& ex)
+    {
+        std::cerr << ex.what() << "\n";
+        REQUIRE(false);
+    }
+}
+
+SECTION("straight line") {
+    try {
+
+        std::vector<double> offsets = { 1, -1 };
+        for (double offset : offsets) {
+            // test straight line consisting of more than one segment.
+            offset_test::test_straight_line(offset);
+        }
+    }
+    catch (std::exception const& ex)
+    {
+        std::cerr << ex.what() << "\n";
+        REQUIRE(false);
+    }
+}
+
+SECTION("curve") {
+    try {
+
+        std::vector<double> offsets = { 1, -1 };
+        for (double offset : offsets) {
+            offset_test::test_offset_curve(offset);
+        }
+    }
+    catch (std::exception const& ex)
+    {
+        std::cerr << ex.what() << "\n";
+        REQUIRE(false);
+    }
+}
+
+SECTION("s curve") {
+    try {
+
+        std::vector<double> offsets = { 1, -1 };
+        for (double offset : offsets) {
+            offset_test::test_s_shaped_curve(offset);
+        }
+    }
+    catch (std::exception const& ex)
+    {
+        std::cerr << ex.what() << "\n";
+        REQUIRE(false);
+    }
+}
+
+}