From 1bd2ccf2398004b02b479e6ad3144f52e7631b2a Mon Sep 17 00:00:00 2001 From: zmiao Date: Mon, 3 Feb 2020 16:53:15 +0200 Subject: Enable within expression with Filter add canonical as pointer fix review findings --- include/mbgl/style/expression/expression.hpp | 6 +-- include/mbgl/style/expression/within.hpp | 6 ++- src/mbgl/layout/pattern_layout.hpp | 8 ++-- src/mbgl/layout/symbol_layout.cpp | 3 +- src/mbgl/renderer/paint_property_binder.hpp | 6 +-- src/mbgl/style/conversion/filter.cpp | 3 ++ src/mbgl/style/expression/within.cpp | 58 +++++++++++++++------------- src/mbgl/tile/geometry_tile_worker.cpp | 3 +- 8 files changed, 54 insertions(+), 39 deletions(-) diff --git a/include/mbgl/style/expression/expression.hpp b/include/mbgl/style/expression/expression.hpp index f19f64c1e7..1e34a8bd38 100644 --- a/include/mbgl/style/expression/expression.hpp +++ b/include/mbgl/style/expression/expression.hpp @@ -54,20 +54,20 @@ public: return *this; }; - EvaluationContext& withCanonicalTileID(const mbgl::CanonicalTileID& canonical_) noexcept { + EvaluationContext& withCanonicalTileID(const mbgl::CanonicalTileID* canonical_) noexcept { canonical = canonical_; return *this; }; optional zoom; optional accumulated; - optional canonical; - GeometryTileFeature const * feature = nullptr; + GeometryTileFeature const* feature = nullptr; optional colorRampParameter; // Contains formatted section object, std::unordered_map. const Value* formattedSection = nullptr; const FeatureState* featureState = nullptr; const std::set* availableImages = nullptr; + const mbgl::CanonicalTileID* canonical = nullptr; }; template diff --git a/include/mbgl/style/expression/within.hpp b/include/mbgl/style/expression/within.hpp index 5e2b5e0671..88e9cc56b8 100644 --- a/include/mbgl/style/expression/within.hpp +++ b/include/mbgl/style/expression/within.hpp @@ -11,9 +11,11 @@ namespace mbgl { namespace style { namespace expression { -class Within : public Expression { +class Within final : public Expression { public: - Within(GeoJSON& geojson) : Expression(Kind::Within, type::Boolean), geoJSONSource(geojson) {} + explicit Within(GeoJSON geojson); + + ~Within() final; EvaluationResult evaluate(const EvaluationContext&) const override; diff --git a/src/mbgl/layout/pattern_layout.hpp b/src/mbgl/layout/pattern_layout.hpp index c6815e4cad..81ff3996db 100644 --- a/src/mbgl/layout/pattern_layout.hpp +++ b/src/mbgl/layout/pattern_layout.hpp @@ -108,7 +108,9 @@ public: const size_t featureCount = sourceLayer->featureCount(); for (size_t i = 0; i < featureCount; ++i) { auto feature = sourceLayer->getFeature(i); - if (!leaderLayerProperties->layerImpl().filter(style::expression::EvaluationContext { this->zoom, feature.get() })) + if (!leaderLayerProperties->layerImpl().filter( + style::expression::EvaluationContext(this->zoom, feature.get()) + .withCanonicalTileID(¶meters.tileID.canonical))) continue; PatternLayerMap patternDependencyMap; @@ -155,8 +157,8 @@ public: void createBucket(const ImagePositions& patternPositions, std::unique_ptr& featureIndex, std::unordered_map& renderData, - const bool, - const bool, + const bool /*firstLoad*/, + const bool /*showCollisionBoxes*/, const CanonicalTileID& canonical) override { auto bucket = std::make_shared(layout, layerPropertiesMap, zoom, overscaling); for (auto & patternFeature : features) { diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index a4890c85d1..e954a26a50 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -132,7 +132,8 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, const size_t featureCount = sourceLayer->featureCount(); for (size_t i = 0; i < featureCount; ++i) { auto feature = sourceLayer->getFeature(i); - if (!leader.filter(expression::EvaluationContext { this->zoom, feature.get() })) + if (!leader.filter(expression::EvaluationContext(this->zoom, feature.get()) + .withCanonicalTileID(¶meters.tileID.canonical))) continue; SymbolFeature ft(std::move(feature)); diff --git a/src/mbgl/renderer/paint_property_binder.hpp b/src/mbgl/renderer/paint_property_binder.hpp index 3a9dceacc0..61544be82d 100644 --- a/src/mbgl/renderer/paint_property_binder.hpp +++ b/src/mbgl/renderer/paint_property_binder.hpp @@ -227,7 +227,7 @@ public: const style::expression::Value& formattedSection) override { using style::expression::EvaluationContext; auto evaluated = expression.evaluate( - EvaluationContext(&feature).withFormattedSection(&formattedSection).withCanonicalTileID(canonical), + EvaluationContext(&feature).withFormattedSection(&formattedSection).withCanonicalTileID(&canonical), defaultValue); this->statistics.add(evaluated); auto value = attributeValue(evaluated); @@ -330,11 +330,11 @@ public: Range range = { expression.evaluate(EvaluationContext(zoomRange.min, &feature) .withFormattedSection(&formattedSection) - .withCanonicalTileID(canonical), + .withCanonicalTileID(&canonical), defaultValue), expression.evaluate(EvaluationContext(zoomRange.max, &feature) .withFormattedSection(&formattedSection) - .withCanonicalTileID(canonical), + .withCanonicalTileID(&canonical), defaultValue), }; this->statistics.add(range.min); diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp index 1b77985322..f3e7050ccc 100644 --- a/src/mbgl/style/conversion/filter.cpp +++ b/src/mbgl/style/conversion/filter.cpp @@ -207,6 +207,9 @@ ParseResult convertLegacyFilter(const Convertible& values, Error& error) { return nullopt; } else if (arrayLength(values) <= 1) { return {std::make_unique(*op != "any")}; + } else if (*op == "within") { + expression::ParsingContext ctx; + return ctx.parseExpression(values); } else { return { *op == "==" || diff --git a/src/mbgl/style/expression/within.cpp b/src/mbgl/style/expression/within.cpp index 1b1b8b11b9..d02ea47cb2 100644 --- a/src/mbgl/style/expression/within.cpp +++ b/src/mbgl/style/expression/within.cpp @@ -1,10 +1,13 @@ +#include + #include #include #include -#include #include +#include #include +namespace mbgl { namespace { double isLeft(mbgl::Point P0, mbgl::Point P1, mbgl::Point P2) { @@ -13,14 +16,13 @@ double isLeft(mbgl::Point P0, mbgl::Point P1, mbgl::Point point, const mapbox::geometry::polygon& polys) { +bool pointWithinPolygons(mbgl::Point point, const mapbox::geometry::polygon& polys) { // wn = the winding number (=0 only when point is outside) int wn = 0; for (auto poly : polys) { auto size = poly.size(); - auto i = size; // loop through every edge of the polygon - for (i = 0; i < size - 1; ++i) { + for (decltype(size) i = 0; i < size - 1; ++i) { if (poly[i].y <= point.y) { if (poly[i + 1].y > point.y) { // upward horizontal crossing from point to edge E(poly[i], poly[i+1]) if (isLeft(poly[i], poly[i + 1], point) > 0) { @@ -42,34 +44,33 @@ bool pointWithinPoly(mbgl::Point point, const mapbox::geometry::polygon< return wn != 0; } -bool pointsWithinPoly(const mbgl::GeometryTileFeature& feature, - const mbgl::CanonicalTileID& canonical, - const mbgl::GeoJSON& geoJson) { - bool result = false; - geoJson.match( - [&](const mapbox::geometry::geometry& geometrySet) { - geometrySet.match( - [&](const mapbox::geometry::polygon& polygons) { - convertGeometry(feature, canonical) +bool pointsWithinPolygons(const mbgl::GeometryTileFeature& feature, + const mbgl::CanonicalTileID& canonical, + const mbgl::GeoJSON& geoJson) { + return geoJson.match( + [&feature, &canonical](const mapbox::geometry::geometry& geometrySet) -> bool { + return geometrySet.match( + [&feature, &canonical](const mapbox::geometry::polygon& polygons) -> bool { + return convertGeometry(feature, canonical) .match( - [&](const mapbox::geometry::point& point) { - result = pointWithinPoly(point, polygons); + [&polygons](const mapbox::geometry::point& point) -> bool { + return pointWithinPolygons(point, polygons); }, - [&](const mapbox::geometry::multi_point& points) { + [&polygons](const mapbox::geometry::multi_point& points) -> bool { + auto result = false; for (const auto& p : points) { - result = pointWithinPoly(p, polygons); + result = pointWithinPolygons(p, polygons); if (!result) { - return; + return result; } } + return result; }, - [&](const auto&) { return; }); + [](const auto&) -> bool { return false; }); }, - [&](const auto&) { return; }); + [](const auto&) -> bool { return false; }); }, - [&](const auto&) { return; }); - - return result; + [](const auto&) -> bool { return false; }); } mbgl::optional parseValue(const mbgl::style::conversion::Convertible& value_, @@ -89,15 +90,18 @@ mbgl::optional parseValue(const mbgl::style::conversion::Converti } } ctx.error("'Within' expression requires valid geojson source that contains polygon geometry type."); - return mbgl::optional(); + return nullopt; } } // namespace -namespace mbgl { namespace style { namespace expression { +Within::Within(GeoJSON geojson) : Expression(Kind::Within, type::Boolean), geoJSONSource(std::move(geojson)) {} + +Within::~Within() {} + using namespace mbgl::style::conversion; EvaluationResult Within::evaluate(const EvaluationContext& params) const { @@ -107,7 +111,9 @@ EvaluationResult Within::evaluate(const EvaluationContext& params) const { auto geometryType = params.feature->getType(); // Currently only support Point/Points in polygon if (geometryType == FeatureType::Point) { - return pointsWithinPoly(*params.feature, *params.canonical, geoJSONSource); + return pointsWithinPolygons(*params.feature, *params.canonical, geoJSONSource); + } else { + mbgl::Log::Warning(mbgl::Event::General, "Within expression currently only support 'Point' geometry type"); } return false; } diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 0729ed2aa5..409dfdf428 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -401,7 +401,8 @@ void GeometryTileWorker::parse() { for (std::size_t i = 0; !obsolete && i < geometryLayer->featureCount(); i++) { std::unique_ptr feature = geometryLayer->getFeature(i); - if (!filter(expression::EvaluationContext { static_cast(this->id.overscaledZ), feature.get() })) + if (!filter(expression::EvaluationContext(static_cast(this->id.overscaledZ), feature.get()) + .withCanonicalTileID(&id.canonical))) continue; const GeometryCollection& geometries = feature->getGeometries(); -- cgit v1.2.1