diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp index 18db0e06f8d..d3c28aa2f4d 100644 --- a/src/mbgl/style/expression/distance.cpp +++ b/src/mbgl/style/expression/distance.cpp @@ -34,7 +34,12 @@ std::size_t getRangeSize(const IndexRange& range) { return range.second - range.first + 1; } +bool isRangeSafe(const IndexRange& range, const std::size_t threshold) { + return (range.second >= range.first && range.second < threshold); +} + 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 +63,10 @@ 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())) { + mbgl::Log::Error(mbgl::Event::General, "index is out of range"); + return DefaultDistanceBBox; + } DistanceBBox bbox = DefaultDistanceBBox; for (std::size_t i = range.first; i <= range.second; ++i) { updateBBox(bbox, points[i]); @@ -67,7 +75,10 @@ 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())) { + mbgl::Log::Error(mbgl::Event::General, "index is out of range"); + return DefaultDistanceBBox; + } DistanceBBox bbox = DefaultDistanceBBox; for (std::size_t i = range.first; i <= range.second; ++i) { updateBBox(bbox, line[i]); @@ -91,6 +102,9 @@ DistanceBBox getBBox(const mapbox::geometry::polygon& polygon) { 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 +143,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,8 +151,7 @@ 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()); + bool rangeSafe = isRangeSafe(range1, line1.size()) && isRangeSafe(range2, line2.size()); if (!rangeSafe) { mbgl::Log::Error(mbgl::Event::General, "index is out of range"); return InvalidDistance; @@ -163,8 +175,7 @@ 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()); + bool rangeSafe = isRangeSafe(range1, points1.size()) && isRangeSafe(range2, points2.size()); if (!rangeSafe) { mbgl::Log::Error(mbgl::Event::General, "index is out of range"); return InvalidDistance; @@ -182,7 +193,7 @@ 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) { const auto nearestPoint = std::get<0>(ruler.pointOnLine(ring, point)); @@ -196,13 +207,12 @@ 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) { + if (!isRangeSafe(range, line.size())) { 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; + if (pointWithinPolygon(line[i], polygon, true /*trueOnBoundary*/)) return 0.0; } double dist = InfiniteDistance; @@ -234,15 +244,16 @@ double polygonToPolygonDistance(const mapbox::geometry::polygon& polygon 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; + 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) { @@ -278,6 +289,7 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, 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, line.size() - 1), IndexRange(0, 0))); @@ -285,7 +297,7 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, 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 @@ -300,9 +312,9 @@ double lineToPolygonDistance(const mapbox::geometry::line_string& line, [&distQueue, &miniDist, &ruler, &line, &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, + // 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); @@ -317,6 +329,7 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin 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, points.size() - 1), IndexRange(0, 0))); @@ -324,11 +337,15 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin 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) { + if (!isRangeSafe(range, points.size())) { + mbgl::Log::Error(mbgl::Event::General, "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); @@ -340,9 +357,9 @@ double pointsToPolygonDistance(const mapbox::geometry::multi_point& poin [&distQueue, &miniDist, &ruler, &points, &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, + // 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); @@ -357,13 +374,14 @@ double lineToLineDistance(const mapbox::geometry::line_string& line1, 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 +398,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 +416,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 +442,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); @@ -441,20 +460,20 @@ 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]); + 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"); return InvalidDistance; @@ -472,9 +491,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); @@ -492,7 +511,7 @@ double pointsToLinesDistance(const mapbox::geometry::multi_point& points double dist = InfiniteDistance; for (const auto& line : lines) { dist = std::min(dist, pointsToLineDistance(points, line, ruler)); - if (dist == 0.0) return 0.0; + if (dist == 0.0) return dist; } return dist; } @@ -503,7 +522,7 @@ 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; } @@ -533,7 +552,7 @@ double pointsToGeometryDistance(const mapbox::geometry::multi_point& poi 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; }, @@ -565,7 +584,7 @@ double lineToGeometryDistance(const mapbox::geometry::line_string& line, 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; }, @@ -590,8 +609,9 @@ double polygonToGeometryDistance(const mapbox::geometry::polygon& polygo double dist = InfiniteDistance; for (const auto& line : lines) { auto tempDist = lineToPolygonDistance(line, polygon, ruler); + 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; }, @@ -602,8 +622,9 @@ double polygonToGeometryDistance(const mapbox::geometry::polygon& polygo double dist = InfiniteDistance; for (const auto& polygon1 : polygons) { 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 +651,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 +664,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 +698,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,7 +722,8 @@ 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); @@ -739,7 +761,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(); });