Fix a line clipping edge case

Fix the edge case when a line string enters the clip region from outside and has a node exactly on the clip region border. Before, the node on the border was skipped, which caused wrong line metrics estimation.
mapbox · May 11, 2021 · 937a23c · 937a23c
1 parent 1c0e989
commit 937a23c
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 8 deletions.
diff --git a/include/mapbox/geojsonvt/clip.hpp b/include/mapbox/geojsonvt/clip.hpp
@@ -115,6 +115,7 @@ class clipper {
             const auto& b = line[i + 1];
             const double ak = get<I>(a);
             const double bk = get<I>(b);
+            const bool lastSegFlag = (i == (len - 2));
 
             if (lineMetrics) segLen = ::hypot((b.x - a.x), (b.y - a.y));
 
@@ -135,9 +136,10 @@ class clipper {
                     t = calc_progress<I>(a, b, k1);
                     slice.emplace_back(intersect<I>(a, b, k1, t));
                     if (lineMetrics) slice.segStart = lineLen + segLen * t;
-
-                    if (i == len - 2)
-                        slice.emplace_back(b); // last point
+                    if (lastSegFlag) slice.emplace_back(b); // last point
+                } else if (bk == k1 && !lastSegFlag) { // --->|..  |
+                    if (lineMetrics) slice.segStart = lineLen + segLen;
+                    slice.emplace_back(b);
                 }
             } else if (ak > k2) {
                 if (bk < k1) { // <--|-----|---
@@ -156,9 +158,10 @@ class clipper {
                     t = calc_progress<I>(a, b, k2);
                     slice.emplace_back(intersect<I>(a, b, k2, t));
                     if (lineMetrics) slice.segStart = lineLen + segLen * t;
-
-                    if (i == len - 2)
-                        slice.emplace_back(b); // last point
+                    if (lastSegFlag) slice.emplace_back(b); // last point
+                } else if (bk == k2 && !lastSegFlag) { // |  ..|<---
+                    if (lineMetrics) slice.segStart = lineLen + segLen;
+                    slice.emplace_back(b);
                 }
             } else {
                 slice.emplace_back(a);
@@ -177,7 +180,7 @@ class clipper {
                     slices.emplace_back(std::move(slice));
                     slice = newSlice(line);
 
-                } else if (i == len - 2) { // | --> |
+                } else if (lastSegFlag) { // | --> |
                     slice.emplace_back(b);
                 }
             }
@@ -186,7 +189,7 @@ class clipper {
         }
 
         if (!slice.empty()) { // add the final slice
-            slice.segEnd = lineLen;
+            if (lineMetrics) slice.segEnd = lineLen;
             slices.emplace_back(std::move(slice));
         }
     }

diff --git a/test/test.cpp b/test/test.cpp
@@ -475,3 +475,41 @@ TEST(geoJSONToTile, Metrics) {
     double rightClipEnd = (rightProps.find("mapbox_clip_end")->second).get<double>();
     EXPECT_DOUBLE_EQ(rightClipEnd, 1.0);
 }
+
+TEST(GeoJSONVT, ClipVertexOnTileBorder) {
+    std::string data = R"geojson({
+        "type": "Feature",
+        "geometry": {
+            "type": "LineString",
+            "coordinates":[
+                [-77.031373697916663,38.895516493055553],
+                [-77.01416015625,38.887532552083336],
+                [-76.99,38.87]
+            ]
+        }
+    })geojson"; // The second node is exactly on the (13, 2344, 3134) tile border.
+    auto geojson = mapbox::geojson::parse(data);
+    mapbox::geojsonvt::Options options;
+    options.lineMetrics = true;
+    options.buffer = 2048;
+    options.extent = 8192;
+
+    const double kEpsilon = 1e-5;
+
+    mapbox::geojsonvt::GeoJSONVT index{geojson,options};
+
+    const Tile& tile = index.getTile(13, 2344, 3134);
+    ASSERT_FALSE(tile.features.empty());
+    const mapbox::geometry::line_string<int16_t> expected{
+        {-2048, 2747}, {408, 5037}
+    };
+    const auto actual = tile.features[0].geometry.get<mapbox::geometry::line_string<int16_t>>();
+    ASSERT_EQ(actual, expected);
+
+    // Check line metrics
+    auto& props = tile.features[0].properties;
+    double clipStart1 = (props.find("mapbox_clip_start")->second).get<double>();
+    double clipEnd1 = (props.find("mapbox_clip_end")->second).get<double>();
+    EXPECT_NEAR(0.660622, clipStart1, kEpsilon);
+    EXPECT_NEAR(1.0, clipEnd1, kEpsilon);
+}