Fix within expression issue with geometry crossing the 180th Meridian

CHANGELOG.md

@@ -27,12 +27,14 @@
 
   Fix the issue that `within` expression evaluates point features inconsistently across zoom levels if the point lies near the boundary of a GeoJSON object ([#16301](https://github.com/mapbox/mapbox-gl-native/issues/16301))
 
- - [core][tile mode] Reduce cut-off labels ([#16336](https://github.com/mapbox/mapbox-gl-native/pull/16336))
+- [core][tile mode] Reduce cut-off labels ([#16336](https://github.com/mapbox/mapbox-gl-native/pull/16336))
 
   Place tile intersecting labels first, across all the layers. Thus we reduce the amount of label cut-offs in Tile mode.
 
   Before, labels were arranged within one symbol layer (one bucket),which was not enough for several symbol layers being placed at the same time.
 
+- [core] Fix issue that `within` expression returns incorrect results for geometries crossing the anti-meridian ([#16330](https://github.com/mapbox/mapbox-gl-native/pull/16330))
+
 ## maps-v1.4.1
 
 ### 🐞 Bug fixes

src/mbgl/style/expression/within.cpp

@@ -67,24 +67,47 @@ MultiPolygon<int64_t> getTilePolygons(const Feature::geometry_type& polygonGeoSe
         [](const auto&) { return MultiPolygon<int64_t>(); });
 }
 
+void updatePoint(Point<int64_t>& p, WithinBBox& bbox, const WithinBBox& polyBBox, const int64_t worldSize) {
+    if (p.x < polyBBox[0] || p.x > polyBBox[2]) {
+        const auto getShift = [](Point<int64_t>& point, const int64_t polygonSide, const int64_t size) -> int64_t {
+            if (point.x - polygonSide > size / 2) {
+                return -size;
+            }
+            if (polygonSide - point.x > size / 2) {
+                return size;
+            }
+            return 0;
+        };
+
+        int64_t shift = getShift(p, polyBBox[0], worldSize);
+        if (shift == 0) shift = getShift(p, polyBBox[2], worldSize);
+        p.x += shift;
+    }
+    updateBBox(bbox, p);
+}
+
 MultiPoint<int64_t> getTilePoints(const GeometryCoordinates& points,
                                   const mbgl::CanonicalTileID& canonical,
-                                  WithinBBox& bbox) {
+                                  WithinBBox& bbox,
+                                  const WithinBBox& polyBBox) {
     const int64_t xShift = util::EXTENT * canonical.x;
     const int64_t yShift = util::EXTENT * canonical.y;
+    const auto worldSize = util::EXTENT * std::pow(2, canonical.z);
     MultiPoint<int64_t> results;
     results.reserve(points.size());
     for (const auto& p : points) {
-        const auto point = Point<int64_t>(p.x + xShift, p.y + yShift);
-        updateBBox(bbox, point);
+        auto point = Point<int64_t>(p.x + xShift, p.y + yShift);
+        updatePoint(point, bbox, polyBBox, worldSize);
         results.push_back(point);
     }
+
     return results;
 }
 
 MultiLineString<int64_t> getTileLines(const GeometryCollection& lines,
                                       const mbgl::CanonicalTileID& canonical,
-                                      WithinBBox& bbox) {
+                                      WithinBBox& bbox,
+                                      const WithinBBox& polyBBox) {
     const int64_t xShift = util::EXTENT * canonical.x;
     const int64_t yShift = util::EXTENT * canonical.y;
     MultiLineString<int64_t> results;
@@ -99,6 +122,16 @@ MultiLineString<int64_t> getTileLines(const GeometryCollection& lines,
         }
         results.push_back(std::move(lineString));
     }
+
+    const auto worldSize = util::EXTENT * std::pow(2, canonical.z);
+    if (bbox[2] - bbox[0] <= worldSize / 2) {
+        bbox = DefaultBBox;
+        for (auto& line : results) {
+            for (auto& p : line) {
+                updatePoint(p, bbox, polyBBox, worldSize);
+            }
+        }
+    }
     return results;
 }
 
@@ -113,15 +146,15 @@ bool featureWithinPolygons(const GeometryTileFeature& feature,
         case FeatureType::Point: {
             assert(!geometries.empty());
             WithinBBox pointBBox = DefaultBBox;
-            MultiPoint<int64_t> points = getTilePoints(geometries.at(0), canonical, pointBBox);
+            MultiPoint<int64_t> points = getTilePoints(geometries.at(0), canonical, pointBBox, polyBBox);
             if (!boxWithinBox(pointBBox, polyBBox)) return false;
 
             return std::all_of(
                 points.begin(), points.end(), [&polygons](const auto& p) { return pointWithinPolygons(p, polygons); });
         }
         case FeatureType::LineString: {
             WithinBBox lineBBox = DefaultBBox;
-            MultiLineString<int64_t> multiLineString = getTileLines(geometries, canonical, lineBBox);
+            MultiLineString<int64_t> multiLineString = getTileLines(geometries, canonical, lineBBox, polyBBox);
             if (!boxWithinBox(lineBBox, polyBBox)) return false;
 
             return std::all_of(multiLineString.begin(), multiLineString.end(), [&polygons](const auto& line) {

src/mbgl/util/geometry_within.cpp

@@ -47,7 +47,11 @@ bool lineIntersectLine(const Point<int64_t>& a,
             y2 = p2.y - q1.y;
             x3 = q2.x - q1.x;
             y3 = q2.y - q1.y;
-            if ((x1 * y3 - x3 * y1) * (x2 * y3 - x3 * y2) < 0) return true;
+            auto ret1 = (x1 * y3 - x3 * y1);
+            auto ret2 = (x2 * y3 - x3 * y2);
+            if ((ret1 > 0 && ret2 < 0) || (ret1 < 0 && ret2 > 0)) {
+                return true;
+            }
             return false;
         };
 

test/style/property_expression.test.cpp

@@ -389,3 +389,75 @@ TEST(PropertyExpression, WithinExpression) {
         EXPECT_FALSE(evaluatedResult);
     }
 }
+
+TEST(PropertyExpression, WithinExpressionAntiMeridian) {
+    CanonicalTileID canonicalTileID(0, 0, 0);
+
+    // evaluation test with line geometries
+    {
+        static const std::string polygon1 = R"data(
+       {
+         "type": "Polygon",
+         "coordinates": [[[-190, 0], [-178, 0], [-178, 10], [-190, 10], [-190, 0]]]
+       })data";
+        // evaluation test with valid geojson source
+        std::stringstream ss;
+        ss << std::string(R"(["within", )") << polygon1 << std::string(R"( ])");
+        auto expression = createExpression(ss.str().c_str());
+        ASSERT_TRUE(expression);
+        PropertyExpression<bool> propExpr(std::move(expression));
+
+        LineString<double> testLine0{{-183, 5}, {-179, 1}};
+        auto geoLine0 = convertGeometry(testLine0, canonicalTileID);
+        StubGeometryTileFeature lineFeature0(FeatureType::LineString, geoLine0);
+
+        LineString<double> testLine1{{183, 5}, {181, 1}};
+        auto geoLine1 = convertGeometry(testLine1, canonicalTileID);
+        StubGeometryTileFeature lineFeature1(FeatureType::LineString, geoLine1);
+
+        auto evaluatedResult =
+            propExpr.evaluate(EvaluationContext(&lineFeature0).withCanonicalTileID(&canonicalTileID));
+        EXPECT_TRUE(evaluatedResult);
+        evaluatedResult = propExpr.evaluate(EvaluationContext(&lineFeature1).withCanonicalTileID(&canonicalTileID));
+        EXPECT_FALSE(evaluatedResult);
+    }
+
+    // evaluation test with point geometries
+    {
+        static const std::string polygon2 = R"data(
+       {
+         "type": "Polygon",
+         "coordinates":  [[[-185.0, 60.0], [-175.0, 60.0], [-175.0, 65.0], [-185.0, 65.0], [-185.0, 60.0]]]
+       })data";
+        // evaluation test with valid geojson source
+        std::stringstream ss;
+        ss << std::string(R"(["within", )") << polygon2 << std::string(R"( ])");
+        auto expression = createExpression(ss.str().c_str());
+        ASSERT_TRUE(expression);
+        PropertyExpression<bool> propExpr(std::move(expression));
+
+        auto getPointFeature = [&canonicalTileID](const Point<double>& testPoint) -> StubGeometryTileFeature {
+            auto geoPoint = convertGeometry(testPoint, canonicalTileID);
+            StubGeometryTileFeature pointFeature(FeatureType::Point, geoPoint);
+            return pointFeature;
+        };
+
+        // check `within` algorithm
+        auto pointFeature = getPointFeature(Point<double>(177, 62.5));
+        auto evaluatedResult =
+            propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID));
+        EXPECT_TRUE(evaluatedResult);
+
+        pointFeature = getPointFeature(Point<double>(180, 62.5));
+        evaluatedResult = propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID));
+        EXPECT_TRUE(evaluatedResult);
+
+        pointFeature = getPointFeature(Point<double>(-180, 62.5));
+        evaluatedResult = propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID));
+        EXPECT_TRUE(evaluatedResult);
+
+        pointFeature = getPointFeature(Point<double>(-190, 62.5));
+        evaluatedResult = propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID));
+        EXPECT_FALSE(evaluatedResult);
+    }
+}