From da35ef9f49139f4dfc7eb5ea2a04f800e895a65f Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 15 Jul 2016 16:07:42 -0700 Subject: [core] Add support for $id key to filters https://github.com/mapbox/mapbox-gl-style-spec/issues/391 --- benchmark/parse/filter.cpp | 2 +- include/mbgl/style/filter.hpp | 4 +- include/mbgl/style/filter_evaluator.hpp | 30 +++++- include/mbgl/util/feature.hpp | 6 +- platform/darwin/test/MGLFeatureTests.mm | 17 ++-- src/mbgl/annotation/shape_annotation_impl.cpp | 2 +- src/mbgl/renderer/symbol_bucket.cpp | 2 +- src/mbgl/style/bucket_parameters.cpp | 2 +- test/style/filter.cpp | 136 ++++++++++++++------------ 9 files changed, 116 insertions(+), 85 deletions(-) diff --git a/benchmark/parse/filter.cpp b/benchmark/parse/filter.cpp index 6b430a9b11..5214f682d6 100644 --- a/benchmark/parse/filter.cpp +++ b/benchmark/parse/filter.cpp @@ -28,7 +28,7 @@ static void Parse_EvaluateFilter(benchmark::State& state) { const PropertyMap properties = { { "foo", std::string("bar") } }; while (state.KeepRunning()) { - filter(FeatureType::Unknown, [&] (const std::string& key) -> optional { + filter(FeatureType::Unknown, {}, [&] (const std::string& key) -> optional { auto it = properties.find(key); if (it == properties.end()) return {}; diff --git a/include/mbgl/style/filter.hpp b/include/mbgl/style/filter.hpp index 6a5afb7b47..dd2b20cd0d 100644 --- a/include/mbgl/style/filter.hpp +++ b/include/mbgl/style/filter.hpp @@ -107,8 +107,10 @@ class Filter : public FilterBase { public: using FilterBase::FilterBase; + bool operator()(const Feature&) const; + template - bool operator()(FeatureType type, PropertyAccessor accessor) const; + bool operator()(FeatureType type, optional id, PropertyAccessor accessor) const; }; } // namespace style diff --git a/include/mbgl/style/filter_evaluator.hpp b/include/mbgl/style/filter_evaluator.hpp index cf91fdab1f..793abe6da0 100644 --- a/include/mbgl/style/filter_evaluator.hpp +++ b/include/mbgl/style/filter_evaluator.hpp @@ -26,6 +26,7 @@ template class FilterEvaluator { public: const FeatureType featureType; + const optional featureIdentifier; const PropertyAccessor propertyAccessor; bool operator()(const NullFilter&) const { @@ -123,9 +124,19 @@ public: private: optional getValue(const std::string& key_) const { - return key_ == "$type" - ? optional(uint64_t(featureType)) - : propertyAccessor(key_); + if (key_ == "$type") { + return optional(uint64_t(featureType)); + } else if (key_ == "$id") { + if (featureIdentifier) { + return FeatureIdentifier::visit(*featureIdentifier, [] (auto id) { + return Value(std::move(id)); + }); + } else { + return optional(); + } + } else { + return propertyAccessor(key_); + } } template @@ -183,9 +194,18 @@ private: } }; +inline bool Filter::operator()(const Feature& feature) const { + return operator()(apply_visitor(ToFeatureType(), feature.geometry), feature.id, [&] (const std::string& key) -> optional { + auto it = feature.properties.find(key); + if (it == feature.properties.end()) + return {}; + return it->second; + }); +} + template -bool Filter::operator()(FeatureType type, PropertyAccessor accessor) const { - return FilterBase::visit(*this, FilterEvaluator { type, accessor }); +bool Filter::operator()(FeatureType type, optional id, PropertyAccessor accessor) const { + return FilterBase::visit(*this, FilterEvaluator { type, id, accessor }); } } // namespace style diff --git a/include/mbgl/util/feature.hpp b/include/mbgl/util/feature.hpp index 7c5c8d7625..b72aa15ddd 100644 --- a/include/mbgl/util/feature.hpp +++ b/include/mbgl/util/feature.hpp @@ -10,10 +10,6 @@ using Value = mapbox::geometry::value; using NullValue = mapbox::geometry::null_value_t; using PropertyMap = mapbox::geometry::property_map; using FeatureIdentifier = mapbox::geometry::identifier; -class Feature : public mapbox::geometry::feature { -public: - Feature(geometry_type&& geometry_) - : mapbox::geometry::feature { std::move(geometry_) } {} -}; +using Feature = mapbox::geometry::feature; } // namespace mbgl diff --git a/platform/darwin/test/MGLFeatureTests.mm b/platform/darwin/test/MGLFeatureTests.mm index 13ad8759b0..1b1722f172 100644 --- a/platform/darwin/test/MGLFeatureTests.mm +++ b/platform/darwin/test/MGLFeatureTests.mm @@ -1,6 +1,7 @@ #import #import +#import #import "../../darwin/src/MGLFeature_Private.h" @interface MGLFeatureTests : XCTestCase @@ -12,16 +13,16 @@ - (void)testGeometryConversion { std::vector features; - mapbox::geometry::point point = { -90.066667, 29.95 }; - features.emplace_back(point); + mbgl::Point point = { -90.066667, 29.95 }; + features.push_back(mbgl::Feature { point }); - mapbox::geometry::line_string lineString = { + mbgl::LineString lineString = { { -84.516667, 39.1 }, { -90.066667, 29.95 }, }; - features.emplace_back(lineString); + features.push_back(mbgl::Feature { lineString }); - mapbox::geometry::polygon polygon = { + mbgl::Polygon polygon = { { { 1, 1 }, { 4, 1 }, @@ -35,7 +36,7 @@ { 2, 3 }, }, }; - features.emplace_back(polygon); + features.push_back(mbgl::Feature { polygon }); NS_ARRAY_OF(MGLShape *) *shapes = MGLFeaturesFromMBGLFeatures(features); XCTAssertEqual(shapes.count, 3, @"All features should be converted into shapes"); @@ -87,8 +88,8 @@ - (void)testPropertyConversion { std::vector features; - mapbox::geometry::point point = { -90.066667, 29.95 }; - mbgl::Feature pointFeature(point); + mbgl::Point point = { -90.066667, 29.95 }; + mbgl::Feature pointFeature { point }; pointFeature.id = { UINT64_MAX }; pointFeature.properties["null"] = nullptr; pointFeature.properties["bool"] = true; diff --git a/src/mbgl/annotation/shape_annotation_impl.cpp b/src/mbgl/annotation/shape_annotation_impl.cpp index 620b1acc76..2236f179b9 100644 --- a/src/mbgl/annotation/shape_annotation_impl.cpp +++ b/src/mbgl/annotation/shape_annotation_impl.cpp @@ -24,7 +24,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati if (!shapeTiler) { mapbox::geometry::feature_collection features; features.emplace_back(ShapeAnnotationGeometry::visit(geometry(), [] (auto&& geom) { - return Feature(std::move(geom)); + return Feature { std::move(geom) }; })); mapbox::geojsonvt::Options options; options.maxZoom = util::clamp(maxZoom, 0, 18); diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp index f48a98eaac..008372b2bf 100644 --- a/src/mbgl/renderer/symbol_bucket.cpp +++ b/src/mbgl/renderer/symbol_bucket.cpp @@ -119,7 +119,7 @@ void SymbolBucket::parseFeatures(const GeometryTileLayer& layer, const Filter& f const GLsizei featureCount = static_cast(layer.featureCount()); for (GLsizei i = 0; i < featureCount; i++) { auto feature = layer.getFeature(i); - if (!filter(feature->getType(), [&] (const auto& key) { return feature->getValue(key); })) + if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); })) continue; SymbolFeature ft; diff --git a/src/mbgl/style/bucket_parameters.cpp b/src/mbgl/style/bucket_parameters.cpp index 5bb08af873..64f53babcd 100644 --- a/src/mbgl/style/bucket_parameters.cpp +++ b/src/mbgl/style/bucket_parameters.cpp @@ -10,7 +10,7 @@ void BucketParameters::eachFilteredFeature(const Filter& filter, auto name = layer.getName(); for (std::size_t i = 0; !cancelled() && i < layer.featureCount(); i++) { auto feature = layer.getFeature(i); - if (!filter(feature->getType(), [&] (const auto& key) { return feature->getValue(key); })) + if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); })) continue; function(*feature, i, name); } diff --git a/test/style/filter.cpp b/test/style/filter.cpp index 53504d84ae..b49f424a39 100644 --- a/test/style/filter.cpp +++ b/test/style/filter.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include @@ -17,97 +19,107 @@ Filter parse(const char * expression) { return *conversion::convert(doc); } -bool evaluate(const Filter& filter, const PropertyMap& properties, FeatureType type = FeatureType::Unknown) { - return filter(type, [&] (const std::string& key) -> optional { - auto it = properties.find(key); - if (it == properties.end()) - return {}; - return it->second; - }); +Feature feature(const PropertyMap& properties, const Geometry& geometry = Point()) { + Feature result { geometry }; + result.properties = properties; + return result; } TEST(Filter, EqualsString) { Filter f = parse("[\"==\", \"foo\", \"bar\"]"); - ASSERT_TRUE(evaluate(f, {{ "foo", std::string("bar") }})); - ASSERT_FALSE(evaluate(f, {{ "foo", std::string("baz") }})); + ASSERT_TRUE(f(feature({{ "foo", std::string("bar") }}))); + ASSERT_FALSE(f(feature({{ "foo", std::string("baz") }}))); } TEST(Filter, EqualsNumber) { Filter f = parse("[\"==\", \"foo\", 0]"); - ASSERT_TRUE(evaluate(f, {{ "foo", int64_t(0) }})); - ASSERT_TRUE(evaluate(f, {{ "foo", uint64_t(0) }})); - ASSERT_TRUE(evaluate(f, {{ "foo", double(0) }})); - ASSERT_FALSE(evaluate(f, {{ "foo", int64_t(1) }})); - ASSERT_FALSE(evaluate(f, {{ "foo", uint64_t(1) }})); - ASSERT_FALSE(evaluate(f, {{ "foo", double(1) }})); - ASSERT_FALSE(evaluate(f, {{ "foo", std::string("0") }})); - ASSERT_FALSE(evaluate(f, {{ "foo", false }})); - ASSERT_FALSE(evaluate(f, {{ "foo", true }})); - ASSERT_FALSE(evaluate(f, {{ "foo", nullptr }})); - ASSERT_FALSE(evaluate(f, {{}})); + ASSERT_TRUE(f(feature({{ "foo", int64_t(0) }}))); + ASSERT_TRUE(f(feature({{ "foo", uint64_t(0) }}))); + ASSERT_TRUE(f(feature({{ "foo", double(0) }}))); + ASSERT_FALSE(f(feature({{ "foo", int64_t(1) }}))); + ASSERT_FALSE(f(feature({{ "foo", uint64_t(1) }}))); + ASSERT_FALSE(f(feature({{ "foo", double(1) }}))); + ASSERT_FALSE(f(feature({{ "foo", std::string("0") }}))); + ASSERT_FALSE(f(feature({{ "foo", false }}))); + ASSERT_FALSE(f(feature({{ "foo", true }}))); + ASSERT_FALSE(f(feature({{ "foo", nullptr }}))); + ASSERT_FALSE(f(feature({{}}))); } TEST(Filter, EqualsType) { Filter f = parse("[\"==\", \"$type\", \"LineString\"]"); - ASSERT_FALSE(evaluate(f, {{}}, FeatureType::Point)); - ASSERT_TRUE(evaluate(f, {{}}, FeatureType::LineString)); + ASSERT_FALSE(f(feature({{}}, Point()))); + ASSERT_TRUE(f(feature({{}}, LineString()))); } TEST(Filter, InType) { Filter f = parse("[\"in\", \"$type\", \"LineString\", \"Polygon\"]"); - ASSERT_FALSE(evaluate(f, {{}}, FeatureType::Point)); - ASSERT_TRUE(evaluate(f, {{}}, FeatureType::LineString)); - ASSERT_TRUE(evaluate(f, {{}}, FeatureType::Polygon)); + ASSERT_FALSE(f(feature({{}}, Point()))); + ASSERT_TRUE(f(feature({{}}, LineString()))); + ASSERT_TRUE(f(feature({{}}, Polygon()))); } TEST(Filter, Any) { - ASSERT_FALSE(evaluate(parse("[\"any\"]"), {{}})); - ASSERT_TRUE(evaluate(parse("[\"any\", [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_FALSE(evaluate(parse("[\"any\", [\"==\", \"foo\", 0]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_TRUE(evaluate(parse("[\"any\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); + ASSERT_FALSE(parse("[\"any\"]")(feature({{}}))); + ASSERT_TRUE(parse("[\"any\", [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_FALSE(parse("[\"any\", [\"==\", \"foo\", 0]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_TRUE(parse("[\"any\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); } TEST(Filter, All) { - ASSERT_TRUE(evaluate(parse("[\"all\"]"), {{}})); - ASSERT_TRUE(evaluate(parse("[\"all\", [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_FALSE(evaluate(parse("[\"all\", [\"==\", \"foo\", 0]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_FALSE(evaluate(parse("[\"all\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); + ASSERT_TRUE(parse("[\"all\"]")(feature({{}}))); + ASSERT_TRUE(parse("[\"all\", [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_FALSE(parse("[\"all\", [\"==\", \"foo\", 0]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_FALSE(parse("[\"all\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); } TEST(Filter, None) { - ASSERT_TRUE(evaluate(parse("[\"none\"]"), {{}})); - ASSERT_FALSE(evaluate(parse("[\"none\", [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_TRUE(evaluate(parse("[\"none\", [\"==\", \"foo\", 0]]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_FALSE(evaluate(parse("[\"none\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]"), - {{ std::string("foo"), int64_t(1) }})); + ASSERT_TRUE(parse("[\"none\"]")(feature({{}}))); + ASSERT_FALSE(parse("[\"none\", [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_TRUE(parse("[\"none\", [\"==\", \"foo\", 0]]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_FALSE(parse("[\"none\", [\"==\", \"foo\", 0], [\"==\", \"foo\", 1]]")( + feature({{ std::string("foo"), int64_t(1) }}))); } TEST(Filter, Has) { - ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"), - {{ std::string("foo"), int64_t(0) }})); - ASSERT_TRUE(evaluate(parse("[\"has\", \"foo\"]"), - {{ std::string("foo"), false }})); - ASSERT_FALSE(evaluate(parse("[\"has\", \"foo\"]"), - {{}})); + ASSERT_TRUE(parse("[\"has\", \"foo\"]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_TRUE(parse("[\"has\", \"foo\"]")( + feature({{ std::string("foo"), int64_t(0) }}))); + ASSERT_TRUE(parse("[\"has\", \"foo\"]")( + feature({{ std::string("foo"), false }}))); + ASSERT_FALSE(parse("[\"has\", \"foo\"]")( + feature({{}}))); } TEST(Filter, NotHas) { - ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"), - {{ std::string("foo"), int64_t(1) }})); - ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"), - {{ std::string("foo"), int64_t(0) }})); - ASSERT_FALSE(evaluate(parse("[\"!has\", \"foo\"]"), - {{ std::string("foo"), false }})); - ASSERT_TRUE(evaluate(parse("[\"!has\", \"foo\"]"), - {{}})); + ASSERT_FALSE(parse("[\"!has\", \"foo\"]")( + feature({{ std::string("foo"), int64_t(1) }}))); + ASSERT_FALSE(parse("[\"!has\", \"foo\"]")( + feature({{ std::string("foo"), int64_t(0) }}))); + ASSERT_FALSE(parse("[\"!has\", \"foo\"]")( + feature({{ std::string("foo"), false }}))); + ASSERT_TRUE(parse("[\"!has\", \"foo\"]")( + feature({{}}))); +} + +TEST(Filter, ID) { + Feature feature1 { Point() }; + feature1.id = { 1234 }; + + ASSERT_TRUE(parse("[\"==\", \"$id\", 1234]")(feature1)); + ASSERT_FALSE(parse("[\"==\", \"$id\", \"1234\"]")(feature1)); + + Feature feature2 { Point() }; + feature2.properties["id"] = { 1234 }; + + ASSERT_FALSE(parse("[\"==\", \"$id\", 1234]")(feature2)); } -- cgit v1.2.1