From 18c8cb9da883fe7c8d2a8e1c5ace1ed1b59fe3af Mon Sep 17 00:00:00 2001
From: Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>
Date: Wed, 5 May 2021 14:09:47 +0300
Subject: [PATCH] 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.
---
 include/mapbox/geojsonvt/clip.hpp | 18 ++++++++++-----
 test/test.cpp                     | 38 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/include/mapbox/geojsonvt/clip.hpp b/include/mapbox/geojsonvt/clip.hpp
index acb10bd..881ef16 100644
--- 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 isLastSeg = (i == (len - 2));
 
             if (lineMetrics) segLen = ::hypot((b.x - a.x), (b.y - a.y));
 
@@ -135,9 +136,11 @@ 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 (isLastSeg) slice.emplace_back(b); // last point
 
-                    if (i == len - 2)
-                        slice.emplace_back(b); // last point
+                } else if (bk == k1 && !isLastSeg) { // --->|..  |
+                    if (lineMetrics) slice.segStart = lineLen + segLen;
+                    slice.emplace_back(b);
                 }
             } else if (ak > k2) {
                 if (bk < k1) { // <--|-----|---
@@ -152,13 +155,16 @@ class clipper {
                     slices.emplace_back(std::move(slice));
 
                     slice = newSlice(line);
+
                 } else if (bk < k2) { // |  <--|---
                     t = calc_progress<I>(a, b, k2);
                     slice.emplace_back(intersect<I>(a, b, k2, t));
                     if (lineMetrics) slice.segStart = lineLen + segLen * t;
+                    if (isLastSeg) slice.emplace_back(b); // last point
 
-                    if (i == len - 2)
-                        slice.emplace_back(b); // last point
+                } else if (bk == k2 && !isLastSeg) { // |  ..|<---
+                    if (lineMetrics) slice.segStart = lineLen + segLen;
+                    slice.emplace_back(b);
                 }
             } else {
                 slice.emplace_back(a);
@@ -177,7 +183,7 @@ class clipper {
                     slices.emplace_back(std::move(slice));
                     slice = newSlice(line);
 
-                } else if (i == len - 2) { // | --> |
+                } else if (isLastSeg) { // | --> |
                     slice.emplace_back(b);
                 }
             }
@@ -186,7 +192,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
index e884309..d10db6a 100644
--- 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);
+}