From a64fe5062f48b6aa4b98b382734856e1005480fb Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 3 May 2016 17:25:08 -0700 Subject: [PATCH] [core] Simplify FilterEvaluator and fix crash Previous implementation was assigning a temporary to FilterEvaluator::extractor. --- src/mbgl/renderer/symbol_bucket.cpp | 2 +- src/mbgl/style/filter_evaluator.hpp | 31 +++++++++++++--------- src/mbgl/style/style_bucket_parameters.cpp | 2 +- src/mbgl/tile/geometry_tile.cpp | 13 --------- src/mbgl/tile/geometry_tile.hpp | 11 -------- test/style/filter.cpp | 17 +++++++----- 6 files changed, 30 insertions(+), 46 deletions(-) delete mode 100644 src/mbgl/tile/geometry_tile.cpp diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp index 65539e491..e14bad673 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 for (GLsizei i = 0; i < featureCount; i++) { auto feature = layer.getFeature(i); - FilterEvaluator evaluator(*feature); + FilterEvaluator evaluator(*feature); if (!Filter::visit(filter, evaluator)) continue; diff --git a/src/mbgl/style/filter_evaluator.hpp b/src/mbgl/style/filter_evaluator.hpp index 0f3ee2a42..ab550ee26 100644 --- a/src/mbgl/style/filter_evaluator.hpp +++ b/src/mbgl/style/filter_evaluator.hpp @@ -1,54 +1,53 @@ #pragma once #include -#include +#include #include namespace mbgl { -template class FilterEvaluator { public: - FilterEvaluator(const Extractor& extractor_) - : extractor(extractor_) {} + FilterEvaluator(const GeometryTileFeature& feature_) + : feature(feature_) {} bool operator()(const NullFilter&) const { return true; } bool operator()(const EqualsFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return actual && equal(*actual, filter.value); } bool operator()(const NotEqualsFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return !actual || !equal(*actual, filter.value); } bool operator()(const LessThanFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ < rhs_; }); } bool operator()(const LessThanEqualsFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ <= rhs_; }); } bool operator()(const GreaterThanFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ > rhs_; }); } bool operator()(const GreaterThanEqualsFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); return actual && compare(*actual, filter.value, [] (const auto& lhs_, const auto& rhs_) { return lhs_ >= rhs_; }); } bool operator()(const InFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); if (!actual) return false; for (const auto& v: filter.values) { @@ -60,7 +59,7 @@ public: } bool operator()(const NotInFilter& filter) const { - optional actual = extractor.getValue(filter.key); + optional actual = getValue(filter.key); if (!actual) return true; for (const auto& v: filter.values) { @@ -99,6 +98,12 @@ public: } private: + optional getValue(const std::string& key) const { + return key == "$type" + ? optional(uint64_t(feature.getType())) + : feature.getValue(key); + } + template struct Comparator { const Op& op; @@ -142,7 +147,7 @@ private: return compare(lhs, rhs, [] (const auto& lhs_, const auto& rhs_) { return lhs_ == rhs_; }); } - const Extractor& extractor; + const GeometryTileFeature& feature; }; } // namespace mbgl diff --git a/src/mbgl/style/style_bucket_parameters.cpp b/src/mbgl/style/style_bucket_parameters.cpp index cf5184bd8..0b4b2affc 100644 --- a/src/mbgl/style/style_bucket_parameters.cpp +++ b/src/mbgl/style/style_bucket_parameters.cpp @@ -10,7 +10,7 @@ void StyleBucketParameters::eachFilteredFeature(const Filter& filter, for (std::size_t i = 0; !cancelled() && i < layer.featureCount(); i++) { auto feature = layer.getFeature(i); - FilterEvaluator evaluator(*feature); + FilterEvaluator evaluator(*feature); if (!Filter::visit(filter, evaluator)) continue; diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp deleted file mode 100644 index e3c7437a9..000000000 --- a/src/mbgl/tile/geometry_tile.cpp +++ /dev/null @@ -1,13 +0,0 @@ -#include - -namespace mbgl { - -optional GeometryTileFeatureExtractor::getValue(const std::string& key) const { - if (key == "$type") { - return Value(uint64_t(feature.getType())); - } - - return feature.getValue(key); -} - -} // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 0bcb92b47..0b01a230c 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -80,17 +80,6 @@ public: virtual std::unique_ptr monitorTile(const Callback&) = 0; }; -class GeometryTileFeatureExtractor { -public: - GeometryTileFeatureExtractor(const GeometryTileFeature& feature_) - : feature(feature_) {} - - optional getValue(const std::string& key) const; - -private: - const GeometryTileFeature& feature; -}; - } // namespace mbgl #endif diff --git a/test/style/filter.cpp b/test/style/filter.cpp index 4d6963267..da0e5ff4a 100644 --- a/test/style/filter.cpp +++ b/test/style/filter.cpp @@ -13,26 +13,28 @@ using namespace mbgl; typedef std::multimap Properties; -class Extractor { +class StubFeature : public GeometryTileFeature { public: - inline Extractor(const Properties& properties_, FeatureType type_) + inline StubFeature(const Properties& properties_, FeatureType type_) : properties(properties_) , type(type_) {} - optional getValue(const std::string &key) const { - if (key == "$type") - return Value(uint64_t(type)); + optional getValue(const std::string &key) const override { auto it = properties.find(key); if (it == properties.end()) return optional(); return it->second; } - FeatureType getType() const { + FeatureType getType() const override { return type; } + GeometryCollection getGeometries() const override { + return GeometryCollection(); + } + private: const Properties properties; FeatureType type; @@ -45,7 +47,8 @@ Filter parse(const char * expression) { } bool evaluate(const Filter& filter, const Properties& properties, FeatureType type = FeatureType::Unknown) { - FilterEvaluator evaluator({properties, type}); + StubFeature feature(properties, type); + FilterEvaluator evaluator(feature); return Filter::visit(filter, evaluator); }