From 549aa701adc2362290ef4361585efc3e5bd23a2a Mon Sep 17 00:00:00 2001 From: zmiao Date: Tue, 28 Apr 2020 13:12:19 +0300 Subject: [PATCH 1/5] Add support for polygon in distance expression --- src/mbgl/style/expression/distance.cpp | 326 ++++++++++++++++++++++--- src/mbgl/util/geometry_util.cpp | 4 + 2 files changed, 297 insertions(+), 33 deletions(-) diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp index 5ab12063403..c839e764fab 100644 --- a/src/mbgl/style/expression/distance.cpp +++ b/src/mbgl/style/expression/distance.cpp @@ -24,6 +24,7 @@ namespace { const std::size_t MinPointsSize = 100; const std::size_t MinLinePointsSize = 50; const double InvalidDistance = std::numeric_limits::quiet_NaN(); +const double InfiniteDistance = std::numeric_limits::infinity(); const mapbox::cheap_ruler::CheapRuler::Unit UnitInMeters = mapbox::cheap_ruler::CheapRuler::Unit::Meters; // Inclusive index range for multipoint or linestring container @@ -33,6 +34,27 @@ std::size_t getRangeSize(const IndexRange& range) { return range.second - range.first + 1; } +std::pair, mbgl::optional> splitRange(const IndexRange& range, bool isLine) { + auto size = getRangeSize(range); + if (isLine) { + if (size == 2) { + return std::make_pair(range, nullopt); + } + auto size1 = size / 2; + IndexRange range1(range.first, range.first + size1); + IndexRange range2(range.first + size1, range.second); + return std::make_pair(std::move(range1), std::move(range2)); + } else { + if (size == 1) { + return std::make_pair(range, nullopt); + } + auto size1 = size / 2 - 1; + IndexRange range1(range.first, range.first + size1); + IndexRange range2(range.first + size1 + 1, range.second); + return std::make_pair(std::move(range1), std::move(range2)); + } +} + using DistanceBBox = GeometryBBox; DistanceBBox getBBox(const mapbox::geometry::multi_point& points, const IndexRange& range) { @@ -53,6 +75,16 @@ DistanceBBox getBBox(const mapbox::geometry::line_string& line, const In return bbox; } +DistanceBBox getBBox(const mapbox::geometry::polygon& polygon) { + DistanceBBox bbox = DefaultDistanceBBox; + for (const auto& ring : polygon) { + for (const auto& p : ring) { + updateBBox(bbox, p); + } + } + return bbox; +} + // Calculate the distance between two bounding boxes. // Calculate the delta in x and y direction, and use two fake points {0.0, 0.0} and {dx, dy} to calculate the distance. // Distance will be 0.0 if bounding box are overlapping. @@ -87,6 +119,20 @@ double pointToLineDistance(const mapbox::geometry::point& point, return ruler.distance(point, nearestPoint); } +// precondition is two segments are not intersecting with each other +double segmentToSegmentDistance(const mapbox::geometry::point& p1, + const mapbox::geometry::point& p2, + const mapbox::geometry::point& q1, + const mapbox::geometry::point& q2, + mapbox::cheap_ruler::CheapRuler& ruler) { + auto dist1 = std::min(pointToLineDistance(p1, mapbox::geometry::line_string{q1, q2}, ruler), + pointToLineDistance(p2, mapbox::geometry::line_string{q1, q2}, ruler)); + auto dist2 = std::min(pointToLineDistance(q1, mapbox::geometry::line_string{p1, p2}, ruler), + pointToLineDistance(q2, mapbox::geometry::line_string{p1, p2}, ruler)); + auto dist = std::min(dist1, dist2); + return dist; +} + double lineToLineDistance(const mapbox::geometry::line_string& line1, IndexRange& range1, const mapbox::geometry::line_string& line2, @@ -98,7 +144,7 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, mbgl::Log::Error(mbgl::Event::General, "index is out of range"); return InvalidDistance; } - double dist = std::numeric_limits::infinity(); + double dist = InfiniteDistance; for (std::size_t i = range1.first; i < range1.second; ++i) { const auto& p1 = line1[i]; const auto& p2 = line1[i + 1]; @@ -106,11 +152,7 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, const auto& q1 = line2[j]; const auto& q2 = line2[j + 1]; if (segmentIntersectSegment(p1, p2, q1, q2)) return 0.0; - auto dist1 = std::min(pointToLineDistance(p1, mapbox::geometry::line_string{q1, q2}, ruler), - pointToLineDistance(p2, mapbox::geometry::line_string{q1, q2}, ruler)); - auto dist2 = std::min(pointToLineDistance(q1, mapbox::geometry::line_string{p1, p2}, ruler), - pointToLineDistance(q2, mapbox::geometry::line_string{p1, p2}, ruler)); - dist = std::min(dist, std::min(dist1, dist2)); + dist = std::min(dist, segmentToSegmentDistance(p1, p2, q1, q2, ruler)); } } return dist; @@ -127,7 +169,7 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point mbgl::Log::Error(mbgl::Event::General, "index is out of range"); return InvalidDistance; } - double dist = std::numeric_limits::infinity(); + double dist = InfiniteDistance; for (std::size_t i = range1.first; i <= range1.second; ++i) { for (std::size_t j = range2.first; j <= range2.second; ++j) { dist = std::min(dist, ruler.distance(points1[i], points2[j])); @@ -137,25 +179,87 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point return dist; } -std::pair, mbgl::optional> splitRange(const IndexRange& range, bool isLine) { - auto size = getRangeSize(range); - if (isLine) { - if (size == 2) { - return std::make_pair(range, nullopt); +double pointToPolygonDistance(const mapbox::geometry::point& point, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler) { + if (pointWithinPolygon(point, polygon, true)) return 0.0; + double dist = InfiniteDistance; + for (const auto& ring : polygon) { + const auto nearestPoint = std::get<0>(ruler.pointOnLine(ring, point)); + dist = std::min(dist, ruler.distance(point, nearestPoint)); + if (dist == 0.0) return dist; + } + return dist; +} + +double lineToPolygonDistance(const mapbox::geometry::line_string& line, + const IndexRange& range, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler) { + bool rangeSafe = (range.second >= range.first && range.second < line.size()); + if (!rangeSafe) { + mbgl::Log::Error(mbgl::Event::General, "index is out of range"); + return InvalidDistance; + } + for (std::size_t i = range.first; i <= range.second; ++i) { + if (pointWithinPolygon(line[i], polygon, true)) return 0.0; + } + + double dist = InfiniteDistance; + for (std::size_t i = range.first; i < range.second; ++i) { + const auto& p1 = line[i]; + const auto& p2 = line[i + 1]; + for (const auto& ring : polygon) { + for (std::size_t j = 0; j < ring.size() - 2; ++j) { + const auto& q1 = ring[j]; + const auto& q2 = ring[j + 1]; + if (segmentIntersectSegment(p1, p2, q1, q2)) return 0.0; + dist = std::min(dist, segmentToSegmentDistance(p1, p2, q1, q2, ruler)); + } } - auto size1 = size / 2; - IndexRange range1(range.first, range.first + size1); - IndexRange range2(range.first + size1, range.second); - return std::make_pair(std::move(range1), std::move(range2)); - } else { - if (size == 1) { - return std::make_pair(range, nullopt); + } + return dist; +} + +double polygonToPolygonDistance(const mapbox::geometry::polygon& polygon1, + const mapbox::geometry::polygon& polygon2, + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + const auto bbox1 = getBBox(polygon1); + const auto bbox2 = getBBox(polygon2); + if (currentMiniDist != InfiniteDistance && bboxToBBoxDistance(bbox1, bbox2, ruler) >= currentMiniDist) + return currentMiniDist; + + const auto polygonIntersect = [](const mapbox::geometry::polygon& poly1, + const mapbox::geometry::polygon& poly2) { + for (const auto& ring : poly1) { + for (std::size_t i = 0; i <= ring.size() - 2; ++i) { + if (pointWithinPolygon(ring[i], poly2, true)) return true; + } + } + return false; + }; + if (boxWithinBox(bbox1, bbox2)) { + if (polygonIntersect(polygon1, polygon2)) return 0.0; + } else if (polygonIntersect(polygon2, polygon1)) + return 0.0; + + double dist = InfiniteDistance; + for (const auto& ring1 : polygon1) { + for (std::size_t i = 0; i < ring1.size() - 2; ++i) { + const auto& p1 = ring1[i]; + const auto& p2 = ring1[i + 1]; + for (const auto& ring2 : polygon2) { + for (std::size_t j = 0; j < ring2.size() - 2; ++j) { + const auto& q1 = ring2[j]; + const auto& q2 = ring2[j + 1]; + if (segmentIntersectSegment(p1, p2, q1, q2)) return 0.0; + dist = std::min(dist, segmentToSegmentDistance(p1, p2, q1, q2, ruler)); + } + } } - auto size1 = size / 2 - 1; - IndexRange range1(range.first, range.first + size1); - IndexRange range2(range.first + size1 + 1, range.second); - return std::make_pair(std::move(range1), std::move(range2)); } + return dist; } // @@ -166,12 +270,90 @@ struct Comparator { // The priority queue will ensure the top element would always be the pair that has the biggest distance using DistQueue = std::priority_queue, Comparator>; +double lineToPolygonDistance(const mapbox::geometry::line_string& line, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(ruler.distance(line[0], polygon[0][0]), currentMiniDist); + DistQueue distQueue; + distQueue.push(std::forward_as_tuple(0, IndexRange(0, line.size() - 1), IndexRange(0, 0))); + + while (!distQueue.empty()) { + auto distPair = distQueue.top(); + distQueue.pop(); + if (std::get<0>(distPair) > miniDist) continue; + auto& range = std::get<1>(distPair); + + // In case the set size are relatively small, we could use brute-force directly + if (getRangeSize(range) <= MinLinePointsSize) { + auto tempDist = lineToPolygonDistance(line, range, polygon, ruler); + if (std::isnan(tempDist)) return tempDist; + miniDist = std::min(miniDist, tempDist); + if (miniDist == 0.0) return 0.0; + } else { + auto newRangesA = splitRange(range, true /*isLine*/); + const auto updateQueue = + [&distQueue, &miniDist, &ruler, &line, &polygon](mbgl::optional& rangeA) { + if (!rangeA) return; + auto tempDist = bboxToBBoxDistance(getBBox(line, *rangeA), getBBox(polygon), ruler); + // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + // The pair with biggest distance will be at the top + if (tempDist <= miniDist) + distQueue.push(std::make_tuple(tempDist, std::move(*rangeA), IndexRange(0, 0))); + }; + updateQueue(newRangesA.first); + updateQueue(newRangesA.second); + } + } + return miniDist; +} + +double pointsToPolygonDistance(const mapbox::geometry::multi_point& points, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(ruler.distance(points[0], polygon[0][0]), currentMiniDist); + DistQueue distQueue; + distQueue.push(std::forward_as_tuple(0, IndexRange(0, points.size() - 1), IndexRange(0, 0))); + + while (!distQueue.empty()) { + auto distPair = distQueue.top(); + distQueue.pop(); + if (std::get<0>(distPair) > miniDist) continue; + auto& range = std::get<1>(distPair); + + // In case the set size are relatively small, we could use brute-force directly + if (getRangeSize(range) <= MinLinePointsSize) { + for (std::size_t i = range.first; i <= range.second; ++i) { + auto tempDist = pointToPolygonDistance(points[i], polygon, ruler); + miniDist = std::min(miniDist, tempDist); + if (miniDist == 0.0) return 0.0; + } + } else { + auto newRangesA = splitRange(range, true /*isLine*/); + const auto updateQueue = + [&distQueue, &miniDist, &ruler, &points, &polygon](mbgl::optional& rangeA) { + if (!rangeA) return; + auto tempDist = bboxToBBoxDistance(getBBox(points, *rangeA), getBBox(polygon), ruler); + // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + // The pair with biggest distance will be at the top + if (tempDist <= miniDist) + distQueue.push(std::make_tuple(tempDist, std::move(*rangeA), IndexRange(0, 0))); + }; + updateQueue(newRangesA.first); + updateQueue(newRangesA.second); + } + } + return miniDist; +} + // Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) // Use index for in-place processing. double lineToLineDistance(const mapbox::geometry::line_string& line1, const mapbox::geometry::multi_point& line2, - mapbox::cheap_ruler::CheapRuler& ruler) { - auto miniDist = ruler.distance(line1[0], line2[0]); + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(ruler.distance(line1[0], line2[0]), currentMiniDist); DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, line1.size() - 1), IndexRange(0, line2.size() - 1))); @@ -308,7 +490,7 @@ double pointsToLineDistance(const mapbox::geometry::multi_point& points, double pointsToLinesDistance(const mapbox::geometry::multi_point& points, const mapbox::geometry::multi_line_string& lines, mapbox::cheap_ruler::CheapRuler& ruler) { - double dist = std::numeric_limits::infinity(); + double dist = InfiniteDistance; for (const auto& line : lines) { dist = std::min(dist, pointsToLineDistance(points, line, ruler)); if (dist == 0.0) return 0.0; @@ -319,9 +501,9 @@ double pointsToLinesDistance(const mapbox::geometry::multi_point& points double lineToLinesDistance(const mapbox::geometry::line_string& line, const mapbox::geometry::multi_line_string& lines, mapbox::cheap_ruler::CheapRuler& ruler) { - double dist = std::numeric_limits::infinity(); + double dist = InfiniteDistance; for (const auto& l : lines) { - dist = std::min(dist, lineToLineDistance(line, l, ruler)); + dist = std::min(dist, lineToLineDistance(line, l, ruler, dist)); if (dist == 0.0) return 0.0; } return dist; @@ -343,6 +525,19 @@ double pointsToGeometryDistance(const mapbox::geometry::multi_point& poi [&points, &ruler](const mapbox::geometry::multi_line_string& lines) { return pointsToLinesDistance(points, lines, ruler); }, + [&points, &ruler](const mapbox::geometry::polygon& polygon) -> double { + return pointsToPolygonDistance(points, polygon, ruler); + }, + [&points, &ruler](const mapbox::geometry::multi_polygon& polygons) -> double { + double dist = InfiniteDistance; + for (const auto& polygon : polygons) { + auto tempDist = pointsToPolygonDistance(points, polygon, ruler, dist); + if (std::isnan(tempDist)) return tempDist; + dist = std::min(dist, tempDist); + if (dist == 0.0 || std::isnan(dist)) return dist; + } + return dist; + }, [](const auto&) { return InvalidDistance; }); } @@ -362,6 +557,57 @@ double lineToGeometryDistance(const mapbox::geometry::line_string& line, [&line, &ruler](const mapbox::geometry::multi_line_string& lines) { return lineToLinesDistance(line, lines, ruler); }, + [&line, &ruler](const mapbox::geometry::polygon& polygon) -> double { + return lineToPolygonDistance(line, polygon, ruler); + }, + [&line, &ruler](const mapbox::geometry::multi_polygon& polygons) -> double { + double dist = InfiniteDistance; + for (const auto& polygon : polygons) { + auto tempDist = lineToPolygonDistance(line, polygon, ruler, dist); + if (std::isnan(tempDist)) return tempDist; + dist = std::min(dist, tempDist); + if (dist == 0.0 || std::isnan(dist)) return dist; + } + return dist; + }, + [](const auto&) { return InvalidDistance; }); +} + +double polygonToGeometryDistance(const mapbox::geometry::polygon& polygon, + const Feature::geometry_type& geoSet) { + assert(!polygon.empty()); + mapbox::cheap_ruler::CheapRuler ruler(polygon.front().front().y, UnitInMeters); + return geoSet.match( + [&polygon, &ruler](const mapbox::geometry::point& p) { + return pointToPolygonDistance(p, polygon, ruler); + }, + [&polygon, &ruler](const mapbox::geometry::multi_point& points) { + return pointsToPolygonDistance(points, polygon, ruler); + }, + [&polygon, &ruler](const mapbox::geometry::line_string& line) { + return lineToPolygonDistance(line, polygon, ruler); + }, + [&polygon, &ruler](const mapbox::geometry::multi_line_string& lines) { + double dist = InfiniteDistance; + for (const auto& line : lines) { + auto tempDist = lineToPolygonDistance(line, polygon, ruler); + dist = std::min(dist, tempDist); + if (dist == 0.0 || std::isnan(dist)) return dist; + } + return dist; + }, + [&polygon, &ruler](const mapbox::geometry::polygon& polygon1) { + return polygonToPolygonDistance(polygon, polygon1, ruler); + }, + [&polygon, &ruler](const mapbox::geometry::multi_polygon& polygons) { + double dist = InfiniteDistance; + for (const auto& polygon1 : polygons) { + auto tempDist = polygonToPolygonDistance(polygon, polygon1, ruler, dist); + dist = std::min(dist, tempDist); + if (dist == 0.0 || std::isnan(dist)) return dist; + } + return dist; + }, [](const auto&) { return InvalidDistance; }); } @@ -380,7 +626,7 @@ double calculateDistance(const GeometryTileFeature& feature, return lineToGeometryDistance(line, geoSet); }, [&geoSet](const mapbox::geometry::multi_line_string& lines) -> double { - double dist = std::numeric_limits::infinity(); + double dist = InfiniteDistance; for (const auto& line : lines) { auto tempDist = lineToGeometryDistance(line, geoSet); if (std::isnan(tempDist)) return tempDist; @@ -389,6 +635,19 @@ double calculateDistance(const GeometryTileFeature& feature, } return dist; }, + [&geoSet](const mapbox::geometry::polygon& polygon) -> double { + return polygonToGeometryDistance(polygon, geoSet); + }, + [&geoSet](const mapbox::geometry::multi_polygon& polygons) -> double { + double dist = InfiniteDistance; + for (const auto& polygon : polygons) { + auto tempDist = polygonToGeometryDistance(polygon, geoSet); + if (std::isnan(tempDist)) return tempDist; + dist = std::min(dist, tempDist); + if (dist == 0.0 || std::isnan(dist)) return dist; + } + return dist; + }, [](const auto&) -> double { return InvalidDistance; }); } @@ -419,10 +678,11 @@ optional parseValue(const style::conversion::Convertible& value, style: optional getGeometry(const Feature& feature, mbgl::style::expression::ParsingContext& ctx) { const auto type = apply_visitor(ToFeatureType(), feature.geometry); - if (type == FeatureType::Point || type == FeatureType::LineString) { + if (type != FeatureType::Unknown) { return feature.geometry; } - ctx.error("'distance' expression requires valid geojson object with valid geometry type: Point/LineString."); + ctx.error( + "'distance' expression requires valid geojson object with valid geometry type: Point, LineString or Polygon."); return nullopt; } } // namespace @@ -442,14 +702,14 @@ EvaluationResult Distance::evaluate(const EvaluationContext& params) const { return EvaluationError{"distance expression requirs valid feature and canonical information."}; } auto geometryType = params.feature->getType(); - if (geometryType == FeatureType::Point || geometryType == FeatureType::LineString) { + if (geometryType != FeatureType::Unknown) { auto distance = calculateDistance(*params.feature, *params.canonical, geometries); if (!std::isnan(distance)) { assert(distance >= 0.0); return distance; } } - return EvaluationError{"distance expression currently only evaluates Point/LineString geometries."}; + return EvaluationError{"distance expression currently only evaluates Point/LineString/Polygon geometries."}; } ParseResult Distance::parse(const Convertible& value, ParsingContext& ctx) { diff --git a/src/mbgl/util/geometry_util.cpp b/src/mbgl/util/geometry_util.cpp index 4a90c75582d..5cade1eff8f 100644 --- a/src/mbgl/util/geometry_util.cpp +++ b/src/mbgl/util/geometry_util.cpp @@ -157,9 +157,13 @@ template bool lineStringWithinPolygon(const LineString& line, const Pol template bool lineStringWithinPolygons(const LineString& line, const MultiPolygon& polygons); template void updateBBox(GeometryBBox& bbox, const Point& p); +template bool boxWithinBox(const GeometryBBox& bbox1, const GeometryBBox& bbox2); template bool segmentIntersectSegment(const Point& a, const Point& b, const Point& c, const Point& d); +template bool pointWithinPolygon(const Point& point, + const Polygon& polygon, + bool trueOnBoundary = false); } // namespace mbgl From 6fe03708ce1b89969443f8f093a4dd6de306e73f Mon Sep 17 00:00:00 2001 From: zmiao Date: Tue, 28 Apr 2020 20:03:46 +0300 Subject: [PATCH 2/5] [test] Distance expression: Update unit tests --- src/mbgl/style/expression/distance.cpp | 25 ++- .../geometry_data/multi_point_3.geojson | 75 +++++++ .../geometry_data/multi_polygon_1.geojson | 132 ++++++++++++ .../geometry_data/multi_polygon_2.geojson | 81 ++++++++ test/style/property_expression.test.cpp | 189 +++++++++++++++--- 5 files changed, 456 insertions(+), 46 deletions(-) create mode 100644 test/fixtures/geometry_data/multi_point_3.geojson create mode 100644 test/fixtures/geometry_data/multi_polygon_1.geojson create mode 100644 test/fixtures/geometry_data/multi_polygon_2.geojson diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp index c839e764fab..18db0e06f8d 100644 --- a/src/mbgl/style/expression/distance.cpp +++ b/src/mbgl/style/expression/distance.cpp @@ -270,6 +270,9 @@ struct Comparator { // The priority queue will ensure the top element would always be the pair that has the biggest distance using DistQueue = std::priority_queue, Comparator>; +// Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) +// Most of the time, use index for in-place processing. + double lineToPolygonDistance(const mapbox::geometry::line_string& line, const mapbox::geometry::polygon& polygon, mapbox::cheap_ruler::CheapRuler& ruler, @@ -278,6 +281,7 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, line.size() - 1), IndexRange(0, 0))); + const auto polyBBox = getBBox(polygon); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); @@ -293,9 +297,9 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, } else { auto newRangesA = splitRange(range, true /*isLine*/); const auto updateQueue = - [&distQueue, &miniDist, &ruler, &line, &polygon](mbgl::optional& rangeA) { + [&distQueue, &miniDist, &ruler, &line, &polyBBox](mbgl::optional& rangeA) { if (!rangeA) return; - auto tempDist = bboxToBBoxDistance(getBBox(line, *rangeA), getBBox(polygon), ruler); + auto tempDist = bboxToBBoxDistance(getBBox(line, *rangeA), polyBBox, ruler); // Insert new pair to the queue if the bbox distance is less or equal to miniDist, // The pair with biggest distance will be at the top if (tempDist <= miniDist) @@ -316,6 +320,7 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, points.size() - 1), IndexRange(0, 0))); + const auto polyBBox = getBBox(polygon); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); @@ -330,11 +335,11 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin if (miniDist == 0.0) return 0.0; } } else { - auto newRangesA = splitRange(range, true /*isLine*/); + auto newRangesA = splitRange(range, false /*isLine*/); const auto updateQueue = - [&distQueue, &miniDist, &ruler, &points, &polygon](mbgl::optional& rangeA) { + [&distQueue, &miniDist, &ruler, &points, &polyBBox](mbgl::optional& rangeA) { if (!rangeA) return; - auto tempDist = bboxToBBoxDistance(getBBox(points, *rangeA), getBBox(polygon), ruler); + auto tempDist = bboxToBBoxDistance(getBBox(points, *rangeA), polyBBox, ruler); // Insert new pair to the queue if the bbox distance is less or equal to miniDist, // The pair with biggest distance will be at the top if (tempDist <= miniDist) @@ -347,8 +352,6 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin return miniDist; } -// Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) -// Use index for in-place processing. double lineToLineDistance(const mapbox::geometry::line_string& line1, const mapbox::geometry::multi_point& line2, mapbox::cheap_ruler::CheapRuler& ruler, @@ -391,8 +394,6 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, return miniDist; } -// Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) -// Use index for in-place processing. double pointsToPointsDistance(const mapbox::geometry::multi_point& pointSet1, const mapbox::geometry::multi_point& pointSet2, mapbox::cheap_ruler::CheapRuler& ruler) { @@ -436,8 +437,6 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point return miniDist; } -// Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) -// Most of the time, use index for in-place processing. double pointsToLineDistance(const mapbox::geometry::multi_point& points, const mapbox::geometry::line_string& line, mapbox::cheap_ruler::CheapRuler& ruler) { @@ -653,7 +652,7 @@ double calculateDistance(const GeometryTileFeature& feature, optional parseValue(const style::conversion::Convertible& value, style::expression::ParsingContext& ctx) { if (isArray(value)) { - // object value, quoted with ["Distance", GeoJSONObj] + // object value, quoted with ["distance", GeoJSONObj] auto length = arrayLength(value); if (length != 2) { ctx.error("'distance' expression requires one argument, but found " + @@ -672,7 +671,7 @@ optional parseValue(const style::conversion::Convertible& value, style: ctx.error(error.message); } } - ctx.error("'distance' expression needs to be an array with format [\"Distance\", GeoJSONObj]."); + ctx.error("'distance' expression needs to be an array with format [\"distance\", GeoJSONObj]."); return nullopt; } diff --git a/test/fixtures/geometry_data/multi_point_3.geojson b/test/fixtures/geometry_data/multi_point_3.geojson new file mode 100644 index 00000000000..3d486cbbf41 --- /dev/null +++ b/test/fixtures/geometry_data/multi_point_3.geojson @@ -0,0 +1,75 @@ +{ + "type": "Feature", + "properties": { + "marker-color": "#e0d96a" + }, + "geometry": { + "type": "MultiPoint", + "coordinates": [ + [ + 24.941325187683105, + 60.169503396855 + ], + [ + 24.943127632141113, + 60.16984495711831 + ], + [ + 24.94493007659912, + 60.17101904344053 + ], + [ + 24.947762489318848, + 60.16753935642882 + ], + [ + 24.9479341506958, + 60.16907644153227 + ], + [ + 24.946002960205078, + 60.16901239775532 + ], + [ + 24.944028854370117, + 60.16824386269413 + ], + [ + 24.9459171295166, + 60.16756070532545 + ], + [ + 24.940338134765625, + 60.168286559558055 + ], + [ + 24.94248390197754, + 60.1678168910033 + ], + [ + 24.944286346435547, + 60.16696293097573 + ], + [ + 24.946260452270508, + 60.16619434797435 + ], + [ + 24.941411018371582, + 60.16619434797435 + ], + [ + 24.943749904632565, + 60.16782756536319 + ], + [ + 24.942612648010254, + 60.16908711548297 + ], + [ + 24.944608211517334, + 60.16952474447549 + ] + ] + } +} \ No newline at end of file diff --git a/test/fixtures/geometry_data/multi_polygon_1.geojson b/test/fixtures/geometry_data/multi_polygon_1.geojson new file mode 100644 index 00000000000..0a804ce38de --- /dev/null +++ b/test/fixtures/geometry_data/multi_polygon_1.geojson @@ -0,0 +1,132 @@ +{ + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": { + "stroke": "#e0d96a", + "stroke-width": 2, + "stroke-opacity": 1, + "fill": "#c6ec00", + "fill-opacity": 0.5 + }, + "geometry": { + "type": "MultiPolygon", + "coordinates": [ + [ + [ + [ + 24.93870735168457, + 60.16429416283512 + ], + [ + 24.936389923095703, + 60.16205223008669 + ], + [ + 24.940123558044434, + 60.16250062887229 + ], + [ + 24.943857192993164, + 60.16333335324268 + ], + [ + 24.943127632141113, + 60.16465712803696 + ], + [ + 24.94123935699463, + 60.16595949958244 + ], + [ + 24.936561584472656, + 60.16561789892518 + ], + [ + 24.93450164794922, + 60.16435821580911 + ], + [ + 24.935274124145508, + 60.163568220403924 + ], + [ + 24.93870735168457, + 60.16429416283512 + ] + ] + ], + [ + [ + [ + 24.95166778564453, + 60.174306261926034 + ], + [ + 24.949607849121094, + 60.172918839697616 + ], + [ + 24.952783584594727, + 60.17144597351834 + ], + [ + 24.95595932006836, + 60.17217174191747 + ], + [ + 24.956517219543457, + 60.1742635728851 + ], + [ + 24.954586029052734, + 60.175736312737385 + ], + [ + 24.950637817382812, + 60.175245406790246 + ], + [ + 24.9514102935791, + 60.174348950911465 + ], + [ + 24.95166778564453, + 60.174306261926034 + ] + ] + ], + [ + [ + [ + 24.930424690246582, + 60.17413550542946 + ], + [ + 24.92875099182129, + 60.171830205844614 + ], + [ + 24.936904907226562, + 60.1703145966823 + ], + [ + 24.937891960144043, + 60.172150396016946 + ], + [ + 24.937891960144043, + 60.173922058560485 + ], + [ + 24.930424690246582, + 60.17413550542946 + ] + ] + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/test/fixtures/geometry_data/multi_polygon_2.geojson b/test/fixtures/geometry_data/multi_polygon_2.geojson new file mode 100644 index 00000000000..ea7723754d4 --- /dev/null +++ b/test/fixtures/geometry_data/multi_polygon_2.geojson @@ -0,0 +1,81 @@ +{ + "type": "Feature", + "properties": {}, + "geometry": { + "type": "MultiPolygon", + "coordinates": [ + [ + [ + [ + 24.94171142578125, + 60.16777419352904 + ], + [ + 24.948577880859375, + 60.16777419352904 + ], + [ + 24.948577880859375, + 60.170122472217564 + ], + [ + 24.94171142578125, + 60.170122472217564 + ], + [ + 24.94171142578125, + 60.16777419352904 + ] + ] + ], + [ + [ + [ + 24.951581954956055, + 60.169503396855 + ], + [ + 24.955186843872067, + 60.169503396855 + ], + [ + 24.955186843872067, + 60.17052806699205 + ], + [ + 24.951581954956055, + 60.17052806699205 + ], + [ + 24.951581954956055, + 60.169503396855 + ] + ] + ], + [ + [ + [ + 24.944243431091305, + 60.16301307713581 + ], + [ + 24.94716167449951, + 60.16301307713581 + ], + [ + 24.94716167449951, + 60.16412335429406 + ], + [ + 24.944243431091305, + 60.16412335429406 + ], + [ + 24.944243431091305, + 60.16301307713581 + ] + ] + ] + ] + } +} \ No newline at end of file diff --git a/test/style/property_expression.test.cpp b/test/style/property_expression.test.cpp index 924b0f91f64..aecbec18e33 100644 --- a/test/style/property_expression.test.cpp +++ b/test/style/property_expression.test.cpp @@ -468,11 +468,12 @@ TEST(PropertyExpression, WithinExpressionAntiMeridian) { TEST(PropertyExpression, DistanceExpression) { static const double invalidResult = std::numeric_limits::quiet_NaN(); static const CanonicalTileID canonicalTileID(15, 18653, 9484); + // Parsing error with invalid geojson source { // geoJSON source must be Point or LineString. const std::string invalidGeoSource = R"({ - "type": "Polygon", + "type": "Unknown", "coordinates": [[[0, 0], [0, 5], [5, 5], [5, 0], [0, 0]]] })"; std::stringstream ss; @@ -481,48 +482,18 @@ TEST(PropertyExpression, DistanceExpression) { ASSERT_FALSE(expression); } - // Parsing error with invalid unit + // Parsing error with extra arguments { const std::string invalidGeoSource = R"({ "type": "Point", "coordinates": [0, 0], })"; std::stringstream ss; - ss << std::string(R"(["distance", )") << invalidGeoSource << std::string(R"(, "InvalidUnits"])"); + ss << std::string(R"(["distance", )") << invalidGeoSource << std::string(R"(, "ExtraArgument"])"); auto expression = createExpression(ss.str().c_str()); ASSERT_FALSE(expression); } - // Evaluation test with non-supported geometry type - { - const std::string pointGeoSource = R"({ - "type": "Point", - "coordinates": - [ - 24.938492774963375, - 60.16980226227959 - ] - })"; - std::stringstream ss; - ss << std::string(R"(["distance", )") << pointGeoSource << std::string(R"( ])"); - auto expression = createExpression(ss.str().c_str()); - ASSERT_TRUE(expression); - PropertyExpression propExpr(std::move(expression)); - - Polygon testRing{{{24.937505722045895, 60.16815846879986}, - {24.940595626831055, 60.16815846879986}, - {24.940595626831055, 60.169268572114106}, - {24.937505722045895, 60.169268572114106}, - {24.937505722045895, 60.16815846879986}}}; - auto geoPoly = convertGeometry(testRing, canonicalTileID); - StubGeometryTileFeature polyFeature(FeatureType::Polygon, geoPoly); - - // return evaluation error - auto evaluatedResult = - propExpr.evaluate(EvaluationContext(&polyFeature).withCanonicalTileID(&canonicalTileID), invalidResult); - EXPECT_TRUE(std::isnan(evaluatedResult)); - } - // Evaluation test with Point to Point distance by using different units { const std::string pointGeoSource = R"({ @@ -752,4 +723,156 @@ TEST(PropertyExpression, DistanceExpression) { EvaluationContext(&multiLineFeature).withCanonicalTileID(&canonicalTileID), invalidResult); EXPECT_NEAR(384.109, evaluatedResult, 0.01); } + + // Evaluation test with MultiPoint to MultiPolygon distance + { + const auto multiPolygon = mbgl::util::read_file("test/fixtures/geometry_data/multi_polygon_1.geojson"); + std::stringstream ss; + ss << std::string(R"(["distance", )") << multiPolygon << std::string(R"( ])"); + auto expression = createExpression(ss.str().c_str()); + ASSERT_TRUE(expression); + PropertyExpression propExpr(std::move(expression)); + + const auto multiPointFeature1 = getFeature("multi_point_3.geojson", FeatureType::Point, canonicalTileID); + auto evaluatedResult = propExpr.evaluate( + EvaluationContext(&multiPointFeature1).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(27.917, evaluatedResult, 0.01); + + const auto multiPointFeature2 = getFeature("multi_point_2.geojson", FeatureType::Point, canonicalTileID); + evaluatedResult = propExpr.evaluate( + EvaluationContext(&multiPointFeature2).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + } + + // Evaluation test with MultiLine to MultiPolygon distance + { + const auto multiPolygon = mbgl::util::read_file("test/fixtures/geometry_data/multi_polygon_1.geojson"); + std::stringstream ss; + ss << std::string(R"(["distance", )") << multiPolygon << std::string(R"( ])"); + auto expression = createExpression(ss.str().c_str()); + ASSERT_TRUE(expression); + PropertyExpression propExpr(std::move(expression)); + + const auto multiLineFeature1 = + getFeature("multi_line_string_1.geojson", FeatureType::LineString, canonicalTileID); + auto evaluatedResult = propExpr.evaluate( + EvaluationContext(&multiLineFeature1).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(193.450, evaluatedResult, 0.01); + + const auto multiLineFeature2 = + getFeature("multi_line_string_2.geojson", FeatureType::LineString, canonicalTileID); + evaluatedResult = propExpr.evaluate(EvaluationContext(&multiLineFeature2).withCanonicalTileID(&canonicalTileID), + invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + } + + // Evaluation test with Polygon to MultiPolygon distance + { + const auto multiPolygon = mbgl::util::read_file("test/fixtures/geometry_data/multi_polygon_1.geojson"); + std::stringstream ss; + ss << std::string(R"(["distance", )") << multiPolygon << std::string(R"( ])"); + auto expression = createExpression(ss.str().c_str()); + ASSERT_TRUE(expression); + PropertyExpression propExpr(std::move(expression)); + + Polygon polygon{{{24.94171142578125, 60.16777419352904}, + {24.948577880859375, 60.16777419352904}, + {24.948577880859375, 60.170122472217564}, + {24.94171142578125, 60.170122472217564}, + {24.94171142578125, 60.16777419352904}}}; + auto geoPolygon = convertGeometry(polygon, canonicalTileID); + StubGeometryTileFeature polygonFeature(FeatureType::Polygon, geoPolygon); + + auto evaluatedResult = + propExpr.evaluate(EvaluationContext(&polygonFeature).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(203.876, evaluatedResult, 0.01); + } + + // Evaluation test with MultiPolygon to MultiPolygon distance + { + const auto multiPolygon = mbgl::util::read_file("test/fixtures/geometry_data/multi_polygon_1.geojson"); + std::stringstream ss; + ss << std::string(R"(["distance", )") << multiPolygon << std::string(R"( ])"); + auto expression = createExpression(ss.str().c_str()); + ASSERT_TRUE(expression); + PropertyExpression propExpr(std::move(expression)); + + const auto multiPolygonFeature = getFeature("multi_polygon_2.geojson", FeatureType::Polygon, canonicalTileID); + auto evaluatedResult = propExpr.evaluate( + EvaluationContext(&multiPolygonFeature).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(41.632, evaluatedResult, 0.01); + } + + // Evaluation test with LineString/Polygon to Polygon distance with special cases + { + const std::string multiPolygon = R"({ + "type": "Polygon", + "coordinates": [[[24.937891960144043, 60.16730451765011], + [24.94892120361328, 60.16730451765011], + [24.94892120361328, 60.17010112498546], + [24.937891960144043, 60.17010112498546], + [24.937891960144043, 60.16730451765011]]]})"; + std::stringstream ss; + ss << std::string(R"(["distance", )") << multiPolygon << std::string(R"( ])"); + auto expression = createExpression(ss.str().c_str()); + ASSERT_TRUE(expression); + PropertyExpression propExpr(std::move(expression)); + + // polygon1 is fully inside polygon + Polygon polygon1{{{24.941797256469727, 60.16850004304534}, + {24.94235515594482, 60.16850004304534}, + {24.94235515594482, 60.169439353910285}, + {24.941797256469727, 60.169439353910285}, + {24.941797256469727, 60.16850004304534}}}; + auto geoPolygon1 = convertGeometry(polygon1, canonicalTileID); + StubGeometryTileFeature polygonFeature1(FeatureType::Polygon, geoPolygon1); + + auto evaluatedResult = + propExpr.evaluate(EvaluationContext(&polygonFeature1).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + + // polygon2's line is intersecting polygon, but no point is inside polygon + Polygon polygon2{{{24.943385124206543, 60.166066249059554}, + {24.94596004486084, 60.166066249059554}, + {24.94596004486084, 60.1714246271462}, + {24.943385124206543, 60.1714246271462}, + {24.943385124206543, 60.166066249059554}}}; + auto geoPolygon2 = convertGeometry(polygon2, canonicalTileID); + StubGeometryTileFeature polygonFeature2(FeatureType::Polygon, geoPolygon2); + + evaluatedResult = + propExpr.evaluate(EvaluationContext(&polygonFeature2).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + + // one of polygon3's point is inside polygon + Polygon polygon3{{{24.94797706604004, 60.1694607015724}, + {24.952354431152344, 60.1694607015724}, + {24.952354431152344, 60.17082692309542}, + {24.94797706604004, 60.17082692309542}, + {24.94797706604004, 60.1694607015724}}}; + auto geoPolygon3 = convertGeometry(polygon3, canonicalTileID); + StubGeometryTileFeature polygonFeature3(FeatureType::Polygon, geoPolygon3); + + evaluatedResult = + propExpr.evaluate(EvaluationContext(&polygonFeature3).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + + // lineString1 is within polygon + LineString lineString1{{24.947097301483154, 60.16921520262075}, {24.947311878204346, 60.1680197032483}}; + auto geoLineString1 = convertGeometry(lineString1, canonicalTileID); + StubGeometryTileFeature lineFeature1(FeatureType::LineString, geoLineString1); + + evaluatedResult = + propExpr.evaluate(EvaluationContext(&lineFeature1).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + + // lineString2 is intersecting the polygon but its points are outside the polygon + LineString lineString2{{24.93999481201172, 60.17116846959888}, {24.940037727355957, 60.16542574699484}}; + auto geoLineString2 = convertGeometry(lineString2, canonicalTileID); + StubGeometryTileFeature lineFeature2(FeatureType::LineString, geoLineString2); + + evaluatedResult = + propExpr.evaluate(EvaluationContext(&lineFeature2).withCanonicalTileID(&canonicalTileID), invalidResult); + EXPECT_NEAR(0.0, evaluatedResult, 0.01); + } } From fa6edb71d3eec36714b4843b25dc582078b2c000 Mon Sep 17 00:00:00 2001 From: zmiao Date: Wed, 29 Apr 2020 11:02:02 +0300 Subject: [PATCH 3/5] [core] Distance expression: Refine logic --- src/mbgl/style/expression/distance.cpp | 277 +++++++++++++++--------- test/style/property_expression.test.cpp | 2 +- 2 files changed, 175 insertions(+), 104 deletions(-) diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp index 18db0e06f8d..023b5230078 100644 --- a/src/mbgl/style/expression/distance.cpp +++ b/src/mbgl/style/expression/distance.cpp @@ -34,7 +34,16 @@ std::size_t getRangeSize(const IndexRange& range) { return range.second - range.first + 1; } +bool isRangeSafe(const IndexRange& range, const std::size_t threshold) { + auto ret = (range.second >= range.first && range.second < threshold); + if (!ret) { + mbgl::Log::Error(mbgl::Event::Style, "Index is out of range"); + } + return ret; +} + std::pair, mbgl::optional> splitRange(const IndexRange& range, bool isLine) { + if (range.first > range.second) return std::make_pair(nullopt, nullopt); auto size = getRangeSize(range); if (isLine) { if (size == 2) { @@ -58,7 +67,8 @@ std::pair, mbgl::optional> splitRange(con using DistanceBBox = GeometryBBox; DistanceBBox getBBox(const mapbox::geometry::multi_point& points, const IndexRange& range) { - assert(range.second >= range.first && range.second < points.size()); + if (!isRangeSafe(range, points.size())) return DefaultDistanceBBox; + DistanceBBox bbox = DefaultDistanceBBox; for (std::size_t i = range.first; i <= range.second; ++i) { updateBBox(bbox, points[i]); @@ -67,7 +77,7 @@ DistanceBBox getBBox(const mapbox::geometry::multi_point& points, const } DistanceBBox getBBox(const mapbox::geometry::line_string& line, const IndexRange& range) { - assert(range.second >= range.first && range.second < line.size()); + if (!isRangeSafe(range, line.size())) return DefaultDistanceBBox; DistanceBBox bbox = DefaultDistanceBBox; for (std::size_t i = range.first; i <= range.second; ++i) { updateBBox(bbox, line[i]); @@ -85,12 +95,45 @@ DistanceBBox getBBox(const mapbox::geometry::polygon& polygon) { return bbox; } +bool isMultiPointValid(const mapbox::geometry::multi_point& points) { + if (points.empty()) { + mbgl::Log::Error(mbgl::Event::Style, "Invalid MultiPoint with empty geometry points"); + return false; + } + return true; +} + +bool isLineStringValid(const mapbox::geometry::line_string& line) { + if (line.size() < 2) { + mbgl::Log::Error(mbgl::Event::Style, "Invalid LineString with fewer than 2 geometry points"); + return false; + } + return true; +} + +bool isPolygonValid(const mapbox::geometry::polygon& polygon) { + if (polygon.empty()) { + mbgl::Log::Error(mbgl::Event::Style, "Invalid Polygon with empty rings"); + return false; + } + for (const auto& ring : polygon) { + if (ring.size() < 3) { + mbgl::Log::Error(mbgl::Event::Style, "Invalid Polygon with ring having fewer than 3 geometry points"); + return false; + } + } + return true; +} + // Calculate the distance between two bounding boxes. // Calculate the delta in x and y direction, and use two fake points {0.0, 0.0} and {dx, dy} to calculate the distance. // Distance will be 0.0 if bounding box are overlapping. double bboxToBBoxDistance(const DistanceBBox& bbox1, const DistanceBBox& bbox2, mapbox::cheap_ruler::CheapRuler& ruler) { + if (bbox1 == DefaultDistanceBBox || bbox2 == DefaultDistanceBBox) { + return InvalidDistance; + } double dx = 0.0; double dy = 0.0; // bbox1 in left side @@ -129,8 +172,7 @@ double segmentToSegmentDistance(const mapbox::geometry::point& p1, pointToLineDistance(p2, mapbox::geometry::line_string{q1, q2}, ruler)); auto dist2 = std::min(pointToLineDistance(q1, mapbox::geometry::line_string{p1, p2}, ruler), pointToLineDistance(q2, mapbox::geometry::line_string{p1, p2}, ruler)); - auto dist = std::min(dist1, dist2); - return dist; + return std::min(dist1, dist2); } double lineToLineDistance(const mapbox::geometry::line_string& line1, @@ -138,12 +180,8 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, const mapbox::geometry::line_string& line2, IndexRange& range2, mapbox::cheap_ruler::CheapRuler& ruler) { - bool rangeSafe = (range1.second >= range1.first && range1.second < line1.size()) && - (range2.second >= range2.first && range2.second < line2.size()); - if (!rangeSafe) { - mbgl::Log::Error(mbgl::Event::General, "index is out of range"); - return InvalidDistance; - } + bool rangeSafe = isRangeSafe(range1, line1.size()) && isRangeSafe(range2, line2.size()); + if (!rangeSafe) return InvalidDistance; double dist = InfiniteDistance; for (std::size_t i = range1.first; i < range1.second; ++i) { const auto& p1 = line1[i]; @@ -163,12 +201,8 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point const mapbox::geometry::multi_point& points2, IndexRange& range2, mapbox::cheap_ruler::CheapRuler& ruler) { - bool rangeSafe = (range1.second >= range1.first && range1.second < points1.size()) && - (range2.second >= range2.first && range2.second < points2.size()); - if (!rangeSafe) { - mbgl::Log::Error(mbgl::Event::General, "index is out of range"); - return InvalidDistance; - } + bool rangeSafe = isRangeSafe(range1, points1.size()) && isRangeSafe(range2, points2.size()); + if (!rangeSafe) return InvalidDistance; double dist = InfiniteDistance; for (std::size_t i = range1.first; i <= range1.second; ++i) { for (std::size_t j = range2.first; j <= range2.second; ++j) { @@ -182,9 +216,15 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point double pointToPolygonDistance(const mapbox::geometry::point& point, const mapbox::geometry::polygon& polygon, mapbox::cheap_ruler::CheapRuler& ruler) { - if (pointWithinPolygon(point, polygon, true)) return 0.0; + if (pointWithinPolygon(point, polygon, true /*trueOnBoundary*/)) return 0.0; double dist = InfiniteDistance; for (const auto& ring : polygon) { + if (ring.front() != ring.back()) { + dist = std::min( + dist, + pointToLineDistance(point, mapbox::geometry::line_string{ring.back(), ring.front()}, ruler)); + if (dist == 0.0) return dist; + } const auto nearestPoint = std::get<0>(ruler.pointOnLine(ring, point)); dist = std::min(dist, ruler.distance(point, nearestPoint)); if (dist == 0.0) return dist; @@ -196,13 +236,9 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, const IndexRange& range, const mapbox::geometry::polygon& polygon, mapbox::cheap_ruler::CheapRuler& ruler) { - bool rangeSafe = (range.second >= range.first && range.second < line.size()); - if (!rangeSafe) { - mbgl::Log::Error(mbgl::Event::General, "index is out of range"); - return InvalidDistance; - } + if (!isRangeSafe(range, line.size())) return InvalidDistance; for (std::size_t i = range.first; i <= range.second; ++i) { - if (pointWithinPolygon(line[i], polygon, true)) return 0.0; + if (pointWithinPolygon(line[i], polygon, true /*trueOnBoundary*/)) return 0.0; } double dist = InfiniteDistance; @@ -210,9 +246,9 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, const auto& p1 = line[i]; const auto& p2 = line[i + 1]; for (const auto& ring : polygon) { - for (std::size_t j = 0; j < ring.size() - 2; ++j) { - const auto& q1 = ring[j]; - const auto& q2 = ring[j + 1]; + for (std::size_t j = 0, len = ring.size(), k = len - 1; j < len; k = j++) { + const auto& q1 = ring[k]; + const auto& q2 = ring[j]; if (segmentIntersectSegment(p1, p2, q1, q2)) return 0.0; dist = std::min(dist, segmentToSegmentDistance(p1, p2, q1, q2, ruler)); } @@ -221,38 +257,40 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, return dist; } +// TODO: Currently the time complexity for polygon to polygon distance is quadratic, performance improvement is needed. double polygonToPolygonDistance(const mapbox::geometry::polygon& polygon1, const mapbox::geometry::polygon& polygon2, mapbox::cheap_ruler::CheapRuler& ruler, double currentMiniDist = InfiniteDistance) { const auto bbox1 = getBBox(polygon1); const auto bbox2 = getBBox(polygon2); - if (currentMiniDist != InfiniteDistance && bboxToBBoxDistance(bbox1, bbox2, ruler) >= currentMiniDist) + if (currentMiniDist != InfiniteDistance && bboxToBBoxDistance(bbox1, bbox2, ruler) >= currentMiniDist) { return currentMiniDist; - + } const auto polygonIntersect = [](const mapbox::geometry::polygon& poly1, const mapbox::geometry::polygon& poly2) { for (const auto& ring : poly1) { - for (std::size_t i = 0; i <= ring.size() - 2; ++i) { - if (pointWithinPolygon(ring[i], poly2, true)) return true; + for (std::size_t i = 0; i <= ring.size() - 1; ++i) { + if (pointWithinPolygon(ring[i], poly2, true /*trueOnBoundary*/)) return true; } } return false; }; if (boxWithinBox(bbox1, bbox2)) { if (polygonIntersect(polygon1, polygon2)) return 0.0; - } else if (polygonIntersect(polygon2, polygon1)) + } else if (polygonIntersect(polygon2, polygon1)) { return 0.0; + } double dist = InfiniteDistance; for (const auto& ring1 : polygon1) { - for (std::size_t i = 0; i < ring1.size() - 2; ++i) { - const auto& p1 = ring1[i]; - const auto& p2 = ring1[i + 1]; + for (std::size_t i = 0, len1 = ring1.size(), l = len1 - 1; i < len1; l = i++) { + const auto& p1 = ring1[l]; + const auto& p2 = ring1[i]; for (const auto& ring2 : polygon2) { - for (std::size_t j = 0; j < ring2.size() - 2; ++j) { - const auto& q1 = ring2[j]; - const auto& q2 = ring2[j + 1]; + for (std::size_t j = 0, len2 = ring2.size(), k = len2 - 1; j < len2; k = j++) { + const auto& q1 = ring2[k]; + const auto& q2 = ring2[j]; if (segmentIntersectSegment(p1, p2, q1, q2)) return 0.0; dist = std::min(dist, segmentToSegmentDistance(p1, p2, q1, q2, ruler)); } @@ -270,39 +308,45 @@ struct Comparator { // The priority queue will ensure the top element would always be the pair that has the biggest distance using DistQueue = std::priority_queue, Comparator>; -// Divide and conqure, the time complexity is O(n*lgn), faster than Brute force O(n*n) +// Divide and conquer, the time complexity is O(n*lgn), faster than Brute force O(n*n) // Most of the time, use index for in-place processing. -double lineToPolygonDistance(const mapbox::geometry::line_string& line, - const mapbox::geometry::polygon& polygon, - mapbox::cheap_ruler::CheapRuler& ruler, - double currentMiniDist = InfiniteDistance) { - auto miniDist = std::min(ruler.distance(line[0], polygon[0][0]), currentMiniDist); +double pointsToPolygonDistance(const mapbox::geometry::multi_point& points, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(ruler.distance(points[0], polygon[0][0]), currentMiniDist); + if (miniDist == 0.0) return miniDist; DistQueue distQueue; - distQueue.push(std::forward_as_tuple(0, IndexRange(0, line.size() - 1), IndexRange(0, 0))); + distQueue.push(std::forward_as_tuple(0, IndexRange(0, points.size() - 1), IndexRange(0, 0))); const auto polyBBox = getBBox(polygon); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); - if (std::get<0>(distPair) > miniDist) continue; + if (std::get<0>(distPair) >= miniDist) continue; auto& range = std::get<1>(distPair); // In case the set size are relatively small, we could use brute-force directly if (getRangeSize(range) <= MinLinePointsSize) { - auto tempDist = lineToPolygonDistance(line, range, polygon, ruler); - if (std::isnan(tempDist)) return tempDist; - miniDist = std::min(miniDist, tempDist); - if (miniDist == 0.0) return 0.0; + if (!isRangeSafe(range, points.size())) { + mbgl::Log::Error(mbgl::Event::Style, "Index is out of range"); + return InvalidDistance; + } + for (std::size_t i = range.first; i <= range.second; ++i) { + auto tempDist = pointToPolygonDistance(points[i], polygon, ruler); + miniDist = std::min(miniDist, tempDist); + if (miniDist == 0.0) return 0.0; + } } else { - auto newRangesA = splitRange(range, true /*isLine*/); + auto newRangesA = splitRange(range, false /*isLine*/); const auto updateQueue = - [&distQueue, &miniDist, &ruler, &line, &polyBBox](mbgl::optional& rangeA) { + [&distQueue, &miniDist, &ruler, &points, &polyBBox](mbgl::optional& rangeA) { if (!rangeA) return; - auto tempDist = bboxToBBoxDistance(getBBox(line, *rangeA), polyBBox, ruler); - // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + auto tempDist = bboxToBBoxDistance(getBBox(points, *rangeA), polyBBox, ruler); + // Insert new pair to the queue if the bbox distance is less than miniDist, // The pair with biggest distance will be at the top - if (tempDist <= miniDist) + if (tempDist < miniDist) distQueue.push(std::make_tuple(tempDist, std::move(*rangeA), IndexRange(0, 0))); }; updateQueue(newRangesA.first); @@ -312,37 +356,37 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, return miniDist; } -double pointsToPolygonDistance(const mapbox::geometry::multi_point& points, - const mapbox::geometry::polygon& polygon, - mapbox::cheap_ruler::CheapRuler& ruler, - double currentMiniDist = InfiniteDistance) { - auto miniDist = std::min(ruler.distance(points[0], polygon[0][0]), currentMiniDist); +double lineToPolygonDistance(const mapbox::geometry::line_string& line, + const mapbox::geometry::polygon& polygon, + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(ruler.distance(line[0], polygon[0][0]), currentMiniDist); + if (miniDist == 0.0) return miniDist; DistQueue distQueue; - distQueue.push(std::forward_as_tuple(0, IndexRange(0, points.size() - 1), IndexRange(0, 0))); + distQueue.push(std::forward_as_tuple(0, IndexRange(0, line.size() - 1), IndexRange(0, 0))); const auto polyBBox = getBBox(polygon); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); - if (std::get<0>(distPair) > miniDist) continue; + if (std::get<0>(distPair) >= miniDist) continue; auto& range = std::get<1>(distPair); // In case the set size are relatively small, we could use brute-force directly if (getRangeSize(range) <= MinLinePointsSize) { - for (std::size_t i = range.first; i <= range.second; ++i) { - auto tempDist = pointToPolygonDistance(points[i], polygon, ruler); - miniDist = std::min(miniDist, tempDist); - if (miniDist == 0.0) return 0.0; - } + auto tempDist = lineToPolygonDistance(line, range, polygon, ruler); + if (std::isnan(tempDist)) return tempDist; + miniDist = std::min(miniDist, tempDist); + if (miniDist == 0.0) return 0.0; } else { - auto newRangesA = splitRange(range, false /*isLine*/); + auto newRangesA = splitRange(range, true /*isLine*/); const auto updateQueue = - [&distQueue, &miniDist, &ruler, &points, &polyBBox](mbgl::optional& rangeA) { + [&distQueue, &miniDist, &ruler, &line, &polyBBox](mbgl::optional& rangeA) { if (!rangeA) return; - auto tempDist = bboxToBBoxDistance(getBBox(points, *rangeA), polyBBox, ruler); - // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + auto tempDist = bboxToBBoxDistance(getBBox(line, *rangeA), polyBBox, ruler); + // Insert new pair to the queue if the bbox distance is less than miniDist, // The pair with biggest distance will be at the top - if (tempDist <= miniDist) + if (tempDist < miniDist) distQueue.push(std::make_tuple(tempDist, std::move(*rangeA), IndexRange(0, 0))); }; updateQueue(newRangesA.first); @@ -353,17 +397,18 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin } double lineToLineDistance(const mapbox::geometry::line_string& line1, - const mapbox::geometry::multi_point& line2, + const mapbox::geometry::line_string& line2, mapbox::cheap_ruler::CheapRuler& ruler, double currentMiniDist = InfiniteDistance) { auto miniDist = std::min(ruler.distance(line1[0], line2[0]), currentMiniDist); + if (miniDist == 0.0) return miniDist; DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, line1.size() - 1), IndexRange(0, line2.size() - 1))); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); - if (std::get<0>(distPair) > miniDist) continue; + if (std::get<0>(distPair) >= miniDist) continue; auto& rangeA = std::get<1>(distPair); auto& rangeB = std::get<2>(distPair); @@ -380,9 +425,9 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, mbgl::optional& range1, mbgl::optional& range2) { if (!range1 || !range2) return; auto tempDist = bboxToBBoxDistance(getBBox(line1, *range1), getBBox(line2, *range2), ruler); - // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + // Insert new pair to the queue if the bbox distance is less than miniDist, // The pair with biggest distance will be at the top - if (tempDist <= miniDist) + if (tempDist < miniDist) distQueue.push(std::make_tuple(tempDist, std::move(*range1), std::move(*range2))); }; updateQueue(newRangesA.first, newRangesB.first); @@ -398,13 +443,14 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point const mapbox::geometry::multi_point& pointSet2, mapbox::cheap_ruler::CheapRuler& ruler) { auto miniDist = ruler.distance(pointSet1[0], pointSet2[0]); + if (miniDist == 0.0) return miniDist; DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, pointSet1.size() - 1), IndexRange(0, pointSet2.size() - 1))); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); - if (std::get<0>(distPair) > miniDist) { + if (std::get<0>(distPair) >= miniDist) { continue; } auto& rangeA = std::get<1>(distPair); @@ -423,9 +469,9 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point mbgl::optional& range1, mbgl::optional& range2) { if (!range1 || !range2) return; auto tempDist = bboxToBBoxDistance(getBBox(pointSet1, *range1), getBBox(pointSet2, *range2), ruler); - // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + // Insert new pair to the queue if the bbox distance is less than miniDist, // The pair with biggest distance will be at the top - if (tempDist <= miniDist) + if (tempDist < miniDist) distQueue.push(std::make_tuple(tempDist, std::move(*range1), std::move(*range2))); }; updateQueue(newRangesA.first, newRangesB.first); @@ -439,24 +485,25 @@ double pointsToPointsDistance(const mapbox::geometry::multi_point& point double pointsToLineDistance(const mapbox::geometry::multi_point& points, const mapbox::geometry::line_string& line, - mapbox::cheap_ruler::CheapRuler& ruler) { - auto miniDist = ruler.distance(points[0], line[0]); + mapbox::cheap_ruler::CheapRuler& ruler, + double currentMiniDist = InfiniteDistance) { + auto miniDist = std::min(currentMiniDist, ruler.distance(points[0], line[0])); + if (miniDist == 0.0) return miniDist; DistQueue distQueue; distQueue.push(std::forward_as_tuple(0, IndexRange(0, points.size() - 1), IndexRange(0, line.size() - 1))); while (!distQueue.empty()) { auto distPair = distQueue.top(); distQueue.pop(); - if (std::get<0>(distPair) > miniDist) continue; + if (std::get<0>(distPair) >= miniDist) continue; auto& rangeA = std::get<1>(distPair); auto& rangeB = std::get<2>(distPair); // In case the set size are relatively small, we could use brute-force directly if (getRangeSize(rangeA) <= MinPointsSize && getRangeSize(rangeB) <= MinLinePointsSize) { - bool rangeSafe = (rangeA.second >= rangeA.first && rangeA.second < points.size()) && - (rangeB.second >= rangeB.first && rangeB.second < line.size()); + bool rangeSafe = isRangeSafe(rangeA, points.size()) && isRangeSafe(rangeB, line.size()); if (!rangeSafe) { - mbgl::Log::Error(mbgl::Event::General, "index is out of range"); + mbgl::Log::Error(mbgl::Event::Style, "Index is out of range"); return InvalidDistance; } auto subLine = @@ -472,9 +519,9 @@ double pointsToLineDistance(const mapbox::geometry::multi_point& points, mbgl::optional& range1, mbgl::optional& range2) { if (!range1 || !range2) return; auto tempDist = bboxToBBoxDistance(getBBox(points, *range1), getBBox(line, *range2), ruler); - // Insert new pair to the queue if the bbox distance is less or equal to miniDist, + // Insert new pair to the queue if the bbox distance is less than miniDist, // The pair with biggest distance will be at the top - if (tempDist <= miniDist) + if (tempDist < miniDist) distQueue.push(std::make_tuple(tempDist, std::move(*range1), std::move(*range2))); }; updateQueue(newRangesA.first, newRangesB.first); @@ -491,8 +538,8 @@ double pointsToLinesDistance(const mapbox::geometry::multi_point& points mapbox::cheap_ruler::CheapRuler& ruler) { double dist = InfiniteDistance; for (const auto& line : lines) { - dist = std::min(dist, pointsToLineDistance(points, line, ruler)); - if (dist == 0.0) return 0.0; + dist = std::min(dist, pointsToLineDistance(points, line, ruler, dist)); + if (dist == 0.0) return dist; } return dist; } @@ -503,37 +550,45 @@ double lineToLinesDistance(const mapbox::geometry::line_string& line, double dist = InfiniteDistance; for (const auto& l : lines) { dist = std::min(dist, lineToLineDistance(line, l, ruler, dist)); - if (dist == 0.0) return 0.0; + if (dist == 0.0) return dist; } return dist; } double pointsToGeometryDistance(const mapbox::geometry::multi_point& points, const Feature::geometry_type& geoSet) { + if (!isMultiPointValid(points)) return InvalidDistance; mapbox::cheap_ruler::CheapRuler ruler(points.front().y, UnitInMeters); return geoSet.match( [&points, &ruler](const mapbox::geometry::point& p) { return pointsToPointsDistance(mapbox::geometry::multi_point{p}, points, ruler); }, [&points, &ruler](const mapbox::geometry::multi_point& points1) { + if (!isMultiPointValid(points1)) return InvalidDistance; return pointsToPointsDistance(points, points1, ruler); }, [&points, &ruler](const mapbox::geometry::line_string& line) { + if (!isLineStringValid(line)) return InvalidDistance; return pointsToLineDistance(points, line, ruler); }, [&points, &ruler](const mapbox::geometry::multi_line_string& lines) { + for (const auto& line : lines) { + if (!isLineStringValid(line)) return InvalidDistance; + } return pointsToLinesDistance(points, lines, ruler); }, [&points, &ruler](const mapbox::geometry::polygon& polygon) -> double { + if (!isPolygonValid(polygon)) return InvalidDistance; return pointsToPolygonDistance(points, polygon, ruler); }, [&points, &ruler](const mapbox::geometry::multi_polygon& polygons) -> double { double dist = InfiniteDistance; for (const auto& polygon : polygons) { + if (!isPolygonValid(polygon)) return InvalidDistance; auto tempDist = pointsToPolygonDistance(points, polygon, ruler, dist); if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, @@ -541,31 +596,38 @@ double pointsToGeometryDistance(const mapbox::geometry::multi_point& poi } double lineToGeometryDistance(const mapbox::geometry::line_string& line, const Feature::geometry_type& geoSet) { - assert(!line.empty()); + if (!isLineStringValid(line)) return InvalidDistance; mapbox::cheap_ruler::CheapRuler ruler(line.front().y, UnitInMeters); return geoSet.match( [&line, &ruler](const mapbox::geometry::point& p) { return pointsToLineDistance(mapbox::geometry::multi_point{p}, line, ruler); }, [&line, &ruler](const mapbox::geometry::multi_point& points) { + if (!isMultiPointValid(points)) return InvalidDistance; return pointsToLineDistance(points, line, ruler); }, [&line, &ruler](const mapbox::geometry::line_string& line1) { + if (!isLineStringValid(line1)) return InvalidDistance; return lineToLineDistance(line, line1, ruler); }, [&line, &ruler](const mapbox::geometry::multi_line_string& lines) { + for (const auto& line1 : lines) { + if (!isLineStringValid(line1)) return InvalidDistance; + } return lineToLinesDistance(line, lines, ruler); }, [&line, &ruler](const mapbox::geometry::polygon& polygon) -> double { + if (!isPolygonValid(polygon)) return InvalidDistance; return lineToPolygonDistance(line, polygon, ruler); }, [&line, &ruler](const mapbox::geometry::multi_polygon& polygons) -> double { double dist = InfiniteDistance; for (const auto& polygon : polygons) { + if (!isPolygonValid(polygon)) return InvalidDistance; auto tempDist = lineToPolygonDistance(line, polygon, ruler, dist); if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, @@ -574,36 +636,43 @@ double lineToGeometryDistance(const mapbox::geometry::line_string& line, double polygonToGeometryDistance(const mapbox::geometry::polygon& polygon, const Feature::geometry_type& geoSet) { - assert(!polygon.empty()); + if (!isPolygonValid(polygon)) return InvalidDistance; mapbox::cheap_ruler::CheapRuler ruler(polygon.front().front().y, UnitInMeters); return geoSet.match( [&polygon, &ruler](const mapbox::geometry::point& p) { return pointToPolygonDistance(p, polygon, ruler); }, [&polygon, &ruler](const mapbox::geometry::multi_point& points) { + if (!isMultiPointValid(points)) return InvalidDistance; return pointsToPolygonDistance(points, polygon, ruler); }, [&polygon, &ruler](const mapbox::geometry::line_string& line) { + if (!isLineStringValid(line)) return InvalidDistance; return lineToPolygonDistance(line, polygon, ruler); }, [&polygon, &ruler](const mapbox::geometry::multi_line_string& lines) { double dist = InfiniteDistance; for (const auto& line : lines) { - auto tempDist = lineToPolygonDistance(line, polygon, ruler); + if (!isLineStringValid(line)) return InvalidDistance; + auto tempDist = lineToPolygonDistance(line, polygon, ruler, dist); + if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, [&polygon, &ruler](const mapbox::geometry::polygon& polygon1) { + if (!isPolygonValid(polygon1)) return InvalidDistance; return polygonToPolygonDistance(polygon, polygon1, ruler); }, [&polygon, &ruler](const mapbox::geometry::multi_polygon& polygons) { double dist = InfiniteDistance; for (const auto& polygon1 : polygons) { + if (!isPolygonValid(polygon1)) return InvalidDistance; auto tempDist = polygonToPolygonDistance(polygon, polygon1, ruler, dist); + if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, @@ -630,7 +699,7 @@ double calculateDistance(const GeometryTileFeature& feature, auto tempDist = lineToGeometryDistance(line, geoSet); if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, @@ -643,7 +712,7 @@ double calculateDistance(const GeometryTileFeature& feature, auto tempDist = polygonToGeometryDistance(polygon, geoSet); if (std::isnan(tempDist)) return tempDist; dist = std::min(dist, tempDist); - if (dist == 0.0 || std::isnan(dist)) return dist; + if (dist == 0.0) return dist; } return dist; }, @@ -677,7 +746,7 @@ optional parseValue(const style::conversion::Convertible& value, style: optional getGeometry(const Feature& feature, mbgl::style::expression::ParsingContext& ctx) { const auto type = apply_visitor(ToFeatureType(), feature.geometry); - if (type != FeatureType::Unknown) { + if (type == FeatureType::Point || type == FeatureType::LineString || type == FeatureType::Polygon) { return feature.geometry; } ctx.error( @@ -701,14 +770,15 @@ EvaluationResult Distance::evaluate(const EvaluationContext& params) const { return EvaluationError{"distance expression requirs valid feature and canonical information."}; } auto geometryType = params.feature->getType(); - if (geometryType != FeatureType::Unknown) { + if (geometryType == FeatureType::Point || geometryType == FeatureType::LineString || + geometryType == FeatureType::Polygon) { auto distance = calculateDistance(*params.feature, *params.canonical, geometries); if (!std::isnan(distance)) { assert(distance >= 0.0); return distance; } } - return EvaluationError{"distance expression currently only evaluates Point/LineString/Polygon geometries."}; + return EvaluationError{"distance expression currently only evaluates valid Point/LineString/Polygon geometries."}; } ParseResult Distance::parse(const Convertible& value, ParsingContext& ctx) { @@ -739,7 +809,8 @@ ParseResult Distance::parse(const Convertible& value, ParsingContext& ctx) { return ParseResult(); }, [&ctx](const auto&) { - ctx.error("'distance' expression requires valid geojson that contains LineString/Point geometries."); + ctx.error( + "'distance' expression requires valid geojson that contains Point/LineString/Polygon geometries."); return ParseResult(); }); @@ -786,7 +857,7 @@ mbgl::Value Distance::serialize() const { serialized.emplace(m.name.GetString(), convertValue(m.value)); } } else { - mbgl::Log::Error(mbgl::Event::General, + mbgl::Log::Error(mbgl::Event::Style, "Failed to serialize 'distance' expression, converted rapidJSON is not an object"); } return std::vector{{getOperator(), serialized}}; diff --git a/test/style/property_expression.test.cpp b/test/style/property_expression.test.cpp index aecbec18e33..faa59025b06 100644 --- a/test/style/property_expression.test.cpp +++ b/test/style/property_expression.test.cpp @@ -800,7 +800,7 @@ TEST(PropertyExpression, DistanceExpression) { const auto multiPolygonFeature = getFeature("multi_polygon_2.geojson", FeatureType::Polygon, canonicalTileID); auto evaluatedResult = propExpr.evaluate( EvaluationContext(&multiPolygonFeature).withCanonicalTileID(&canonicalTileID), invalidResult); - EXPECT_NEAR(41.632, evaluatedResult, 0.01); + EXPECT_NEAR(21.446, evaluatedResult, 0.01); } // Evaluation test with LineString/Polygon to Polygon distance with special cases From c4f330a16486a88c6bab85451f975529813686ec Mon Sep 17 00:00:00 2001 From: zmiao Date: Wed, 29 Apr 2020 22:45:54 +0300 Subject: [PATCH 4/5] Update baseline --- metrics/binary-size/android-arm64-v8a/metrics.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics/binary-size/android-arm64-v8a/metrics.json b/metrics/binary-size/android-arm64-v8a/metrics.json index b250c661846..61b7bd537d3 100644 --- a/metrics/binary-size/android-arm64-v8a/metrics.json +++ b/metrics/binary-size/android-arm64-v8a/metrics.json @@ -3,7 +3,7 @@ [ "android-arm64-v8a", "/tmp/attach/install/android-arm64-v8a-release/lib/libmbgl-render-test-runner.so", - 1856758 + 1878341 ] ] } \ No newline at end of file From a78984fd313a0bdbee4923d074e5ff6b696edfe8 Mon Sep 17 00:00:00 2001 From: zmiao Date: Wed, 29 Apr 2020 22:50:13 +0300 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7febf31a06..2217b143d2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master +### ✨ New features + +- [core] Add support for `Polygon`, `MultiPolygon` geometry types in `distance` expression. ([#16446](https://github.com/mapbox/mapbox-gl-native/pull/16446)) + ### 🐞 Bug fixes - [core][tile mode] Labels priority fixes ([#16432](https://github.com/mapbox/mapbox-gl-native/pull/16432))