From 25678a4d997602d591ea5b691338419be3675f14 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Thu, 7 Mar 2024 10:36:50 -0800 Subject: [PATCH] Track expression dependencies (#2113) --- .../mbgl/style/color_ramp_property_value.hpp | 6 +- include/mbgl/style/expression/at.hpp | 2 +- .../style/expression/boolean_operator.hpp | 4 +- include/mbgl/style/expression/case.hpp | 8 +- include/mbgl/style/expression/coalesce.hpp | 2 +- include/mbgl/style/expression/coercion.hpp | 2 +- .../style/expression/collator_expression.hpp | 2 +- include/mbgl/style/expression/error.hpp | 2 +- include/mbgl/style/expression/expression.hpp | 109 ++++-- .../mbgl/style/expression/find_zoom_curve.hpp | 7 +- .../style/expression/format_expression.hpp | 17 +- .../expression/format_section_override.hpp | 4 +- include/mbgl/style/expression/let.hpp | 4 +- include/mbgl/style/expression/literal.hpp | 4 +- include/mbgl/style/expression/match.hpp | 2 +- include/mbgl/style/layer.hpp | 6 + include/mbgl/style/layer_properties.hpp | 6 +- include/mbgl/style/property_expression.hpp | 16 +- include/mbgl/style/property_value.hpp | 7 + .../ios/benchmark/MBXBenchViewController.mm | 8 +- platform/ios/test/core/iosTestRunner.mm | 1 - src/mbgl/map/map_impl.cpp | 37 +++ .../data_driven_property_evaluator.hpp | 2 +- .../layers/render_background_layer.cpp | 4 +- .../renderer/layers/render_circle_layer.cpp | 4 +- .../layers/render_fill_extrusion_layer.cpp | 4 +- .../renderer/layers/render_fill_layer.cpp | 4 +- .../renderer/layers/render_heatmap_layer.cpp | 1 + .../layers/render_hillshade_layer.cpp | 4 +- .../renderer/layers/render_line_layer.cpp | 4 +- .../renderer/layers/render_raster_layer.cpp | 4 +- .../renderer/layers/render_symbol_layer.cpp | 4 +- .../possibly_evaluated_property_value.hpp | 14 + src/mbgl/renderer/render_layer.hpp | 5 + src/mbgl/renderer/render_orchestrator.cpp | 10 +- src/mbgl/style/expression/assertion.cpp | 2 +- src/mbgl/style/expression/coercion.cpp | 55 +-- .../style/expression/collator_expression.cpp | 25 +- src/mbgl/style/expression/comparison.cpp | 4 +- .../style/expression/compound_expression.cpp | 313 +++++++++++------- src/mbgl/style/expression/distance.cpp | 2 +- src/mbgl/style/expression/expression.cpp | 40 +++ src/mbgl/style/expression/find_zoom_curve.cpp | 18 +- .../style/expression/format_expression.cpp | 45 ++- .../style/expression/image_expression.cpp | 2 +- src/mbgl/style/expression/in.cpp | 2 +- src/mbgl/style/expression/index_of.cpp | 2 +- src/mbgl/style/expression/interpolate.cpp | 2 +- src/mbgl/style/expression/length.cpp | 2 +- src/mbgl/style/expression/number_format.cpp | 5 +- src/mbgl/style/expression/slice.cpp | 2 +- src/mbgl/style/expression/step.cpp | 2 +- src/mbgl/style/expression/within.cpp | 2 +- src/mbgl/style/layer.cpp | 5 + src/mbgl/style/layer_impl.hpp | 3 + .../layers/background_layer_properties.cpp | 4 + .../layers/background_layer_properties.hpp | 2 + .../style/layers/circle_layer_properties.cpp | 4 + .../style/layers/circle_layer_properties.hpp | 2 + .../layers/custom_drawable_layer_impl.hpp | 2 + src/mbgl/style/layers/custom_layer_impl.hpp | 2 + .../fill_extrusion_layer_properties.cpp | 4 + .../fill_extrusion_layer_properties.hpp | 2 + .../style/layers/fill_layer_properties.cpp | 4 + .../style/layers/fill_layer_properties.hpp | 2 + .../style/layers/heatmap_layer_properties.cpp | 4 + .../style/layers/heatmap_layer_properties.hpp | 2 + .../layers/hillshade_layer_properties.cpp | 4 + .../layers/hillshade_layer_properties.hpp | 2 + .../style/layers/layer_properties.cpp.ejs | 4 + .../style/layers/layer_properties.hpp.ejs | 2 + src/mbgl/style/layers/line_layer_impl.hpp | 4 + .../style/layers/line_layer_properties.cpp | 4 + .../style/layers/line_layer_properties.hpp | 2 + .../location_indicator_layer_properties.cpp | 4 + .../location_indicator_layer_properties.hpp | 2 + .../style/layers/raster_layer_properties.cpp | 4 + .../style/layers/raster_layer_properties.hpp | 2 + .../style/layers/symbol_layer_properties.cpp | 4 + .../style/layers/symbol_layer_properties.hpp | 2 + src/mbgl/style/properties.hpp | 67 +++- src/mbgl/style/property_expression.cpp | 21 +- test/CMakeLists.txt | 1 + .../expression_equality/collator1.a.json | 1 + .../expression_equality/collator1.b.json | 1 + .../expression_equality/collator2.a.json | 1 + .../expression_equality/collator2.b.json | 1 + .../expression_equality/collator3.a.json | 1 + .../expression_equality/collator3.b.json | 1 + .../expression_equality/collator4.a.json | 1 + .../expression_equality/collator4.b.json | 1 + .../expression_equality/collator5.a.json | 1 + .../expression_equality/collator5.b.json | 1 + .../style_parser/expressions.info.json | 2 +- .../style_parser/expressions.style.json | 18 + test/src/mbgl/test/fixture_log_observer.cpp | 7 +- test/src/mbgl/test/fixture_log_observer.hpp | 1 + test/style/conversion/function.test.cpp | 51 +-- test/style/expression/dependency.test.cpp | 79 +++++ test/style/expression/expression.test.cpp | 12 +- test/style/filter.test.cpp | 132 +++++++- test/style/property_expression.test.cpp | 45 ++- test/style/style.test.cpp | 6 +- test/style/style_layer.test.cpp | 4 +- test/style/style_parser.test.cpp | 38 ++- 105 files changed, 1082 insertions(+), 339 deletions(-) create mode 100644 test/fixtures/expression_equality/collator1.a.json create mode 100644 test/fixtures/expression_equality/collator1.b.json create mode 100644 test/fixtures/expression_equality/collator2.a.json create mode 100644 test/fixtures/expression_equality/collator2.b.json create mode 100644 test/fixtures/expression_equality/collator3.a.json create mode 100644 test/fixtures/expression_equality/collator3.b.json create mode 100644 test/fixtures/expression_equality/collator4.a.json create mode 100644 test/fixtures/expression_equality/collator4.b.json create mode 100644 test/fixtures/expression_equality/collator5.a.json create mode 100644 test/fixtures/expression_equality/collator5.b.json create mode 100644 test/style/expression/dependency.test.cpp diff --git a/include/mbgl/style/color_ramp_property_value.hpp b/include/mbgl/style/color_ramp_property_value.hpp index 192ff2bb2..ebcc3f68c 100644 --- a/include/mbgl/style/color_ramp_property_value.hpp +++ b/include/mbgl/style/color_ramp_property_value.hpp @@ -24,8 +24,7 @@ private: } public: - ColorRampPropertyValue() - : value(nullptr) {} + ColorRampPropertyValue() noexcept = default; ColorRampPropertyValue(std::shared_ptr value_) : value(std::move(value_)) {} @@ -46,6 +45,9 @@ public: bool hasDataDrivenPropertyDifference(const ColorRampPropertyValue&) const { return false; } const expression::Expression& getExpression() const { return *value; } + + using Dependency = style::expression::Dependency; + Dependency getDependencies() const noexcept { return value ? value->dependencies : Dependency::None; } }; } // namespace style diff --git a/include/mbgl/style/expression/at.hpp b/include/mbgl/style/expression/at.hpp index ab6a6247f..58fa54b67 100644 --- a/include/mbgl/style/expression/at.hpp +++ b/include/mbgl/style/expression/at.hpp @@ -11,7 +11,7 @@ namespace expression { class At : public Expression { public: At(std::unique_ptr index_, std::unique_ptr input_) - : Expression(Kind::At, input_->getType().get().itemType), + : Expression(Kind::At, input_->getType().get().itemType, depsOf(index_) | depsOf(input_)), index(std::move(index_)), input(std::move(input_)) {} diff --git a/include/mbgl/style/expression/boolean_operator.hpp b/include/mbgl/style/expression/boolean_operator.hpp index 442de3d27..5c231397c 100644 --- a/include/mbgl/style/expression/boolean_operator.hpp +++ b/include/mbgl/style/expression/boolean_operator.hpp @@ -12,7 +12,7 @@ namespace expression { class Any : public Expression { public: Any(std::vector> inputs_) - : Expression(Kind::Any, type::Boolean), + : Expression(Kind::Any, type::Boolean, collectDependencies(inputs_)), inputs(std::move(inputs_)) {} static ParseResult parse(const mbgl::style::conversion::Convertible& value, ParsingContext& ctx); @@ -31,7 +31,7 @@ private: class All : public Expression { public: All(std::vector> inputs_) - : Expression(Kind::All, type::Boolean), + : Expression(Kind::All, type::Boolean, collectDependencies(inputs_)), inputs(std::move(inputs_)) {} static ParseResult parse(const mbgl::style::conversion::Convertible& value, ParsingContext& ctx); diff --git a/include/mbgl/style/expression/case.hpp b/include/mbgl/style/expression/case.hpp index 93a4edc87..d381825be 100644 --- a/include/mbgl/style/expression/case.hpp +++ b/include/mbgl/style/expression/case.hpp @@ -17,7 +17,7 @@ public: using Branch = std::pair, std::unique_ptr>; Case(type::Type type_, std::vector branches_, std::unique_ptr otherwise_) - : Expression(Kind::Case, std::move(type_)), + : Expression(Kind::Case, std::move(type_), collectDependencies(branches_) | depsOf(otherwise_)), branches(std::move(branches_)), otherwise(std::move(otherwise_)) {} @@ -32,6 +32,12 @@ public: std::string getOperator() const override { return "case"; } +protected: + using Expression::collectDependencies; + static Dependency collectDependencies(const std::vector& branches) { + return collectDependencies(branches, [](const Branch& v) { return depsOf(v.first) | depsOf(v.second); }); + } + private: std::vector branches; std::unique_ptr otherwise; diff --git a/include/mbgl/style/expression/coalesce.hpp b/include/mbgl/style/expression/coalesce.hpp index 044ba7b16..bb2475b00 100644 --- a/include/mbgl/style/expression/coalesce.hpp +++ b/include/mbgl/style/expression/coalesce.hpp @@ -15,7 +15,7 @@ class Coalesce : public Expression { public: using Args = std::vector>; Coalesce(const type::Type& type_, Args args_) - : Expression(Kind::Coalesce, type_), + : Expression(Kind::Coalesce, type_, collectDependencies(args_)), args(std::move(args_)) {} static ParseResult parse(const mbgl::style::conversion::Convertible& value, ParsingContext& ctx); diff --git a/include/mbgl/style/expression/coercion.hpp b/include/mbgl/style/expression/coercion.hpp index 124a1176b..14ca5d036 100644 --- a/include/mbgl/style/expression/coercion.hpp +++ b/include/mbgl/style/expression/coercion.hpp @@ -12,7 +12,7 @@ namespace expression { class Coercion : public Expression { public: - Coercion(type::Type type_, std::vector> inputs_); + Coercion(const type::Type& type_, std::vector> inputs_); static ParseResult parse(const mbgl::style::conversion::Convertible& value, ParsingContext& ctx); diff --git a/include/mbgl/style/expression/collator_expression.hpp b/include/mbgl/style/expression/collator_expression.hpp index 3b51f32ea..390d8f2ae 100644 --- a/include/mbgl/style/expression/collator_expression.hpp +++ b/include/mbgl/style/expression/collator_expression.hpp @@ -21,7 +21,7 @@ public: void eachChild(const std::function&) const override; - bool operator==(const Expression& e) const override; + bool operator==(const Expression&) const override; std::vector> possibleOutputs() const override { // Technically the set of possible outputs is the combinatoric set of diff --git a/include/mbgl/style/expression/error.hpp b/include/mbgl/style/expression/error.hpp index bcb6e7397..b1385b420 100644 --- a/include/mbgl/style/expression/error.hpp +++ b/include/mbgl/style/expression/error.hpp @@ -11,7 +11,7 @@ namespace expression { class Error : public Expression { public: Error(std::string message_) - : Expression(Kind::Error, type::Error), + : Expression(Kind::Error, type::Error, Dependency::None), message(std::move(message_)) {} void eachChild(const std::function&) const override {} diff --git a/include/mbgl/style/expression/expression.hpp b/include/mbgl/style/expression/expression.hpp index 99da94e1e..7a7fe00c6 100644 --- a/include/mbgl/style/expression/expression.hpp +++ b/include/mbgl/style/expression/expression.hpp @@ -5,12 +5,14 @@ #include #include #include +#include #include #include -#include #include +#include #include +#include namespace mbgl { @@ -135,23 +137,23 @@ public: }; /** - Expression is an abstract class that serves as an interface and base class - for particular expression implementations. + Expression is an abstract class that serves as an interface and base class + for particular expression implementations. - CompoundExpression implements the majority of expressions in the spec by - inferring the argument and output from a simple function (const T0& arg0, - const T1& arg1, ...) -> Result where T0, T1, ..., U are member types of - mbgl::style::expression::Value. + CompoundExpression implements the majority of expressions in the spec by + inferring the argument and output from a simple function (const T0& arg0, + const T1& arg1, ...) -> Result where T0, T1, ..., U are member types of + mbgl::style::expression::Value. - The other Expression subclasses (Let, Curve, Match, etc.) exist in order to - implement expressions that need specialized parsing, type checking, or - evaluation logic that can't be handled by CompoundExpression's inference - mechanism. + The other Expression subclasses (Let, Curve, Match, etc.) exist in order to + implement expressions that need specialized parsing, type checking, or + evaluation logic that can't be handled by CompoundExpression's inference + mechanism. - Each Expression subclass also provides a static - ParseResult ExpressionClass::parse(const V&, ParsingContext), - which handles parsing a style-spec JSON representation of the expression. -*/ + Each Expression subclass also provides a static + ParseResult ExpressionClass::parse(const V&, ParsingContext), + which handles parsing a style-spec JSON representation of the expression. + */ enum class Kind : int32_t { Coalesce, @@ -183,10 +185,33 @@ enum class Kind : int32_t { Slice }; +enum class Dependency : uint32_t { + None = 0, + Feature = 1 << 0, // Data reference + Image = 1 << 1, // Image reference (equivalent to not "runtime constant") + Zoom = 1 << 2, // Zoom level + Location = 1 << 3, // Not used yet, "distance-from-center" not supported + Bind = 1 << 4, // Create variable binding ("let") + Var = 1 << 5, // Use variable binding + Override = 1 << 6, // Property override + MaskCount = 7, +}; +inline constexpr static Dependency operator|(Dependency x, Dependency y) noexcept { + return Dependency{mbgl::underlying_type(x) | mbgl::underlying_type(y)}; +} +inline constexpr static Dependency operator&(Dependency x, Dependency y) noexcept { + return Dependency{mbgl::underlying_type(x) & mbgl::underlying_type(y)}; +} +inline static Dependency& operator|=(Dependency& target, Dependency source) noexcept { + target = target | source; + return target; +} + class Expression { public: - Expression(Kind kind_, type::Type type_) - : kind(kind_), + Expression(Kind kind_, type::Type type_, Dependency dependencies_) + : dependencies(dependencies_), + kind(kind_), type(std::move(type_)) {} virtual ~Expression() = default; @@ -195,8 +220,8 @@ public: virtual bool operator==(const Expression&) const = 0; bool operator!=(const Expression& rhs) const { return !operator==(rhs); } - Kind getKind() const { return kind; }; - type::Type getType() const { return type; }; + Kind getKind() const { return kind; } + const type::Type& getType() const { return type; } EvaluationResult evaluate(std::optional zoom, const Feature& feature, @@ -229,6 +254,8 @@ public: virtual std::string getOperator() const = 0; + const Dependency dependencies; + protected: template static bool childrenEqual(const T& lhs, const T& rhs) { @@ -261,6 +288,43 @@ protected: return *(lhs.first) == *(rhs.first) && *(lhs.second) == *(rhs.second); } + static Dependency depsOf(const std::shared_ptr& ex) { return ex ? ex->dependencies : Dependency::None; } + static Dependency depsOf(const std::unique_ptr& ex) { return ex ? ex->dependencies : Dependency::None; } + + template + static Dependency depsOf(const std::optional& ex) { + return ex ? depsOf(*ex) : Dependency::None; + } + + /// Combine the dependencies of all the expressions in a container of shared- or unique-pointers to expressions + template + static constexpr Dependency collectDependencies(const std::vector& container) { + return std::accumulate(container.begin(), container.end(), Dependency::None, [](auto a, const auto& ex) { + return a | depsOf(ex); + }); + } + + template + static constexpr Dependency collectDependencies(const std::vector& container, F op) { + return std::accumulate(container.begin(), container.end(), Dependency::None, [&](auto a, const auto& item) { + return a | op(item); + }); + } + + template + static constexpr Dependency collectDependencies(const std::map& container) { + return std::accumulate(container.begin(), container.end(), Dependency::None, [](auto a, const auto& v) { + return a | depsOf(v.second); + }); + } + + template + static constexpr Dependency collectDependencies(const std::unordered_map& container) { + return std::accumulate(container.begin(), container.end(), Dependency::None, [](auto a, const auto& kv) { + return a | depsOf(kv.second); + }); + } + private: Kind kind; type::Type type; @@ -268,4 +332,11 @@ private: } // namespace expression } // namespace style + +namespace util { +std::string toString(style::expression::Dependency); +} // namespace util + +std::ostream& operator<<(std::ostream&, style::expression::Dependency); + } // namespace mbgl diff --git a/include/mbgl/style/expression/find_zoom_curve.hpp b/include/mbgl/style/expression/find_zoom_curve.hpp index ff9e5984c..83fcc3686 100644 --- a/include/mbgl/style/expression/find_zoom_curve.hpp +++ b/include/mbgl/style/expression/find_zoom_curve.hpp @@ -1,9 +1,8 @@ #pragma once -#include #include +#include #include - #include #include @@ -12,9 +11,9 @@ namespace mbgl { namespace style { namespace expression { -std::optional> findZoomCurve(const expression::Expression* e); +std::optional> findZoomCurve(const expression::Expression*); -variant findZoomCurveChecked(const expression::Expression* e); +variant findZoomCurveChecked(const expression::Expression&); } // namespace expression } // namespace style diff --git a/include/mbgl/style/expression/format_expression.hpp b/include/mbgl/style/expression/format_expression.hpp index edbd3ffe3..fa2fe341b 100644 --- a/include/mbgl/style/expression/format_expression.hpp +++ b/include/mbgl/style/expression/format_expression.hpp @@ -10,17 +10,17 @@ namespace expression { struct FormatExpressionSection { explicit FormatExpressionSection(std::unique_ptr content_); - void setTextSectionOptions(std::optional> fontScale_, - std::optional> textFont_, - std::optional> textColor_); + void setTextSectionOptions(std::unique_ptr&& fontScale_, + std::unique_ptr&& textFont_, + std::unique_ptr&& textColor_); // Content can be expression that evaluates to String or Image. std::shared_ptr content; // Text related section options. - std::optional> fontScale; - std::optional> textFont; - std::optional> textColor; + std::shared_ptr fontScale; + std::shared_ptr textFont; + std::shared_ptr textColor; }; class FormatExpression final : public Expression { @@ -45,6 +45,11 @@ public: mbgl::Value serialize() const override; std::string getOperator() const override { return "format"; } +private: + static Dependency depsOfSection(const FormatExpressionSection& s) { + return depsOf(s.content) | depsOf(s.fontScale) | depsOf(s.textFont) | depsOf(s.textColor); + } + private: std::vector sections; }; diff --git a/include/mbgl/style/expression/format_section_override.hpp b/include/mbgl/style/expression/format_section_override.hpp index c7698b0da..f56a2a3e9 100644 --- a/include/mbgl/style/expression/format_section_override.hpp +++ b/include/mbgl/style/expression/format_section_override.hpp @@ -13,7 +13,9 @@ public: FormatSectionOverride(const type::Type& type_, PossiblyEvaluatedPropertyValue defaultValue_, std::string propertyName_) - : Expression(Kind::FormatSectionOverride, type_), + : Expression(Kind::FormatSectionOverride, + type_, + defaultValue_.getDependencies() | Dependency::Override | Dependency::Feature), defaultValue(std::move(defaultValue_)), propertyName(std::move(propertyName_)) {} diff --git a/include/mbgl/style/expression/let.hpp b/include/mbgl/style/expression/let.hpp index 5b37139df..89007a0fa 100644 --- a/include/mbgl/style/expression/let.hpp +++ b/include/mbgl/style/expression/let.hpp @@ -17,7 +17,7 @@ public: using Bindings = std::map>; Let(Bindings bindings_, std::unique_ptr result_) - : Expression(Kind::Let, result_->getType()), + : Expression(Kind::Let, result_->getType(), collectDependencies(bindings_) | Dependency::Bind), bindings(std::move(bindings_)), result(std::move(result_)) {} @@ -49,7 +49,7 @@ private: class Var : public Expression { public: Var(std::string name_, std::shared_ptr value_) - : Expression(Kind::Var, value_->getType()), + : Expression(Kind::Var, value_->getType(), depsOf(value_) | Dependency::Var), name(std::move(name_)), value(std::move(value_)) {} diff --git a/include/mbgl/style/expression/literal.hpp b/include/mbgl/style/expression/literal.hpp index 857f07cef..d8ca0d31a 100644 --- a/include/mbgl/style/expression/literal.hpp +++ b/include/mbgl/style/expression/literal.hpp @@ -13,11 +13,11 @@ namespace expression { class Literal : public Expression { public: Literal(const Value& value_) - : Expression(Kind::Literal, typeOf(value_)), + : Expression(Kind::Literal, typeOf(value_), Dependency::None), value(value_) {} Literal(const type::Array& type_, std::vector value_) - : Expression(Kind::Literal, type_), + : Expression(Kind::Literal, type_, Dependency::None), value(std::move(value_)) {} EvaluationResult evaluate(const EvaluationContext&) const override { return value; } diff --git a/include/mbgl/style/expression/match.hpp b/include/mbgl/style/expression/match.hpp index ed59705fc..431856252 100644 --- a/include/mbgl/style/expression/match.hpp +++ b/include/mbgl/style/expression/match.hpp @@ -19,7 +19,7 @@ public: std::unique_ptr input_, Branches branches_, std::unique_ptr otherwise_) - : Expression(Kind::Match, type_), + : Expression(Kind::Match, type_, depsOf(input_) | depsOf(otherwise_) | collectDependencies(branches_)), input(std::move(input_)), branches(std::move(branches_)), otherwise(std::move(otherwise_)) {} diff --git a/include/mbgl/style/layer.hpp b/include/mbgl/style/layer.hpp index cff9a68c0..a1d1f4b3e 100644 --- a/include/mbgl/style/layer.hpp +++ b/include/mbgl/style/layer.hpp @@ -16,6 +16,9 @@ namespace mbgl { namespace style { +namespace expression { +enum class Dependency : uint32_t; +} // namespace expression class LayerObserver; class Filter; @@ -159,6 +162,9 @@ public: mapbox::base::WeakPtr makeWeakPtr() { return weakFactory.makeWeakPtr(); } + /// Collect dependencies + expression::Dependency getDependencies() const noexcept; + protected: virtual Mutable mutableBaseImpl() const = 0; void serializeProperty(Value&, const StyleProperty&, const char* propertyName, bool isPaint) const; diff --git a/include/mbgl/style/layer_properties.hpp b/include/mbgl/style/layer_properties.hpp index c8e9688ae..60c6ef2ed 100644 --- a/include/mbgl/style/layer_properties.hpp +++ b/include/mbgl/style/layer_properties.hpp @@ -20,18 +20,20 @@ public: /// Contains render passes used by the renderer, see `mbgl::RenderPass`. uint8_t renderPasses = 0u; + virtual expression::Dependency getDependencies() const noexcept = 0; + protected: LayerProperties(Immutable impl) : baseImpl(std::move(impl)) {} }; template -inline const auto& getEvaluated(const Immutable& properties) { +inline const auto& getEvaluated(const Immutable& properties) noexcept { return static_cast(*properties).evaluated; } template -inline const auto& getCrossfade(const Immutable& properties) { +inline const auto& getCrossfade(const Immutable& properties) noexcept { return static_cast(*properties).crossfade; } diff --git a/include/mbgl/style/property_expression.hpp b/include/mbgl/style/property_expression.hpp index 3e2e7afeb..0c625f972 100644 --- a/include/mbgl/style/property_expression.hpp +++ b/include/mbgl/style/property_expression.hpp @@ -16,9 +16,9 @@ class PropertyExpressionBase { public: explicit PropertyExpressionBase(std::unique_ptr); - bool isZoomConstant() const noexcept; - bool isFeatureConstant() const noexcept; - bool isRuntimeConstant() const noexcept; + bool isZoomConstant() const noexcept { return isZoomConstant_; } + bool isFeatureConstant() const noexcept { return isFeatureConstant_; } + bool isRuntimeConstant() const noexcept { return isRuntimeConstant_; } float interpolationFactor(const Range&, float) const noexcept; Range getCoveringStops(float, float) const noexcept; const expression::Expression& getExpression() const noexcept; @@ -30,12 +30,20 @@ public: bool useIntegerZoom = false; + using Dependency = expression::Dependency; + Dependency getDependencies() const noexcept { + auto v = expression ? expression->dependencies : Dependency::None; + assert(isZoomConstant_ == !(underlying_type(v) & underlying_type(Dependency::Zoom))); + assert(isFeatureConstant_ == !(underlying_type(v) & underlying_type(Dependency::Feature))); + return v; + } + protected: std::shared_ptr expression; - variant zoomCurve; bool isZoomConstant_; bool isFeatureConstant_; bool isRuntimeConstant_; + variant zoomCurve; }; template diff --git a/include/mbgl/style/property_value.hpp b/include/mbgl/style/property_value.hpp index 3071e7d07..c8f7964aa 100644 --- a/include/mbgl/style/property_value.hpp +++ b/include/mbgl/style/property_value.hpp @@ -62,6 +62,13 @@ public: bool hasDataDrivenPropertyDifference(const PropertyValue& other) const { return *this != other && (isDataDriven() || other.isDataDriven()); } + + using Dependency = style::expression::Dependency; + Dependency getDependencies() const noexcept { + return value.match([](const Undefined&) noexcept { return Dependency::None; }, + [](const T&) noexcept { return Dependency::None; }, + [](const PropertyExpression& ex) noexcept { return ex.getDependencies(); }); + } }; } // namespace style diff --git a/platform/ios/benchmark/MBXBenchViewController.mm b/platform/ios/benchmark/MBXBenchViewController.mm index cc7218e45..411275e1b 100644 --- a/platform/ios/benchmark/MBXBenchViewController.mm +++ b/platform/ios/benchmark/MBXBenchViewController.mm @@ -248,14 +248,14 @@ namespace mbgl { double totalFrameEncodingTime = 0; double totalFrameRenderingTime = 0; for (const auto& row : result) { - NSLog(@"| %-*s | %4.1f ms | %4.1f ms |", int(colWidth), row.first.c_str(), 1e3 * row.second.first, 1e3 * row.second.second); + NSLog(@"| %-*s | %4.2f ms | %4.2f ms |", int(colWidth), row.first.c_str(), 1e3 * row.second.first, 1e3 * row.second.second); totalFrameEncodingTime += row.second.first; totalFrameRenderingTime += row.second.second; } - NSLog(@"Average frame encoding time: %4.1f ms", totalFrameEncodingTime * 1e3 / result.size()); - NSLog(@"Average frame rendering time: %4.1f ms", totalFrameRenderingTime * 1e3 / result.size()); - + NSLog(@"Average frame encoding time: %4.2f ms", totalFrameEncodingTime * 1e3 / result.size()); + NSLog(@"Average frame rendering time: %4.2f ms", totalFrameRenderingTime * 1e3 / result.size()); + // NSLog(@"Total uploads: %zu", mbgl::uploadCount); // NSLog(@"Total uploads with dirty vattr: %zu", mbgl::uploadVertextAttrsDirty); // NSLog(@"Total uploads with invalid segs: %zu", mbgl::uploadInvalidSegments); diff --git a/platform/ios/test/core/iosTestRunner.mm b/platform/ios/test/core/iosTestRunner.mm index a8b7f794b..bd1bb4798 100644 --- a/platform/ios/test/core/iosTestRunner.mm +++ b/platform/ios/test/core/iosTestRunner.mm @@ -23,7 +23,6 @@ self.testStatus = NO; self.runner = new TestRunner(); - NSError *error; NSFileManager *fileManager = [NSFileManager defaultManager]; NSString *xcTestBundleRoot = [[NSBundle bundleForClass:[self class]] resourcePath]; diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index d9de1bb75..52ea688f6 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -4,9 +4,42 @@ #include #include #include +#include +#include namespace mbgl { +#if !defined(NDEBUG) +namespace { +void logStyleDependencies(EventSeverity severity, Event event, const style::Style& style) { + using Dependency = style::expression::Dependency; + constexpr auto maskCount = underlying_type(Dependency::MaskCount); + std::array counts = {0}; + const auto layers = style.getLayers(); + for (const auto& layer : layers) { + const auto deps = layer->getDependencies(); + if (deps == Dependency::None) { + counts[0]++; + } else { + for (size_t i = 0; i < maskCount; ++i) { + const auto mask = Dependency{1u << i}; + if ((deps & mask) == mask) { + counts[i + 1]++; + } + } + } + } + std::ostringstream ss; + ss << "Style '" << style.getName() << "' has " << layers.size() << " layers:\n"; + ss << " " << Dependency::None << ": " << counts[0] << "\n"; + for (size_t i = 0; i < maskCount; ++i) { + ss << " " << Dependency{1u << i} << ": " << counts[i + 1] << "\n"; + } + Log::Record(severity, event, ss.str()); +} +} // namespace +#endif + Map::Impl::Impl(RendererFrontend& frontend_, MapObserver& observer_, std::shared_ptr fileSource_, @@ -84,6 +117,10 @@ void Map::Impl::onStyleLoaded() { annotationManager.onStyleLoaded(); } observer.onDidFinishLoadingStyle(); + +#if !defined(NDEBUG) + logStyleDependencies(EventSeverity::Info, Event::Style, *style); +#endif } void Map::Impl::onStyleError(std::exception_ptr error) { diff --git a/src/mbgl/renderer/data_driven_property_evaluator.hpp b/src/mbgl/renderer/data_driven_property_evaluator.hpp index 25b8bc8d1..4202b126b 100644 --- a/src/mbgl/renderer/data_driven_property_evaluator.hpp +++ b/src/mbgl/renderer/data_driven_property_evaluator.hpp @@ -23,7 +23,7 @@ public: ResultType operator()(const T& constant) const { return ResultType(constant); } ResultType operator()(const style::PropertyExpression& expression) const { - if (useIntegerZoom) { // Compiler will optimize out the unused branch. + if constexpr (useIntegerZoom) { // Compiler will optimize out the unused branch. if (!expression.isFeatureConstant() || !expression.isRuntimeConstant()) { auto returnExpression = expression; returnExpression.useIntegerZoom = true; diff --git a/src/mbgl/renderer/layers/render_background_layer.cpp b/src/mbgl/renderer/layers/render_background_layer.cpp index 9a83beabc..04a109149 100644 --- a/src/mbgl/renderer/layers/render_background_layer.cpp +++ b/src/mbgl/renderer/layers/render_background_layer.cpp @@ -42,7 +42,9 @@ inline const BackgroundLayer::Impl& impl_cast(const Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderBackgroundLayer::~RenderBackgroundLayer() = default; diff --git a/src/mbgl/renderer/layers/render_circle_layer.cpp b/src/mbgl/renderer/layers/render_circle_layer.cpp index 469e2d5a5..4f8b270b3 100644 --- a/src/mbgl/renderer/layers/render_circle_layer.cpp +++ b/src/mbgl/renderer/layers/render_circle_layer.cpp @@ -62,7 +62,9 @@ inline const style::CircleLayer::Impl& impl_cast(const Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} void RenderCircleLayer::transition(const TransitionParameters& parameters) { unevaluated = impl_cast(baseImpl).paint.transitioned(parameters, std::move(unevaluated)); diff --git a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp index 39325bfd2..0d96259d0 100644 --- a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp +++ b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp @@ -47,7 +47,9 @@ inline const FillExtrusionLayer::Impl& impl_cast(const Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderFillExtrusionLayer::~RenderFillExtrusionLayer() = default; diff --git a/src/mbgl/renderer/layers/render_fill_layer.cpp b/src/mbgl/renderer/layers/render_fill_layer.cpp index d921ddb6f..d6ac93769 100644 --- a/src/mbgl/renderer/layers/render_fill_layer.cpp +++ b/src/mbgl/renderer/layers/render_fill_layer.cpp @@ -59,7 +59,9 @@ inline const FillLayer::Impl& impl_cast(const Immutable& imp RenderFillLayer::RenderFillLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderFillLayer::~RenderFillLayer() = default; diff --git a/src/mbgl/renderer/layers/render_heatmap_layer.cpp b/src/mbgl/renderer/layers/render_heatmap_layer.cpp index 429e93436..355832870 100644 --- a/src/mbgl/renderer/layers/render_heatmap_layer.cpp +++ b/src/mbgl/renderer/layers/render_heatmap_layer.cpp @@ -42,6 +42,7 @@ inline const HeatmapLayer::Impl& impl_cast(const Immutable& impl) { RenderHeatmapLayer::RenderHeatmapLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); colorRamp = std::make_shared(Size(256, 1)); } diff --git a/src/mbgl/renderer/layers/render_hillshade_layer.cpp b/src/mbgl/renderer/layers/render_hillshade_layer.cpp index b5635cb5a..b499d68a5 100644 --- a/src/mbgl/renderer/layers/render_hillshade_layer.cpp +++ b/src/mbgl/renderer/layers/render_hillshade_layer.cpp @@ -43,7 +43,9 @@ inline const HillshadeLayer::Impl& impl_cast(const Immutable RenderHillshadeLayer::RenderHillshadeLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderHillshadeLayer::~RenderHillshadeLayer() = default; diff --git a/src/mbgl/renderer/layers/render_line_layer.cpp b/src/mbgl/renderer/layers/render_line_layer.cpp index d31051d74..19f377bed 100644 --- a/src/mbgl/renderer/layers/render_line_layer.cpp +++ b/src/mbgl/renderer/layers/render_line_layer.cpp @@ -55,7 +55,9 @@ const auto posNormalAttribName = "a_pos_normal"; RenderLineLayer::RenderLineLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), unevaluated(impl_cast(baseImpl).paint.untransitioned()), - colorRamp(std::make_shared(Size(256, 1))) {} + colorRamp(std::make_shared(Size(256, 1))) { + styleDependencies = unevaluated.getDependencies(); +} RenderLineLayer::~RenderLineLayer() = default; diff --git a/src/mbgl/renderer/layers/render_raster_layer.cpp b/src/mbgl/renderer/layers/render_raster_layer.cpp index 23058ef9d..00386b30f 100644 --- a/src/mbgl/renderer/layers/render_raster_layer.cpp +++ b/src/mbgl/renderer/layers/render_raster_layer.cpp @@ -39,7 +39,9 @@ inline const RasterLayer::Impl& impl_cast(const Immutable& i RenderRasterLayer::RenderRasterLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderRasterLayer::~RenderRasterLayer() = default; diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp index fbdd68840..0203d0ba0 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -358,7 +358,9 @@ inline const SymbolLayer::Impl& impl_cast(const Immutable& i RenderSymbolLayer::RenderSymbolLayer(Immutable _impl) : RenderLayer(makeMutable(std::move(_impl))), - unevaluated(impl_cast(baseImpl).paint.untransitioned()) {} + unevaluated(impl_cast(baseImpl).paint.untransitioned()) { + styleDependencies = unevaluated.getDependencies(); +} RenderSymbolLayer::~RenderSymbolLayer() = default; diff --git a/src/mbgl/renderer/possibly_evaluated_property_value.hpp b/src/mbgl/renderer/possibly_evaluated_property_value.hpp index 747b63532..5f34de402 100644 --- a/src/mbgl/renderer/possibly_evaluated_property_value.hpp +++ b/src/mbgl/renderer/possibly_evaluated_property_value.hpp @@ -57,6 +57,13 @@ public: return expression.evaluate(zoom, feature, featureState, defaultValue); }); } + + using Dependency = style::expression::Dependency; + Dependency getDependencies() const noexcept { + return value.match( + [](const T&) noexcept { return Dependency::None; }, + [](const style::PropertyExpression& expression) noexcept { return expression.getDependencies(); }); + } }; template @@ -105,6 +112,13 @@ public: } }); } + + using Dependency = style::expression::Dependency; + Dependency getDependencies() const noexcept { + return value.match( + [](const Faded&) noexcept { return Dependency::None; }, + [](const style::PropertyExpression& expression) noexcept { return expression.getDependencies(); }); + } }; namespace util { diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp index 84fb70d22..483f0f1e3 100644 --- a/src/mbgl/renderer/render_layer.hpp +++ b/src/mbgl/renderer/render_layer.hpp @@ -194,6 +194,9 @@ public: bool isLayerRenderable() const noexcept { return isRenderable; } #endif + using Dependency = style::expression::Dependency; + Dependency getStyleDependencies() const { return styleDependencies; } + protected: // Checks whether the current hardware can render this layer. If it can't, // we'll show a warning in the console to inform the developer. @@ -299,6 +302,8 @@ protected: // Current layer index as specified by the layerIndexChanged event int32_t layerIndex{0}; + Dependency styleDependencies = Dependency::None; + // Current renderable status as specified by the markLayerRenderable event bool isRenderable{false}; diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 990becf88..50924d4d9 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -300,8 +300,14 @@ std::unique_ptr RenderOrchestrator::createRenderTree( for (RenderLayer& layer : orderedLayers) { const std::string& id = layer.getID(); const bool layerAddedOrChanged = layerDiff.added.count(id) || layerDiff.changed.count(id); - if (layerAddedOrChanged || zoomChanged || layer.hasTransition() || layer.hasCrossfade()) { - auto previousMask = layer.evaluatedProperties->constantsMask(); + + // Only re-evaluate on change of zoom if the style has some reference to it + using Dependency = expression::Dependency; + const bool zoomChangedAndMatters = zoomChanged && !layerAddedOrChanged && + (layer.getStyleDependencies() & Dependency::Zoom) != Dependency::None; + + if (layerAddedOrChanged || zoomChangedAndMatters || layer.hasTransition() || layer.hasCrossfade()) { + const auto previousMask = layer.evaluatedProperties->constantsMask(); layer.evaluate(evaluationParameters); if (previousMask != layer.evaluatedProperties->constantsMask()) { constantsMaskChanged.insert(id); diff --git a/src/mbgl/style/expression/assertion.cpp b/src/mbgl/style/expression/assertion.cpp index 3dfa71ddf..939ebb366 100644 --- a/src/mbgl/style/expression/assertion.cpp +++ b/src/mbgl/style/expression/assertion.cpp @@ -10,7 +10,7 @@ namespace expression { using namespace mbgl::style::conversion; Assertion::Assertion(type::Type type_, std::vector> inputs_) - : Expression(Kind::Assertion, std::move(type_)), + : Expression(Kind::Assertion, std::move(type_), collectDependencies(inputs_)), inputs(std::move(inputs_)) { assert(!inputs.empty()); } diff --git a/src/mbgl/style/expression/coercion.cpp b/src/mbgl/style/expression/coercion.cpp index 0236b8eef..0429d34fa 100644 --- a/src/mbgl/style/expression/coercion.cpp +++ b/src/mbgl/style/expression/coercion.cpp @@ -8,6 +8,10 @@ namespace mbgl { namespace style { namespace expression { +using CoerceFunction = EvaluationResult (*)(const Value&); + +namespace { + EvaluationResult toBoolean(const Value& v) { return v.match([&](double f) { return static_cast(f); }, [&](const std::string& s) { return s.length() > 0; }, @@ -76,28 +80,39 @@ EvaluationResult toImage(const Value& imageValue) { return Image(toString(imageValue).c_str()); } -Coercion::Coercion(type::Type type_, std::vector> inputs_) - : Expression(Kind::Coercion, std::move(type_)), +EvaluationResult toStringValue(const Value& imageValue) { + return toString(imageValue); +} + +CoerceFunction getCoerceFunction(const type::Type& t) { + if (t.is()) { + return toBoolean; + } else if (t.is()) { + return toColor; + } else if (t.is()) { + return toNumber; + } else if (t.is()) { + return toStringValue; + } else if (t.is()) { + return toFormatted; + } else if (t.is()) { + return toImage; + } + assert(false); + return toStringValue; +} + +Dependency extraDependency(const type::Type& t) { + return t.is() ? Dependency::Image : Dependency::None; +} + +} // namespace + +Coercion::Coercion(const type::Type& type_, std::vector> inputs_) + : Expression(Kind::Coercion, type_, collectDependencies(inputs_) | extraDependency(type_)), + coerceSingleValue(getCoerceFunction(getType())), inputs(std::move(inputs_)) { assert(!inputs.empty()); - type::Type t = getType(); - if (t.is()) { - coerceSingleValue = toBoolean; - } else if (t.is()) { - coerceSingleValue = toColor; - } else if (t.is()) { - coerceSingleValue = toNumber; - } else if (t.is()) { - coerceSingleValue = [](const Value& v) -> EvaluationResult { - return toString(v); - }; - } else if (t.is()) { - coerceSingleValue = toFormatted; - } else if (t.is()) { - coerceSingleValue = toImage; - } else { - assert(false); - } } mbgl::Value Coercion::serialize() const { diff --git a/src/mbgl/style/expression/collator_expression.cpp b/src/mbgl/style/expression/collator_expression.cpp index 397e2c8b0..9355cf45c 100644 --- a/src/mbgl/style/expression/collator_expression.cpp +++ b/src/mbgl/style/expression/collator_expression.cpp @@ -11,7 +11,9 @@ namespace expression { CollatorExpression::CollatorExpression(std::unique_ptr caseSensitive_, std::unique_ptr diacriticSensitive_, std::optional> locale_) - : Expression(Kind::CollatorExpression, type::Collator), + : Expression(Kind::CollatorExpression, + type::Collator, + depsOf(caseSensitive_) | depsOf(diacriticSensitive_) | depsOf(locale_)), caseSensitive(std::move(caseSensitive_)), diacriticSensitive(std::move(diacriticSensitive_)), locale(std::move(locale_)) {} @@ -56,7 +58,7 @@ ParseResult CollatorExpression::parse(const Convertible& value, ParsingContext& ParseResult locale; if (localeOption) { locale = ctx.parse(*localeOption, 1, {type::String}); - if (!locale) { + if (!locale || !*locale) { return ParseResult(); } } @@ -68,15 +70,17 @@ ParseResult CollatorExpression::parse(const Convertible& value, ParsingContext& void CollatorExpression::eachChild(const std::function& fn) const { fn(*caseSensitive); fn(*diacriticSensitive); - if (locale) { + if (locale && *locale) { fn(**locale); } } bool CollatorExpression::operator==(const Expression& e) const { if (e.getKind() == Kind::CollatorExpression) { - auto rhs = static_cast(&e); - if ((locale && (!rhs->locale || **locale != **(rhs->locale))) || (!locale && rhs->locale)) { + const auto* rhs = static_cast(&e); + const bool lLocSet = locale && *locale; + const bool rLocSet = rhs->locale && *rhs->locale; + if ((!lLocSet && rLocSet) || (lLocSet && (!rLocSet || **locale != **rhs->locale))) { return false; } return *caseSensitive == *(rhs->caseSensitive) && *diacriticSensitive == *(rhs->diacriticSensitive); @@ -88,7 +92,7 @@ mbgl::Value CollatorExpression::serialize() const { std::unordered_map options; options["case-sensitive"] = caseSensitive->serialize(); options["diacritic-sensitive"] = diacriticSensitive->serialize(); - if (locale) { + if (locale && *locale) { options["locale"] = (*locale)->serialize(); } return std::vector{{std::string("collator"), options}}; @@ -105,12 +109,13 @@ EvaluationResult CollatorExpression::evaluate(const EvaluationContext& params) c } if (locale) { - auto localeResult = (*locale)->evaluate(params); - if (!localeResult) { + if (auto localeResult = (*locale)->evaluate(params)) { + return Collator(caseSensitiveResult->get(), + diacriticSensitiveResult->get(), + localeResult->get()); + } else { return localeResult.error(); } - return Collator( - caseSensitiveResult->get(), diacriticSensitiveResult->get(), localeResult->get()); } else { return Collator(caseSensitiveResult->get(), diacriticSensitiveResult->get()); } diff --git a/src/mbgl/style/expression/comparison.cpp b/src/mbgl/style/expression/comparison.cpp index 38c12a443..393bdfd29 100644 --- a/src/mbgl/style/expression/comparison.cpp +++ b/src/mbgl/style/expression/comparison.cpp @@ -110,7 +110,7 @@ static CollatorComparison::CompareFunctionType getCollatorComparisonFunction(con } BasicComparison::BasicComparison(std::string op_, std::unique_ptr lhs_, std::unique_ptr rhs_) - : Expression(Kind::Comparison, type::Boolean), + : Expression(Kind::Comparison, type::Boolean, depsOf(lhs_) | depsOf(rhs_)), op(std::move(op_)), compare(getBasicCompareFunction(op)), lhs(std::move(lhs_)), @@ -168,7 +168,7 @@ CollatorComparison::CollatorComparison(std::string op_, std::unique_ptr lhs_, std::unique_ptr rhs_, std::unique_ptr collator_) - : Expression(Kind::Comparison, type::Boolean), + : Expression(Kind::Comparison, type::Boolean, depsOf(lhs_) | depsOf(rhs_) | depsOf(collator_)), op(std::move(op_)), compare(getCollatorComparisonFunction(op)), lhs(std::move(lhs_)), diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp index e1430979a..56909a85a 100644 --- a/src/mbgl/style/expression/compound_expression.cpp +++ b/src/mbgl/style/expression/compound_expression.cpp @@ -47,17 +47,22 @@ namespace detail { struct SignatureBase { using Args = std::vector>; - SignatureBase(type::Type result_, variant, VarargsType> params_, std::string name_) + SignatureBase(type::Type result_, + variant, VarargsType> params_, + std::string name_, + Dependency dependencies_) : result(std::move(result_)), params(std::move(params_)), - name(std::move(name_)) {} + name(std::move(name_)), + dependencies(dependencies_) {} virtual ~SignatureBase() = default; virtual EvaluationResult apply(const EvaluationContext&, const Args&) const = 0; - type::Type result; - variant, VarargsType> params; - std::string name; + const type::Type result; + const variant, VarargsType> params; + const std::string name; + const Dependency dependencies; }; /* @@ -87,10 +92,11 @@ struct Signature; // Simple evaluate function (const T0&, const T1&, ...) -> Result template struct Signature : SignatureBase { - Signature(R (*evaluate_)(Params...), const std::string& name_) + Signature(R (*evaluate_)(Params...), const std::string& name_, Dependency dependencies_) : SignatureBase(valueTypeToExpressionType>(), std::vector{valueTypeToExpressionType>()...}, - name_), + name_, + dependencies_), evaluate(evaluate_) {} EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { @@ -119,10 +125,11 @@ private: // Varargs evaluate function (const Varargs&) -> Result template struct Signature&)> : SignatureBase { - Signature(R (*evaluate_)(const Varargs&), const std::string& name_) + Signature(R (*evaluate_)(const Varargs&), const std::string& name_, Dependency dependencies_) : SignatureBase(valueTypeToExpressionType>(), VarargsType{valueTypeToExpressionType()}, - name_), + name_, + dependencies_), evaluate(evaluate_) {} EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { @@ -145,10 +152,11 @@ struct Signature&)> : SignatureBase { // (const EvaluationParams&, const T0&, const T1&, ...) -> Result template struct Signature : SignatureBase { - Signature(R (*evaluate_)(const EvaluationContext&, Params...), const std::string& name_) + Signature(R (*evaluate_)(const EvaluationContext&, Params...), const std::string& name_, Dependency dependencies_) : SignatureBase(valueTypeToExpressionType>(), std::vector{valueTypeToExpressionType>()...}, - name_), + name_, + dependencies_), evaluate(evaluate_) {} EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { @@ -178,10 +186,13 @@ private: // (const EvaluationContext&, const Varargs&) -> Result template struct Signature&)> : SignatureBase { - Signature(R (*evaluate_)(const EvaluationContext&, const Varargs&), const std::string& name_) + Signature(R (*evaluate_)(const EvaluationContext&, const Varargs&), + const std::string& name_, + Dependency dependencies_) : SignatureBase(valueTypeToExpressionType>(), VarargsType{valueTypeToExpressionType()}, - name_), + name_, + dependencies_), evaluate(evaluate_) {} EvaluationResult apply(const EvaluationContext& evaluationParameters, const Args& args) const override { @@ -215,8 +226,11 @@ struct SignatureType>> { }; template -static std::unique_ptr makeSignature(std::string name, Fn evaluateFunction) { - return std::make_unique::Type>>(evaluateFunction, std::move(name)); +static std::unique_ptr makeSignature(std::string name, + Fn evaluateFunction, + Dependency dependencies = Dependency::None) { + return std::make_unique::Type>>( + evaluateFunction, std::move(name), dependencies); } } // namespace detail @@ -322,14 +336,17 @@ const auto& rgbCompoundExpression() { } const auto& zoomCompoundExpression() { - static auto signature = detail::makeSignature("zoom", [](const EvaluationContext& params) -> Result { - if (!params.zoom) { - return EvaluationError{ - "The 'zoom' expression is unavailable in the current " - "evaluation context."}; - } - return *(params.zoom); - }); + static auto signature = detail::makeSignature( + "zoom", + [](const EvaluationContext& params) -> Result { + if (!params.zoom) { + return EvaluationError{ + "The 'zoom' expression is unavailable in the current " + "evaluation context."}; + } + return *(params.zoom); + }, + Dependency::Zoom); return signature; } @@ -374,7 +391,8 @@ const auto& accumulatedCompoundExpression() { const auto& hasContextCompoundExpression() { static auto signature = detail::makeSignature( - "has", [](const EvaluationContext& params, const std::string& key) -> Result { + "has", + [](const EvaluationContext& params, const std::string& key) -> Result { if (!params.feature) { return EvaluationError{ "Feature data is unavailable in the current evaluation " @@ -382,21 +400,25 @@ const auto& hasContextCompoundExpression() { } return static_cast(params.feature->getValue(key)); - }); + }, + Dependency::Feature); return signature; } const auto& hasObjectCompoundExpression() { static auto signature = detail::makeSignature( - "has", [](const std::string& key, const std::unordered_map& object) -> Result { + "has", + [](const std::string& key, const std::unordered_map& object) -> Result { return object.find(key) != object.end(); - }); + }, + Dependency::Feature); return signature; } const auto& getContextCompoundExpression() { static auto signature = detail::makeSignature( - "get", [](const EvaluationContext& params, const std::string& key) -> Result { + "get", + [](const EvaluationContext& params, const std::string& key) -> Result { if (!params.feature) { return EvaluationError{ "Feature data is unavailable in the current evaluation " @@ -408,28 +430,30 @@ const auto& getContextCompoundExpression() { return Null; } return Value(toExpressionValue(*propertyValue)); - }); + }, + Dependency::Feature); return signature; } const auto& getObjectCompoundExpression() { static auto signature = detail::makeSignature( - "get", [](const std::string& key, const std::unordered_map& object) -> Result { + "get", + [](const std::string& key, const std::unordered_map& object) -> Result { if (object.find(key) == object.end()) { return Null; } return object.at(key); - }); + }, + Dependency::Feature); return signature; } const auto& propertiesCompoundExpression() { static auto signature = detail::makeSignature( - "properties", [](const EvaluationContext& params) -> Result> { + "properties", + [](const EvaluationContext& params) -> Result> { if (!params.feature) { - return EvaluationError{ - "Feature data is unavailable in the current evaluation " - "context."}; + return EvaluationError{"Feature data is unavailable in the current evaluation context."}; } std::unordered_map result; const PropertyMap& properties = params.feature->getProperties(); @@ -438,44 +462,50 @@ const auto& propertiesCompoundExpression() { result[entry.first] = toExpressionValue(entry.second); } return result; - }); + }, + Dependency::Feature); return signature; } const auto& geometryTypeCompoundExpression() { - static auto signature = detail::makeSignature("geometry-type", - [](const EvaluationContext& params) -> Result { - if (!params.feature) { - return EvaluationError{ - "Feature data is unavailable in the current evaluation " - "context."}; - } + static auto signature = detail::makeSignature( + "geometry-type", + [](const EvaluationContext& params) -> Result { + if (!params.feature) { + return EvaluationError{ + "Feature data is unavailable in the current evaluation " + "context."}; + } - auto type = params.feature->getType(); - if (type == FeatureType::Point) { - return "Point"; - } else if (type == FeatureType::LineString) { - return "LineString"; - } else if (type == FeatureType::Polygon) { - return "Polygon"; - } else { - return "Unknown"; - } - }); + auto type = params.feature->getType(); + if (type == FeatureType::Point) { + return "Point"; + } else if (type == FeatureType::LineString) { + return "LineString"; + } else if (type == FeatureType::Polygon) { + return "Polygon"; + } else { + return "Unknown"; + } + }, + Dependency::Feature); return signature; } const auto& idCompoundExpression() { - static auto signature = detail::makeSignature("id", [](const EvaluationContext& params) -> Result { - if (!params.feature) { - return EvaluationError{ - "Feature data is unavailable in the current evaluation " - "context."}; - } + static auto signature = detail::makeSignature( + "id", + [](const EvaluationContext& params) -> Result { + if (!params.feature) { + return EvaluationError{ + "Feature data is unavailable in the current evaluation " + "context."}; + } - auto id = params.feature->getID(); - return id.match([](const auto& idValue) { return toExpressionValue(mbgl::Value(idValue)); }); - }); + auto id = params.feature->getID(); + return id.match([](const auto& idValue) { return toExpressionValue(mbgl::Value(idValue)); }); + }, + Dependency::Feature); return signature; } @@ -675,7 +705,8 @@ const auto& errorCompoundExpression() { const auto& featureStateCompoundExpression() { static auto signature = detail::makeSignature( - "feature-state", [](const EvaluationContext& params, const std::string& key) -> Result { + "feature-state", + [](const EvaluationContext& params, const std::string& key) -> Result { mbgl::Value state; if (params.featureState != nullptr) { auto it = params.featureState->find(key); @@ -684,43 +715,52 @@ const auto& featureStateCompoundExpression() { } } return toExpressionValue(state); - }); + }, + Dependency::Feature); return signature; } // Legacy Filters const auto& filterEqualsCompoundExpression() { static auto signature = detail::makeSignature( - "filter-==", [](const EvaluationContext& params, const std::string& key, const Value& lhs) -> Result { + "filter-==", + [](const EvaluationContext& params, const std::string& key, const Value& lhs) -> Result { const auto rhs = featurePropertyAsExpressionValue(params, key); return rhs ? lhs == *rhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdEqualsCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id-==", [](const EvaluationContext& params, const Value& lhs) -> Result { + "filter-id-==", + [](const EvaluationContext& params, const Value& lhs) -> Result { return lhs == featureIdAsExpressionValue(params); - }); + }, + Dependency::Feature); return signature; } const auto& filterTypeEqualsCompoundExpression() { static auto signature = detail::makeSignature( - "filter-type-==", [](const EvaluationContext& params, const std::string& lhs) -> Result { + "filter-type-==", + [](const EvaluationContext& params, const std::string& lhs) -> Result { if (!params.feature) return false; return featureTypeAsString(params.feature->getType()) == lhs; - }); + }, + Dependency::Feature); return signature; } const auto& filterLessThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter-<", [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { + "filter-<", + [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { auto rhs = featurePropertyAsDouble(params, key); return rhs ? rhs < lhs : false; - }); + }, + Dependency::Feature); return signature; } @@ -730,34 +770,41 @@ const auto& filterLessThanStringCompoundExpression() { [](const EvaluationContext& params, const std::string& key, const std::string& lhs) -> Result { auto rhs = featurePropertyAsString(params, key); return rhs ? rhs < lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdLessThanNumberCompoundExpression() { - static auto signature = detail::makeSignature("filter-id-<", - [](const EvaluationContext& params, double lhs) -> Result { - auto rhs = featureIdAsDouble(params); - return rhs ? rhs < lhs : false; - }); + static auto signature = detail::makeSignature( + "filter-id-<", + [](const EvaluationContext& params, double lhs) -> Result { + auto rhs = featureIdAsDouble(params); + return rhs ? rhs < lhs : false; + }, + Dependency::Feature); return signature; } const auto& filterIdLessThanStringCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id-<", [](const EvaluationContext& params, const std::string& lhs) -> Result { + "filter-id-<", + [](const EvaluationContext& params, const std::string& lhs) -> Result { auto rhs = featureIdAsString(params); return rhs ? rhs < lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterMoreThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter->", [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { + "filter->", + [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { auto rhs = featurePropertyAsDouble(params, key); return rhs ? rhs > lhs : false; - }); + }, + Dependency::Feature); return signature; } @@ -767,34 +814,41 @@ const auto& filterMoreThanStringCompoundExpression() { [](const EvaluationContext& params, const std::string& key, const std::string& lhs) -> Result { auto rhs = featurePropertyAsString(params, key); return rhs ? rhs > lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdMoreThanNumberCompoundExpression() { - static auto signature = detail::makeSignature("filter-id->", - [](const EvaluationContext& params, double lhs) -> Result { - auto rhs = featureIdAsDouble(params); - return rhs ? rhs > lhs : false; - }); + static auto signature = detail::makeSignature( + "filter-id->", + [](const EvaluationContext& params, double lhs) -> Result { + auto rhs = featureIdAsDouble(params); + return rhs ? rhs > lhs : false; + }, + Dependency::Feature); return signature; } const auto& filterIdMoreThanStringCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id->", [](const EvaluationContext& params, const std::string& lhs) -> Result { + "filter-id->", + [](const EvaluationContext& params, const std::string& lhs) -> Result { auto rhs = featureIdAsString(params); return rhs ? rhs > lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterLessOrEqualThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter-<=", [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { + "filter-<=", + [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { auto rhs = featurePropertyAsDouble(params, key); return rhs ? rhs <= lhs : false; - }); + }, + Dependency::Feature); return signature; } @@ -804,34 +858,41 @@ const auto& filterLessOrEqualThanStringCompoundExpression() { [](const EvaluationContext& params, const std::string& key, const std::string& lhs) -> Result { auto rhs = featurePropertyAsString(params, key); return rhs ? rhs <= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdLessOrEqualThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id-<=", [](const EvaluationContext& params, double lhs) -> Result { + "filter-id-<=", + [](const EvaluationContext& params, double lhs) -> Result { auto rhs = featureIdAsDouble(params); return rhs ? rhs <= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdLessOrEqualThanStringCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id-<=", [](const EvaluationContext& params, const std::string& lhs) -> Result { + "filter-id-<=", + [](const EvaluationContext& params, const std::string& lhs) -> Result { auto rhs = featureIdAsString(params); return rhs ? rhs <= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterGreaterOrEqualThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter->=", [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { + "filter->=", + [](const EvaluationContext& params, const std::string& key, double lhs) -> Result { auto rhs = featurePropertyAsDouble(params, key); return rhs ? rhs >= lhs : false; - }); + }, + Dependency::Feature); return signature; } @@ -841,72 +902,88 @@ const auto& filterGreaterOrEqualThanStringCompoundExpression() { [](const EvaluationContext& params, const std::string& key, const std::string& lhs) -> Result { auto rhs = featurePropertyAsString(params, key); return rhs ? rhs >= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdGreaterOrEqualThanNumberCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id->=", [](const EvaluationContext& params, double lhs) -> Result { + "filter-id->=", + [](const EvaluationContext& params, double lhs) -> Result { auto rhs = featureIdAsDouble(params); return rhs ? rhs >= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterIdGreaterOrEqualThanStringCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id->=", [](const EvaluationContext& params, const std::string& lhs) -> Result { + "filter-id->=", + [](const EvaluationContext& params, const std::string& lhs) -> Result { auto rhs = featureIdAsString(params); return rhs ? rhs >= lhs : false; - }); + }, + Dependency::Feature); return signature; } const auto& filterHasCompoundExpression() { static auto signature = detail::makeSignature( - "filter-has", [](const EvaluationContext& params, const std::string& key) -> Result { + "filter-has", + [](const EvaluationContext& params, const std::string& key) -> Result { assert(params.feature); return bool(params.feature->getValue(key)); - }); + }, + Dependency::Feature); return signature; } const auto& filterHasIdCompoundExpression() { - static auto signature = detail::makeSignature("filter-has-id", [](const EvaluationContext& params) -> Result { - assert(params.feature); - return !params.feature->getID().is(); - }); + static auto signature = detail::makeSignature( + "filter-has-id", + [](const EvaluationContext& params) -> Result { + assert(params.feature); + return !params.feature->getID().is(); + }, + Dependency::Feature); return signature; } const auto& filterTypeInCompoundExpression() { static auto signature = detail::makeSignature( - "filter-type-in", [](const EvaluationContext& params, const Varargs& types) -> Result { + "filter-type-in", + [](const EvaluationContext& params, const Varargs& types) -> Result { assert(params.feature); std::optional type = featureTypeAsString(params.feature->getType()); return std::find(types.begin(), types.end(), type) != types.end(); - }); + }, + Dependency::Feature); return signature; } const auto& filterIdInCompoundExpression() { static auto signature = detail::makeSignature( - "filter-id-in", [](const EvaluationContext& params, const Varargs& ids) -> Result { + "filter-id-in", + [](const EvaluationContext& params, const Varargs& ids) -> Result { auto id = featureIdAsExpressionValue(params); return std::find(ids.begin(), ids.end(), id) != ids.end(); - }); + }, + Dependency::Feature); return signature; } const auto& filterInCompoundExpression() { static auto signature = detail::makeSignature( - "filter-in", [](const EvaluationContext& params, const Varargs& varargs) -> Result { + "filter-in", + [](const EvaluationContext& params, const Varargs& varargs) -> Result { if (varargs.size() < 2) return false; assert(varargs[0].is()); auto value = featurePropertyAsExpressionValue(params, varargs[0].get()); return value ? std::find(varargs.begin() + 1, varargs.end(), *value) != varargs.end() : false; - }); + }, + Dependency::Feature); return signature; } @@ -1158,7 +1235,7 @@ ParseResult createCompoundExpression(const std::string& name, CompoundExpression::CompoundExpression(const detail::SignatureBase& signature_, std::vector> args_) - : Expression(Kind::CompoundExpression, signature_.result), + : Expression(Kind::CompoundExpression, signature_.result, signature_.dependencies | collectDependencies(args_)), signature(signature_), args(std::move(args_)) {} diff --git a/src/mbgl/style/expression/distance.cpp b/src/mbgl/style/expression/distance.cpp index 099b1c87c..9a7b07baa 100644 --- a/src/mbgl/style/expression/distance.cpp +++ b/src/mbgl/style/expression/distance.cpp @@ -770,7 +770,7 @@ namespace style { namespace expression { Distance::Distance(GeoJSON geojson, Feature::geometry_type geometries_) - : Expression(Kind::Distance, type::Number), + : Expression(Kind::Distance, type::Number, Dependency::Feature), geoJSONSource(std::move(geojson)), geometries(std::move(geometries_)) {} diff --git a/src/mbgl/style/expression/expression.cpp b/src/mbgl/style/expression/expression.cpp index 224e0264c..6a7ed27ee 100644 --- a/src/mbgl/style/expression/expression.cpp +++ b/src/mbgl/style/expression/expression.cpp @@ -1,9 +1,49 @@ #include #include #include + +#include #include namespace mbgl { + +using Dependency = style::expression::Dependency; + +std::ostream& operator<<(std::ostream& os, const Dependency deps) { + if (deps == Dependency::None) { + os << "None"; + return os; + } + + bool any = false; + const auto add = [&](Dependency dep, std::string_view name) { + if ((deps & dep) != Dependency::None) { + os << (any ? "|" : std::string_view{}) << name; + any = true; + } + }; + + add(Dependency::Feature, "Feature"); + add(Dependency::Image, "Image"); + add(Dependency::Zoom, "Zoom"); + add(Dependency::Location, "Location"); + add(Dependency::Bind, "Bind"); + add(Dependency::Var, "Var"); + add(Dependency::Override, "Override"); + + return os; +} + +namespace util { + +std::string toString(const style::expression::Dependency deps) { + std::ostringstream ss; + ss << deps; + return ss.str(); +} + +} // namespace util + namespace style { namespace expression { diff --git a/src/mbgl/style/expression/find_zoom_curve.cpp b/src/mbgl/style/expression/find_zoom_curve.cpp index 1a7a91704..6a48d0751 100644 --- a/src/mbgl/style/expression/find_zoom_curve.cpp +++ b/src/mbgl/style/expression/find_zoom_curve.cpp @@ -3,7 +3,7 @@ #include #include #include - +#include #include #include @@ -79,16 +79,16 @@ std::optional> findZoomCu return result; } -variant findZoomCurveChecked(const expression::Expression* e) { - if (isZoomConstant(*e)) { - return nullptr; - } - return findZoomCurve(e)->match( - [](const ParsingError&) -> variant { - assert(false); +using ZoomCurveOrNull = variant; + +ZoomCurveOrNull findZoomCurveChecked(const expression::Expression& e) { + assert(!expression::isZoomConstant(e)); + return findZoomCurve(&e)->match( + [](const ParsingError& err) -> ZoomCurveOrNull { + Log::Error(Event::Style, "Invalid Expression: " + err.message); return nullptr; }, - [](auto zoomCurve) -> variant { return zoomCurve; }); + [](auto zoomCurve) -> ZoomCurveOrNull { return zoomCurve; }); } } // namespace expression diff --git a/src/mbgl/style/expression/format_expression.cpp b/src/mbgl/style/expression/format_expression.cpp index 9999cad8c..f166508b0 100644 --- a/src/mbgl/style/expression/format_expression.cpp +++ b/src/mbgl/style/expression/format_expression.cpp @@ -9,27 +9,24 @@ namespace expression { FormatExpressionSection::FormatExpressionSection(std::unique_ptr content_) : content(std::move(content_)) {} -void FormatExpressionSection::setTextSectionOptions(std::optional> fontScale_, - std::optional> textFont_, - std::optional> textColor_) { +void FormatExpressionSection::setTextSectionOptions(std::unique_ptr&& fontScale_, + std::unique_ptr&& textFont_, + std::unique_ptr&& textColor_) { if (fontScale_) { - assert(*fontScale_); - fontScale = std::shared_ptr(std::move(*fontScale_)); + fontScale = std::shared_ptr(std::move(fontScale_)); } if (textFont_) { - assert(*textFont_); - textFont = std::shared_ptr(std::move(*textFont_)); + textFont = std::shared_ptr(std::move(textFont_)); } if (textColor_) { - assert(*textColor_); - textColor = std::shared_ptr(std::move(*textColor_)); + textColor = std::shared_ptr(std::move(textColor_)); } } FormatExpression::FormatExpression(std::vector sections_) - : Expression(Kind::FormatExpression, type::Formatted), + : Expression(Kind::FormatExpression, type::Formatted, collectDependencies(sections_, &depsOfSection)), sections(std::move(sections_)) {} using namespace mbgl::style::conversion; @@ -80,7 +77,9 @@ ParseResult FormatExpression::parse(const Convertible& value, ParsingContext& ct } } - sections.back().setTextSectionOptions(std::move(fontScale), std::move(textFont), std::move(textColor)); + sections.back().setTextSectionOptions(fontScale ? std::move(*fontScale) : std::unique_ptr{}, + textFont ? std::move(*textFont) : std::unique_ptr{}, + textColor ? std::move(*textColor) : std::unique_ptr{}); } else { ParseResult parsedArg = ctx.parse(arg, 1, {type::Value}); if (!parsedArg) { @@ -100,13 +99,13 @@ void FormatExpression::eachChild(const std::function& f for (auto& section : sections) { fn(*section.content); if (section.fontScale) { - fn(**section.fontScale); + fn(*section.fontScale); } if (section.textFont) { - fn(**section.textFont); + fn(*section.textFont); } if (section.textColor) { - fn(**section.textColor); + fn(*section.textColor); } } } @@ -123,15 +122,15 @@ bool FormatExpression::operator==(const Expression& e) const { if (*lhsSection.content != *rhsSection.content) { return false; } - if ((lhsSection.fontScale && (!rhsSection.fontScale || **lhsSection.fontScale != **rhsSection.fontScale)) || + if ((lhsSection.fontScale && (!rhsSection.fontScale || *lhsSection.fontScale != *rhsSection.fontScale)) || (!lhsSection.fontScale && rhsSection.fontScale)) { return false; } - if ((lhsSection.textFont && (!rhsSection.textFont || **lhsSection.textFont != **rhsSection.textFont)) || + if ((lhsSection.textFont && (!rhsSection.textFont || *lhsSection.textFont != *rhsSection.textFont)) || (!lhsSection.textFont && rhsSection.textFont)) { return false; } - if ((lhsSection.textColor && (!rhsSection.textColor || **lhsSection.textColor != **rhsSection.textColor)) || + if ((lhsSection.textColor && (!rhsSection.textColor || *lhsSection.textColor != *rhsSection.textColor)) || (!lhsSection.textColor && rhsSection.textColor)) { return false; } @@ -147,13 +146,13 @@ mbgl::Value FormatExpression::serialize() const { serialized.push_back(section.content->serialize()); std::unordered_map options; if (section.fontScale) { - options.emplace(kFormattedSectionFontScale, (*section.fontScale)->serialize()); + options.emplace(kFormattedSectionFontScale, section.fontScale->serialize()); } if (section.textFont) { - options.emplace(kFormattedSectionTextFont, (*section.textFont)->serialize()); + options.emplace(kFormattedSectionTextFont, section.textFont->serialize()); } if (section.textColor) { - options.emplace(kFormattedSectionTextColor, (*section.textColor)->serialize()); + options.emplace(kFormattedSectionTextColor, section.textColor->serialize()); } serialized.emplace_back(options); } @@ -188,7 +187,7 @@ EvaluationResult FormatExpression::evaluate(const EvaluationContext& params) con std::optional evaluatedFontScale; if (section.fontScale) { - auto fontScaleResult = (*section.fontScale)->evaluate(params); + auto fontScaleResult = section.fontScale->evaluate(params); if (!fontScaleResult) { return fontScaleResult.error(); } @@ -197,7 +196,7 @@ EvaluationResult FormatExpression::evaluate(const EvaluationContext& params) con std::optional evaluatedTextFont; if (section.textFont) { - auto textFontResult = (*section.textFont)->evaluate(params); + auto textFontResult = section.textFont->evaluate(params); if (!textFontResult) { return textFontResult.error(); } @@ -212,7 +211,7 @@ EvaluationResult FormatExpression::evaluate(const EvaluationContext& params) con std::optional evaluatedTextColor; if (section.textColor) { - auto textColorResult = (*section.textColor)->evaluate(params); + auto textColorResult = section.textColor->evaluate(params); if (!textColorResult) { return textColorResult.error(); } diff --git a/src/mbgl/style/expression/image_expression.cpp b/src/mbgl/style/expression/image_expression.cpp index b49a19551..7c4776ebf 100644 --- a/src/mbgl/style/expression/image_expression.cpp +++ b/src/mbgl/style/expression/image_expression.cpp @@ -7,7 +7,7 @@ namespace style { namespace expression { ImageExpression::ImageExpression(std::unique_ptr imageID_) - : Expression(Kind::ImageExpression, type::Image), + : Expression(Kind::ImageExpression, type::Image, depsOf(imageID_) | Dependency::Image), imageID(std::move(imageID_)) { assert(imageID); } diff --git a/src/mbgl/style/expression/in.cpp b/src/mbgl/style/expression/in.cpp index 99b215288..5f0f632aa 100644 --- a/src/mbgl/style/expression/in.cpp +++ b/src/mbgl/style/expression/in.cpp @@ -29,7 +29,7 @@ bool isSearchableRuntimeType(const type::Type& type) { } // namespace In::In(std::unique_ptr needle_, std::unique_ptr haystack_) - : Expression(Kind::In, type::Boolean), + : Expression(Kind::In, type::Boolean, depsOf(needle_) | depsOf(haystack_)), needle(std::move(needle_)), haystack(std::move(haystack_)) { assert(isComparableType(needle->getType())); diff --git a/src/mbgl/style/expression/index_of.cpp b/src/mbgl/style/expression/index_of.cpp index 432e40b7c..93924d20d 100644 --- a/src/mbgl/style/expression/index_of.cpp +++ b/src/mbgl/style/expression/index_of.cpp @@ -10,7 +10,7 @@ namespace expression { IndexOf::IndexOf(std::unique_ptr keyword_, std::unique_ptr input_, std::unique_ptr fromIndex_) - : Expression(Kind::IndexOf, type::Number), + : Expression(Kind::IndexOf, type::Number, depsOf(keyword_) | depsOf(input_) | depsOf(fromIndex_)), keyword(std::move(keyword_)), input(std::move(input_)), fromIndex(std::move(fromIndex_)) {} diff --git a/src/mbgl/style/expression/interpolate.cpp b/src/mbgl/style/expression/interpolate.cpp index 8cde78dc6..2c49bca51 100644 --- a/src/mbgl/style/expression/interpolate.cpp +++ b/src/mbgl/style/expression/interpolate.cpp @@ -250,7 +250,7 @@ Interpolate::Interpolate(const type::Type& type_, Interpolator interpolator_, std::unique_ptr input_, std::map> stops_) - : Expression(Kind::Interpolate, type_), + : Expression(Kind::Interpolate, type_, depsOf(input_) | collectDependencies(stops_)), interpolator(std::move(interpolator_)), input(std::move(input_)), stops(std::move(stops_)) { diff --git a/src/mbgl/style/expression/length.cpp b/src/mbgl/style/expression/length.cpp index 1e3432f42..1f73d4f48 100644 --- a/src/mbgl/style/expression/length.cpp +++ b/src/mbgl/style/expression/length.cpp @@ -7,7 +7,7 @@ namespace style { namespace expression { Length::Length(std::unique_ptr input_) - : Expression(Kind::Length, type::Number), + : Expression(Kind::Length, type::Number, depsOf(input_)), input(std::move(input_)) {} EvaluationResult Length::evaluate(const EvaluationContext& params) const { diff --git a/src/mbgl/style/expression/number_format.cpp b/src/mbgl/style/expression/number_format.cpp index b5864e495..2e1339b91 100644 --- a/src/mbgl/style/expression/number_format.cpp +++ b/src/mbgl/style/expression/number_format.cpp @@ -16,7 +16,10 @@ NumberFormat::NumberFormat(std::unique_ptr number_, std::unique_ptr currency_, std::unique_ptr minFractionDigits_, std::unique_ptr maxFractionDigits_) - : Expression(Kind::NumberFormat, type::String), + : Expression(Kind::NumberFormat, + type::String, + depsOf(number_) | depsOf(locale_) | depsOf(currency_) | depsOf(minFractionDigits_) | + depsOf(maxFractionDigits_)), number(std::move(number_)), locale(std::move(locale_)), currency(std::move(currency_)), diff --git a/src/mbgl/style/expression/slice.cpp b/src/mbgl/style/expression/slice.cpp index 5c3a723ca..4919ad185 100644 --- a/src/mbgl/style/expression/slice.cpp +++ b/src/mbgl/style/expression/slice.cpp @@ -22,7 +22,7 @@ int normalizeIndex(int index, int length) { Slice::Slice(std::unique_ptr input_, std::unique_ptr fromIndex_, std::unique_ptr toIndex_) - : Expression(Kind::Slice, input_->getType()), + : Expression(Kind::Slice, input_->getType(), depsOf(input_) | depsOf(fromIndex_) | depsOf(toIndex_)), input(std::move(input_)), fromIndex(std::move(fromIndex_)), toIndex(std::move(toIndex_)) {} diff --git a/src/mbgl/style/expression/step.cpp b/src/mbgl/style/expression/step.cpp index 72f54d0fc..940b8cfd8 100644 --- a/src/mbgl/style/expression/step.cpp +++ b/src/mbgl/style/expression/step.cpp @@ -12,7 +12,7 @@ namespace expression { Step::Step(const type::Type& type_, std::unique_ptr input_, std::map> stops_) - : Expression(Kind::Step, type_), + : Expression(Kind::Step, type_, depsOf(input_) | collectDependencies(stops_)), input(std::move(input_)), stops(std::move(stops_)) { assert(input->getType() == type::Number); diff --git a/src/mbgl/style/expression/within.cpp b/src/mbgl/style/expression/within.cpp index 1b2f5a0a0..429a2d8b3 100644 --- a/src/mbgl/style/expression/within.cpp +++ b/src/mbgl/style/expression/within.cpp @@ -206,7 +206,7 @@ namespace style { namespace expression { Within::Within(GeoJSON geojson, Feature::geometry_type geometries_) - : Expression(Kind::Within, type::Boolean), + : Expression(Kind::Within, type::Boolean, Dependency::Feature), geoJSONSource(std::move(geojson)), geometries(std::move(geometries_)) {} diff --git a/src/mbgl/style/layer.cpp b/src/mbgl/style/layer.cpp index 3b94aa9d9..2b7f70132 100644 --- a/src/mbgl/style/layer.cpp +++ b/src/mbgl/style/layer.cpp @@ -223,5 +223,10 @@ const LayerTypeInfo* Layer::getTypeInfo() const noexcept { return baseImpl->getTypeInfo(); } +/// Collect dependencies +expression::Dependency Layer::getDependencies() const noexcept { + return baseImpl->getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layer_impl.hpp b/src/mbgl/style/layer_impl.hpp index 1951d898f..980e40eb4 100644 --- a/src/mbgl/style/layer_impl.hpp +++ b/src/mbgl/style/layer_impl.hpp @@ -48,6 +48,9 @@ public: // Populates the given \a fontStack with fonts being used by the layer. virtual void populateFontStack(std::set& fontStack) const; + /// Collect the style dependencies for this layer + virtual expression::Dependency getDependencies() const noexcept { return expression::Dependency::None; } + std::string id; std::string source; std::string sourceLayer; diff --git a/src/mbgl/style/layers/background_layer_properties.cpp b/src/mbgl/style/layers/background_layer_properties.cpp index 9bf405ab0..ec4b71f2a 100644 --- a/src/mbgl/style/layers/background_layer_properties.cpp +++ b/src/mbgl/style/layers/background_layer_properties.cpp @@ -31,6 +31,10 @@ const BackgroundLayer::Impl& BackgroundLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency BackgroundLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/background_layer_properties.hpp b/src/mbgl/style/layers/background_layer_properties.hpp index 676063ad8..9459d2241 100644 --- a/src/mbgl/style/layers/background_layer_properties.hpp +++ b/src/mbgl/style/layers/background_layer_properties.hpp @@ -45,6 +45,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const BackgroundLayer::Impl& layerImpl() const; // Data members. CrossfadeParameters crossfade; diff --git a/src/mbgl/style/layers/circle_layer_properties.cpp b/src/mbgl/style/layers/circle_layer_properties.cpp index a72b7bed6..dc1639850 100644 --- a/src/mbgl/style/layers/circle_layer_properties.cpp +++ b/src/mbgl/style/layers/circle_layer_properties.cpp @@ -29,6 +29,10 @@ const CircleLayer::Impl& CircleLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency CircleLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies() | layerImpl().layout.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/circle_layer_properties.hpp b/src/mbgl/style/layers/circle_layer_properties.hpp index 08418eef1..565fbd265 100644 --- a/src/mbgl/style/layers/circle_layer_properties.hpp +++ b/src/mbgl/style/layers/circle_layer_properties.hpp @@ -93,6 +93,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const CircleLayer::Impl& layerImpl() const; // Data members. CirclePaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/layers/custom_drawable_layer_impl.hpp b/src/mbgl/style/layers/custom_drawable_layer_impl.hpp index 7d74decd4..baf600009 100644 --- a/src/mbgl/style/layers/custom_drawable_layer_impl.hpp +++ b/src/mbgl/style/layers/custom_drawable_layer_impl.hpp @@ -28,6 +28,8 @@ class CustomDrawableLayerProperties final : public LayerProperties { public: explicit CustomDrawableLayerProperties(Immutable impl) : LayerProperties(std::move(impl)) {} + + expression::Dependency getDependencies() const noexcept override { return expression::Dependency::None; } }; } // namespace style diff --git a/src/mbgl/style/layers/custom_layer_impl.hpp b/src/mbgl/style/layers/custom_layer_impl.hpp index 370db229f..e7db35ca8 100644 --- a/src/mbgl/style/layers/custom_layer_impl.hpp +++ b/src/mbgl/style/layers/custom_layer_impl.hpp @@ -28,6 +28,8 @@ class CustomLayerProperties final : public LayerProperties { public: explicit CustomLayerProperties(Immutable impl) : LayerProperties(std::move(impl)) {} + + expression::Dependency getDependencies() const noexcept override { return expression::Dependency::None; } }; } // namespace style diff --git a/src/mbgl/style/layers/fill_extrusion_layer_properties.cpp b/src/mbgl/style/layers/fill_extrusion_layer_properties.cpp index e1af47070..2f306511b 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer_properties.cpp +++ b/src/mbgl/style/layers/fill_extrusion_layer_properties.cpp @@ -31,6 +31,10 @@ const FillExtrusionLayer::Impl& FillExtrusionLayerProperties::layerImpl() const return static_cast(*baseImpl); } +expression::Dependency FillExtrusionLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/fill_extrusion_layer_properties.hpp b/src/mbgl/style/layers/fill_extrusion_layer_properties.hpp index f7cc0e2b6..4349bd14a 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer_properties.hpp +++ b/src/mbgl/style/layers/fill_extrusion_layer_properties.hpp @@ -70,6 +70,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const FillExtrusionLayer::Impl& layerImpl() const; // Data members. CrossfadeParameters crossfade; diff --git a/src/mbgl/style/layers/fill_layer_properties.cpp b/src/mbgl/style/layers/fill_layer_properties.cpp index c02ae9cc1..96b8eca29 100644 --- a/src/mbgl/style/layers/fill_layer_properties.cpp +++ b/src/mbgl/style/layers/fill_layer_properties.cpp @@ -31,6 +31,10 @@ const FillLayer::Impl& FillLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency FillLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies() | layerImpl().layout.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/fill_layer_properties.hpp b/src/mbgl/style/layers/fill_layer_properties.hpp index 7f843722b..56cc1bef5 100644 --- a/src/mbgl/style/layers/fill_layer_properties.hpp +++ b/src/mbgl/style/layers/fill_layer_properties.hpp @@ -74,6 +74,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const FillLayer::Impl& layerImpl() const; // Data members. CrossfadeParameters crossfade; diff --git a/src/mbgl/style/layers/heatmap_layer_properties.cpp b/src/mbgl/style/layers/heatmap_layer_properties.cpp index 9070e479e..5d92433d6 100644 --- a/src/mbgl/style/layers/heatmap_layer_properties.cpp +++ b/src/mbgl/style/layers/heatmap_layer_properties.cpp @@ -29,6 +29,10 @@ const HeatmapLayer::Impl& HeatmapLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency HeatmapLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/heatmap_layer_properties.hpp b/src/mbgl/style/layers/heatmap_layer_properties.hpp index 710192c23..c1c351447 100644 --- a/src/mbgl/style/layers/heatmap_layer_properties.hpp +++ b/src/mbgl/style/layers/heatmap_layer_properties.hpp @@ -53,6 +53,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const HeatmapLayer::Impl& layerImpl() const; // Data members. HeatmapPaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/layers/hillshade_layer_properties.cpp b/src/mbgl/style/layers/hillshade_layer_properties.cpp index 09b3355d8..7d7c30e38 100644 --- a/src/mbgl/style/layers/hillshade_layer_properties.cpp +++ b/src/mbgl/style/layers/hillshade_layer_properties.cpp @@ -29,6 +29,10 @@ const HillshadeLayer::Impl& HillshadeLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency HillshadeLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/hillshade_layer_properties.hpp b/src/mbgl/style/layers/hillshade_layer_properties.hpp index 8dde56a92..4b35f01a5 100644 --- a/src/mbgl/style/layers/hillshade_layer_properties.hpp +++ b/src/mbgl/style/layers/hillshade_layer_properties.hpp @@ -59,6 +59,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const HillshadeLayer::Impl& layerImpl() const; // Data members. HillshadePaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/layers/layer_properties.cpp.ejs b/src/mbgl/style/layers/layer_properties.cpp.ejs index 09f4c4837..cebc64114 100644 --- a/src/mbgl/style/layers/layer_properties.cpp.ejs +++ b/src/mbgl/style/layers/layer_properties.cpp.ejs @@ -40,6 +40,10 @@ const <%- camelize(type) %>Layer::Impl& <%- camelize(type) %>LayerProperties::la return static_castLayer::Impl&>(*baseImpl); } +expression::Dependency <%- camelize(type) %>LayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies()<% if (layoutProperties.length) { -%> | layerImpl().layout.getDependencies()<% } -%>; +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/layer_properties.hpp.ejs b/src/mbgl/style/layers/layer_properties.hpp.ejs index 66cff8c3c..314317ab3 100644 --- a/src/mbgl/style/layers/layer_properties.hpp.ejs +++ b/src/mbgl/style/layers/layer_properties.hpp.ejs @@ -76,6 +76,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const <%- camelize(type) %>Layer::Impl& layerImpl() const; // Data members. <% if (type === 'background' || type === 'fill' || type === 'line' || type === 'fill-extrusion') { -%> diff --git a/src/mbgl/style/layers/line_layer_impl.hpp b/src/mbgl/style/layers/line_layer_impl.hpp index af6c13dfd..a5c7229fb 100644 --- a/src/mbgl/style/layers/line_layer_impl.hpp +++ b/src/mbgl/style/layers/line_layer_impl.hpp @@ -14,6 +14,10 @@ public: bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; + expression::Dependency getDependencies() const noexcept override { + return layout.getDependencies() | paint.getDependencies(); + } + LineLayoutProperties::Unevaluated layout; LinePaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/line_layer_properties.cpp b/src/mbgl/style/layers/line_layer_properties.cpp index ca346bfa3..d5e7199ce 100644 --- a/src/mbgl/style/layers/line_layer_properties.cpp +++ b/src/mbgl/style/layers/line_layer_properties.cpp @@ -31,6 +31,10 @@ const LineLayer::Impl& LineLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency LineLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies() | layerImpl().layout.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/line_layer_properties.hpp b/src/mbgl/style/layers/line_layer_properties.hpp index 1a2cf0b0b..1fe89ff90 100644 --- a/src/mbgl/style/layers/line_layer_properties.hpp +++ b/src/mbgl/style/layers/line_layer_properties.hpp @@ -123,6 +123,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const LineLayer::Impl& layerImpl() const; // Data members. CrossfadeParameters crossfade; diff --git a/src/mbgl/style/layers/location_indicator_layer_properties.cpp b/src/mbgl/style/layers/location_indicator_layer_properties.cpp index 818e45b5a..0391551ea 100644 --- a/src/mbgl/style/layers/location_indicator_layer_properties.cpp +++ b/src/mbgl/style/layers/location_indicator_layer_properties.cpp @@ -29,6 +29,10 @@ const LocationIndicatorLayer::Impl& LocationIndicatorLayerProperties::layerImpl( return static_cast(*baseImpl); } +expression::Dependency LocationIndicatorLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies() | layerImpl().layout.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/location_indicator_layer_properties.hpp b/src/mbgl/style/layers/location_indicator_layer_properties.hpp index 879da1eeb..b8523e046 100644 --- a/src/mbgl/style/layers/location_indicator_layer_properties.hpp +++ b/src/mbgl/style/layers/location_indicator_layer_properties.hpp @@ -100,6 +100,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const LocationIndicatorLayer::Impl& layerImpl() const; // Data members. LocationIndicatorPaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/layers/raster_layer_properties.cpp b/src/mbgl/style/layers/raster_layer_properties.cpp index a08dfa504..eccc8e26f 100644 --- a/src/mbgl/style/layers/raster_layer_properties.cpp +++ b/src/mbgl/style/layers/raster_layer_properties.cpp @@ -29,6 +29,10 @@ const RasterLayer::Impl& RasterLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency RasterLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/raster_layer_properties.hpp b/src/mbgl/style/layers/raster_layer_properties.hpp index dda057f38..5bc97bb16 100644 --- a/src/mbgl/style/layers/raster_layer_properties.hpp +++ b/src/mbgl/style/layers/raster_layer_properties.hpp @@ -69,6 +69,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const RasterLayer::Impl& layerImpl() const; // Data members. RasterPaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/layers/symbol_layer_properties.cpp b/src/mbgl/style/layers/symbol_layer_properties.cpp index 87229c73f..f29279e49 100644 --- a/src/mbgl/style/layers/symbol_layer_properties.cpp +++ b/src/mbgl/style/layers/symbol_layer_properties.cpp @@ -29,6 +29,10 @@ const SymbolLayer::Impl& SymbolLayerProperties::layerImpl() const { return static_cast(*baseImpl); } +expression::Dependency SymbolLayerProperties::getDependencies() const noexcept { + return layerImpl().paint.getDependencies() | layerImpl().layout.getDependencies(); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/symbol_layer_properties.hpp b/src/mbgl/style/layers/symbol_layer_properties.hpp index b468fca01..89ba28055 100644 --- a/src/mbgl/style/layers/symbol_layer_properties.hpp +++ b/src/mbgl/style/layers/symbol_layer_properties.hpp @@ -351,6 +351,8 @@ public: unsigned long constantsMask() const override; + expression::Dependency getDependencies() const noexcept override; + const SymbolLayer::Impl& layerImpl() const; // Data members. SymbolPaintProperties::PossiblyEvaluated evaluated; diff --git a/src/mbgl/style/properties.hpp b/src/mbgl/style/properties.hpp index fb9d7579a..85844b1ec 100644 --- a/src/mbgl/style/properties.hpp +++ b/src/mbgl/style/properties.hpp @@ -1,10 +1,11 @@ #pragma once -#include -#include -#include #include #include +#include +#include +#include +#include #include #include @@ -133,6 +134,8 @@ public: using DataDrivenProperties = FilteredTypeList; using OverridableProperties = FilteredTypeList; + using Dependency = expression::Dependency; + template using Tuple = IndexedTuple; @@ -245,6 +248,26 @@ public: return Evaluated{evaluate(z, feature)...}; } + /// Extract dependencies from a possibly-evaluated property which may have an expression. + template + Dependency getDependencies(const P&) const noexcept { + return Dependency::None; + } + template + Dependency getDependencies(const PossiblyEvaluatedPropertyValue

& v) const noexcept { + return v.getDependencies(); + } + template + Dependency getDependencies(const PossiblyEvaluatedPropertyValue>& v) const noexcept { + return v.getDependencies(); + } + + Dependency getDependencies() const noexcept { + Dependency result = Dependency::None; + util::ignore({(result |= getDependencies(this->template get()))...}); + return result; + } + unsigned long constantsMask() const { return ConstantsMask::getMask(*this); } }; @@ -276,6 +299,26 @@ public: util::ignore({(conversion::stringify(writer, this->template get()), 0)...}); writer.EndObject(); } + + /// Get the combined dependencies of any contained expressions + constexpr Dependency getDependencies() const noexcept { + Dependency result = Dependency::None; + util::ignore({(result |= getDependencies(this->template get()))...}); + return result; + } + + unsigned long constantsMask() const { return ConstantsMask::getMask(*this); } + + protected: + template + Dependency getDependencies(const PropertyValue

& v) const noexcept { + return v.getDependencies(); + } + + template + Dependency getDependencies(const Transitioning

& v) const noexcept { + return v.getValue().getDependencies(); + } }; class Transitionable : public Tuple { @@ -298,6 +341,24 @@ public: other.template get().value))...}); return result; } + + Dependency getDependencies() const noexcept { + Dependency result = Dependency::None; + util::ignore({(result |= getDependencies(this->template get()))...}); + return result; + } + + protected: + template + Dependency getDependencies(const style::Transitionable>& v) const noexcept { + return v.value.getDependencies(); + } + Dependency getDependencies(const style::ColorRampPropertyValue& v) const noexcept { + return v.getDependencies(); + } + Dependency getDependencies(const style::Transitionable& v) const noexcept { + return v.value.getDependencies(); + } }; }; diff --git a/src/mbgl/style/property_expression.cpp b/src/mbgl/style/property_expression.cpp index a6e0b4712..745fc90b0 100644 --- a/src/mbgl/style/property_expression.cpp +++ b/src/mbgl/style/property_expression.cpp @@ -5,23 +5,10 @@ namespace style { PropertyExpressionBase::PropertyExpressionBase(std::unique_ptr expression_) : expression(std::move(expression_)), - zoomCurve(expression::findZoomCurveChecked(expression.get())) { - isZoomConstant_ = expression::isZoomConstant(*expression); - isFeatureConstant_ = expression::isFeatureConstant(*expression); - isRuntimeConstant_ = expression::isRuntimeConstant(*expression); -} - -bool PropertyExpressionBase::isZoomConstant() const noexcept { - return isZoomConstant_; -} - -bool PropertyExpressionBase::isFeatureConstant() const noexcept { - return isFeatureConstant_; -} - -bool PropertyExpressionBase::isRuntimeConstant() const noexcept { - return isRuntimeConstant_; -} + isZoomConstant_(expression::isZoomConstant(*expression)), + isFeatureConstant_(expression::isFeatureConstant(*expression)), + isRuntimeConstant_(expression::isRuntimeConstant(*expression)), + zoomCurve(isZoomConstant_ ? nullptr : expression::findZoomCurveChecked(*expression)) {} float PropertyExpressionBase::interpolationFactor(const Range& inputLevels, const float inputValue) const noexcept { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e53d39291..ede393834 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -51,6 +51,7 @@ add_library( ${PROJECT_SOURCE_DIR}/test/style/conversion/property_value.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/stringify.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/tileset.test.cpp + ${PROJECT_SOURCE_DIR}/test/style/expression/dependency.test.cpp ${PROJECT_SOURCE_DIR}/test/style/expression/expression.test.cpp ${PROJECT_SOURCE_DIR}/test/style/expression/util.test.cpp ${PROJECT_SOURCE_DIR}/test/style/filter.test.cpp diff --git a/test/fixtures/expression_equality/collator1.a.json b/test/fixtures/expression_equality/collator1.a.json new file mode 100644 index 000000000..63620828e --- /dev/null +++ b/test/fixtures/expression_equality/collator1.a.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator1.b.json b/test/fixtures/expression_equality/collator1.b.json new file mode 100644 index 000000000..d7992a351 --- /dev/null +++ b/test/fixtures/expression_equality/collator1.b.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": false, "diacritic-sensitive": true, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator2.a.json b/test/fixtures/expression_equality/collator2.a.json new file mode 100644 index 000000000..63620828e --- /dev/null +++ b/test/fixtures/expression_equality/collator2.a.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator2.b.json b/test/fixtures/expression_equality/collator2.b.json new file mode 100644 index 000000000..b3f2656b2 --- /dev/null +++ b/test/fixtures/expression_equality/collator2.b.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": false, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator3.a.json b/test/fixtures/expression_equality/collator3.a.json new file mode 100644 index 000000000..63620828e --- /dev/null +++ b/test/fixtures/expression_equality/collator3.a.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator3.b.json b/test/fixtures/expression_equality/collator3.b.json new file mode 100644 index 000000000..40bd0c531 --- /dev/null +++ b/test/fixtures/expression_equality/collator3.b.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true, "locale": "en"} ] diff --git a/test/fixtures/expression_equality/collator4.a.json b/test/fixtures/expression_equality/collator4.a.json new file mode 100644 index 000000000..f83bb6ca1 --- /dev/null +++ b/test/fixtures/expression_equality/collator4.a.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true} ] diff --git a/test/fixtures/expression_equality/collator4.b.json b/test/fixtures/expression_equality/collator4.b.json new file mode 100644 index 000000000..b3f2656b2 --- /dev/null +++ b/test/fixtures/expression_equality/collator4.b.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": false, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator5.a.json b/test/fixtures/expression_equality/collator5.a.json new file mode 100644 index 000000000..63620828e --- /dev/null +++ b/test/fixtures/expression_equality/collator5.a.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": true, "locale": "de"} ] diff --git a/test/fixtures/expression_equality/collator5.b.json b/test/fixtures/expression_equality/collator5.b.json new file mode 100644 index 000000000..23926dbcf --- /dev/null +++ b/test/fixtures/expression_equality/collator5.b.json @@ -0,0 +1 @@ +[ "collator", {"case-sensitive": true, "diacritic-sensitive": false} ] diff --git a/test/fixtures/style_parser/expressions.info.json b/test/fixtures/style_parser/expressions.info.json index 75fb70c1d..af8a6053e 100644 --- a/test/fixtures/style_parser/expressions.info.json +++ b/test/fixtures/style_parser/expressions.info.json @@ -4,7 +4,7 @@ [1, "WARNING", "ParseStyle", "Expected color but found number instead."], [1, "WARNING", "ParseStyle", "Expected arguments of type (number, number, number, number), but found (number, string, number, number) instead."], [1, "WARNING", "ParseStyle", "\"zoom\" expression may only be used as input to a top-level \"step\" or \"interpolate\" expression."], - [1, "WARNING", "ParseStyle", "value must be a string"], + [3, "WARNING", "ParseStyle", "value must be a string"], [1, "WARNING", "ParseStyle", "data expressions not supported"], [1, "WARNING", "ParseStyle", "Type array is not interpolatable."] ] diff --git a/test/fixtures/style_parser/expressions.style.json b/test/fixtures/style_parser/expressions.style.json index bb5a03e8c..901c11422 100644 --- a/test/fixtures/style_parser/expressions.style.json +++ b/test/fixtures/style_parser/expressions.style.json @@ -52,6 +52,24 @@ "visibility": ["literal", true] } }, + { + "id": "invalid value - must be a string", + "type": "fill", + "source": "source", + "source-layer": 42069, + "layout": { + "visibility": "visible" + } + }, + { + "id": "invalid value - must be a string (enum)", + "type": "fill", + "source": "source", + "source-layer": "layer", + "layout": { + "visibility": [42069, true] + } + }, { "id": "invalid expression - not a DDS property", "type": "fill-extrusion", diff --git a/test/src/mbgl/test/fixture_log_observer.cpp b/test/src/mbgl/test/fixture_log_observer.cpp index e78705e04..f41fb1bf6 100644 --- a/test/src/mbgl/test/fixture_log_observer.cpp +++ b/test/src/mbgl/test/fixture_log_observer.cpp @@ -72,9 +72,14 @@ size_t FixtureLog::uncheckedCount() const { return observer ? observer->uncheckedCount() : 0; } +std::vector FixtureLog::unchecked() const { + return observer ? observer->unchecked() : std::vector{}; +} + FixtureLog::~FixtureLog() { if (observer) { - Log::removeObserver(); + Log::removeObserver(); // destroys `observer` + observer = nullptr; } } diff --git a/test/src/mbgl/test/fixture_log_observer.hpp b/test/src/mbgl/test/fixture_log_observer.hpp index 8454c875f..fb620ae1f 100644 --- a/test/src/mbgl/test/fixture_log_observer.hpp +++ b/test/src/mbgl/test/fixture_log_observer.hpp @@ -51,6 +51,7 @@ public: bool empty() const; size_t uncheckedCount() const; size_t count(const Message& message, bool substring = false) const; + std::vector unchecked() const; ~FixtureLog(); diff --git a/test/style/conversion/function.test.cpp b/test/style/conversion/function.test.cpp index 3ad353a8d..f92bd827e 100644 --- a/test/style/conversion/function.test.cpp +++ b/test/style/conversion/function.test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using namespace mbgl; using namespace mbgl::style; @@ -52,33 +53,37 @@ TEST(StyleConversion, Function) { ASSERT_EQ("function base must be a number", error.message); } -TEST(StyleConversion, CompositeFunctionExpression) { +namespace { + +template +void parseCheck(const std::string& json, std::string expectedError = {}) { + using namespace std::string_literals; + SCOPED_TRACE("Expression: "s + json); + Error error; + const auto expr = convertJSON>(json, error, true, false); + if (expectedError.empty()) { + ASSERT_TRUE(expr); + } else { + ASSERT_FALSE(expr); + ASSERT_EQ(expectedError, error.message); + } +} +} // namespace - auto parseFunction = [&](const std::string& json) { - return convertJSON>(json, error, true, false); - }; - - auto fn1 = parseFunction(R"(["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10])"); - ASSERT_TRUE(fn1); - - auto fn2 = parseFunction( - R"(["coalesce", ["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10], 0])"); - ASSERT_TRUE(fn2); - - auto fn3 = parseFunction( +TEST(StyleConversion, CompositeFunctionExpression) { + parseCheck(R"(["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10])"); + parseCheck(R"(["coalesce", ["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10], 0])"); + parseCheck( R"(["let", "a", 0, ["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10] ])"); - ASSERT_TRUE(fn3); - - auto fn4 = parseFunction( + parseCheck( R"(["coalesce", ["let", "a", 0, ["interpolate", ["linear"], ["zoom"], 0, ["number", ["get", "x"]], 10, 10]], 0])"); - ASSERT_TRUE(fn4); - - auto fn5 = parseFunction( - R"(["coalesce", ["interpolate", ["linear"], ["number", ["get", "x"]], 0, ["zoom"], 10, 10], 0])"); - ASSERT_FALSE(fn5); - ASSERT_EQ(R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)", - error.message); + parseCheck( + R"(["coalesce", ["interpolate", ["linear"], ["number", ["get", "x"]], 0, ["zoom"], 10, 10], 0])", + R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)"); + parseCheck(R"(["number", ["get", "x", ["literal", {"x": 0}]]])"); + parseCheck(R"(["has", "x", ["literal", {"x": 0}]])"); + parseCheck(R"(["has", "x", ["properties"]])"); } TEST(StyleConversion, TokenStrings) { diff --git a/test/style/expression/dependency.test.cpp b/test/style/expression/dependency.test.cpp new file mode 100644 index 000000000..b6044cdd7 --- /dev/null +++ b/test/style/expression/dependency.test.cpp @@ -0,0 +1,79 @@ +#include +#include +#include +#include + +using namespace mbgl; +using namespace mbgl::style; +using namespace mbgl::style::expression; +using namespace mbgl::style::expression::dsl; + +using namespace std::string_literals; + +namespace std { +using ::mbgl::operator<<; +} + +template +auto createOverride(expression::type::Type exprType, + PossiblyEvaluatedPropertyValue propValue, + std::string propName) { + return std::make_unique>(std::move(exprType), std::move(propValue), std::move(propName)); +} + +TEST(ExpressionDependencies, Get) { + EXPECT_EQ(Dependency::Feature, number(get("property"))->dependencies); +} + +TEST(ExpressionDependencies, Id) { + EXPECT_EQ(Dependency::Feature, id()->dependencies); +} + +TEST(ExpressionDependencies, Zoom) { + EXPECT_EQ(Dependency::Zoom, zoom()->dependencies); +} + +TEST(ExpressionDependencies, Convert) { + EXPECT_EQ(Dependency::Feature, toColor(string(get("color")))->dependencies); + EXPECT_EQ(Dependency::Feature, toString(number(string(get("color"))))->dependencies); + EXPECT_EQ(Dependency::Feature, boolean(string(get("color")))->dependencies); + EXPECT_EQ(Dependency::Feature, toFormatted(toColor(string(get("color"))))->dependencies); + EXPECT_EQ(Dependency::Feature | Dependency::Image, toImage(string(get("color")))->dependencies); +} + +TEST(ExpressionDependencies, Compare) { + EXPECT_EQ(Dependency::Feature, eq(string(get("color")), string(get("color")))->dependencies); + EXPECT_EQ(Dependency::Feature, lt(string(get("color")), string(get("color")))->dependencies); + EXPECT_EQ(Dependency::Feature, gt(string(get("color")), string(get("color")))->dependencies); + EXPECT_EQ(Dependency::Feature, eq(string(get("color")), string(get("color")))->dependencies); +} + +TEST(ExpressionDependencies, Interpolate) { + EXPECT_EQ(Dependency::Feature, step(number(id()), literal(0.0), 0.0, literal(0.0))->dependencies); + EXPECT_EQ(Dependency::Feature, step(literal(0.0), number(id()), 0.0, literal(0.0))->dependencies); + EXPECT_EQ(Dependency::Feature, step(literal(0.0), literal(0.0), 0.0, number(id()))->dependencies); + + interpolate(linear(), number(id()), 0.0, literal(0.0), 0.0, literal(0.0), 0.0, literal(0.0)); + interpolate(linear(), literal(0.0), 0.0, number(id()), 0.0, literal(0.0), 0.0, literal(0.0)); + interpolate(linear(), literal(0.0), 0.0, literal(0.0), 0.0, number(id()), 0.0, literal(0.0)); + interpolate(linear(), literal(0.0), 0.0, literal(0.0), 0.0, literal(0.0), 0.0, number(id())); +} + +TEST(ExpressionDependencies, Image) { + EXPECT_EQ(Dependency::Image, image(literal("airport-11"))->dependencies); + EXPECT_EQ(Dependency::Image | Dependency::Feature, image(get(literal("image_name"s)))->dependencies); +} + +TEST(ExpressionDependencies, Distance) { + const std::string pointGeoSource = R"({ + "type": "Point", + "coordinates":[24.938492774963375,60.16980226227959]})"; + const auto exprStr = R"(["distance", )" + pointGeoSource + R"( ])"; + const auto expression = createExpression(exprStr.c_str()); + EXPECT_EQ(Dependency::Feature, expression->dependencies); +} + +TEST(ExpressionDependencies, CustomLayer) { + auto impl = makeMutable("", nullptr); + EXPECT_EQ(Dependency::None, CustomLayerProperties{std::move(impl)}.getDependencies()); +} diff --git a/test/style/expression/expression.test.cpp b/test/style/expression/expression.test.cpp index 72abf4be0..c24559fa5 100644 --- a/test/style/expression/expression.test.cpp +++ b/test/style/expression/expression.test.cpp @@ -1,9 +1,10 @@ +#include +#include +#include +#include #include #include -#include #include -#include -#include #include @@ -81,6 +82,10 @@ TEST_P(ExpressionEqualityTest, ExpressionEquality) { EXPECT_TRUE(*expression_a1 == *expression_a2); EXPECT_TRUE(*expression_a1 != *expression_b); + + // Exercise the type-not-equal branches + using namespace expression; + EXPECT_FALSE(*expression_b == *((expression_b->getKind() == Kind::Literal) ? dsl::id() : dsl::literal(0.0))); } static void populateNames(std::vector& names) { @@ -127,6 +132,7 @@ static void populateNames(std::vector& names) { INSTANTIATE_TEST_SUITE_P(Expression, ExpressionEqualityTest, ::testing::ValuesIn([] { std::vector names; populateNames(names); + std::sort(names.begin(), names.end()); EXPECT_GT(names.size(), 0u); return names; }())); diff --git a/test/style/filter.test.cpp b/test/style/filter.test.cpp index 57e346b4b..c505d8d4b 100644 --- a/test/style/filter.test.cpp +++ b/test/style/filter.test.cpp @@ -192,15 +192,44 @@ TEST(Filter, NotHas) { } TEST(Filter, ID) { - FeatureIdentifier id1{uint64_t{1234}}; + const FeatureIdentifier id1{uint64_t{1234}}; ASSERT_TRUE(filter("[\"==\", \"$id\", 1234]", {{}}, id1)); ASSERT_FALSE(filter("[\"==\", \"$id\", \"1234\"]", {{}}, id1)); + ASSERT_FALSE(filter("[\"==\", \"$id\", 1234]", {{"id", uint64_t(1234)}})); + ASSERT_FALSE(filter("[\"==\", \"$id\", \"1234\"]", {{"id", uint64_t(1234)}})); - FeatureIdentifier id2{std::string{"1"}}; + ASSERT_FALSE(filter("[\"<\", \"$id\", 0]", {{}}, id1)); + ASSERT_FALSE(filter("[\"<\", \"$id\", 1234]", {{}}, id1)); + ASSERT_TRUE(filter("[\"<=\", \"$id\", 1234]", {{}}, id1)); + ASSERT_FALSE(filter("[\"<\", \"$id\", 123]", {{}}, id1)); + ASSERT_FALSE(filter("[\"<=\", \"$id\", 123]", {{}}, id1)); + ASSERT_TRUE(filter("[\">\", \"$id\", 0]", {{}}, id1)); + ASSERT_TRUE(filter("[\">\", \"$id\", 123]", {{}}, id1)); + ASSERT_TRUE(filter("[\">=\", \"$id\", 123]", {{}}, id1)); + ASSERT_FALSE(filter("[\">\", \"$id\", 1234]", {{}}, id1)); + ASSERT_TRUE(filter("[\">=\", \"$id\", 1234]", {{}}, id1)); + + ASSERT_FALSE(filter("[\">\", \"$id\", \"1\"]", {{}}, id1)); + ASSERT_FALSE(filter("[\"<\", \"$id\", \"1\"]", {{}}, id1)); + ASSERT_FALSE(filter("[\">=\", \"$id\", \"1\"]", {{}}, id1)); + ASSERT_FALSE(filter("[\"<=\", \"$id\", \"1\"]", {{}}, id1)); + + const FeatureIdentifier id2{std::string{"1"}}; ASSERT_FALSE(filter("[\"<\", \"$id\", \"0\"]", {{}}, id2)); ASSERT_TRUE(filter("[\"<\", \"$id\", \"1234\"]", {{}}, id2)); + ASSERT_TRUE(filter("[\"<=\", \"$id\", \"1234\"]", {{}}, id2)); + ASSERT_FALSE(filter("[\"<\", \"$id\", \"012\"]", {{}}, id2)); + ASSERT_FALSE(filter("[\"<=\", \"$id\", \"012\"]", {{}}, id2)); + ASSERT_TRUE(filter("[\">\", \"$id\", \"0\"]", {{}}, id2)); + ASSERT_FALSE(filter("[\">\", \"$id\", \"234\"]", {{}}, id2)); + ASSERT_TRUE(filter("[\">=\", \"$id\", \"012\"]", {{}}, id2)); + ASSERT_FALSE(filter("[\">\", \"$id\", \"1234\"]", {{}}, id2)); + ASSERT_FALSE(filter("[\">=\", \"$id\", \"1234\"]", {{}}, id2)); - ASSERT_FALSE(filter("[\"==\", \"$id\", 1234]", {{"id", uint64_t(1234)}})); + ASSERT_FALSE(filter("[\">\", \"$id\", 1]", {{}}, id2)); + ASSERT_FALSE(filter("[\"<\", \"$id\", 1]", {{}}, id2)); + ASSERT_FALSE(filter("[\">=\", \"$id\", 1]", {{}}, id2)); + ASSERT_FALSE(filter("[\"<=\", \"$id\", 1]", {{}}, id2)); } TEST(Filter, Expression) { @@ -214,15 +243,80 @@ TEST(Filter, PropertyExpression) { } TEST(Filter, LegacyProperty) { + // < + + ASSERT_TRUE(filter("[\"<\", \"two\", 2]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\"<\", \"two\", \"2\"]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\"<\", \"two\", 2]", {{"two", std::string("1")}})); + ASSERT_TRUE(filter("[\"<\", \"two\", \"2\"]", {{"two", std::string("1")}})); + + ASSERT_FALSE(filter("[\"<\", \"two\", 2]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\"<\", \"two\", \"2\"]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\"<\", \"two\", 2]", {{"two", std::string("2")}})); + ASSERT_FALSE(filter("[\"<\", \"two\", \"2\"]", {{"two", std::string("2")}})); + + // <= + + ASSERT_TRUE(filter("[\"<=\", \"two\", 2]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\"<=\", \"two\", \"2\"]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\"<=\", \"two\", 2]", {{"two", std::string("1")}})); + ASSERT_TRUE(filter("[\"<=\", \"two\", \"2\"]", {{"two", std::string("1")}})); + ASSERT_TRUE(filter("[\"<=\", \"two\", 2]", {{"two", int64_t(2)}})); - ASSERT_FALSE(filter("[\"==\", \"two\", 4]", {{"two", int64_t(2)}})); - ASSERT_FALSE(filter("[\"<=\", \"two\", \"2\"]", {{"two", int64_t(2)}})); - ASSERT_FALSE(filter("[\"==\", \"bool\", false]", {{"two", true}})); - + ASSERT_FALSE(filter("[\"<=\", \"two\", 2]", {{"two", std::string("2")}})); ASSERT_TRUE(filter("[\"<=\", \"two\", \"2\"]", {{"two", std::string("2")}})); - ASSERT_FALSE(filter("[\"<\", \"two\", \"1\"]", {{"two", std::string("2")}})); + + ASSERT_FALSE(filter("[\"<=\", \"two\", 2]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\"<=\", \"two\", \"2\"]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\"<=\", \"two\", 2]", {{"two", std::string("3")}})); + ASSERT_FALSE(filter("[\"<=\", \"two\", \"2\"]", {{"two", std::string("3")}})); + + // > + + ASSERT_FALSE(filter("[\">\", \"two\", 2]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\">\", \"two\", \"2\"]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\">\", \"two\", 2]", {{"two", std::string("2")}})); + ASSERT_FALSE(filter("[\">\", \"two\", \"2\"]", {{"two", std::string("2")}})); + + ASSERT_TRUE(filter("[\">\", \"two\", 2]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\">\", \"two\", \"2\"]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\">\", \"two\", 2]", {{"two", std::string("3")}})); + ASSERT_TRUE(filter("[\">\", \"two\", \"2\"]", {{"two", std::string("3")}})); + + // >= + + ASSERT_FALSE(filter("[\">=\", \"two\", 2]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", \"2\"]", {{"two", int64_t(1)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", 2]", {{"two", std::string("1")}})); + ASSERT_FALSE(filter("[\">=\", \"two\", \"2\"]", {{"two", std::string("1")}})); + + ASSERT_TRUE(filter("[\">=\", \"two\", 2]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", \"2\"]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", 2]", {{"two", std::string("2")}})); + ASSERT_TRUE(filter("[\">=\", \"two\", \"2\"]", {{"two", std::string("2")}})); + + ASSERT_TRUE(filter("[\">=\", \"two\", 2]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", \"2\"]", {{"two", int64_t(3)}})); + ASSERT_FALSE(filter("[\">=\", \"two\", 2]", {{"two", std::string("3")}})); + ASSERT_TRUE(filter("[\">=\", \"two\", \"2\"]", {{"two", std::string("3")}})); + + // + + ASSERT_TRUE(filter("[\"==\", \"two\", 4]", {{"two", int64_t(4)}})); + ASSERT_FALSE(filter("[\"==\", \"two\", 4]", {{"two", std::string("4")}})); + ASSERT_FALSE(filter("[\"==\", \"two\", \"4\"]", {{"two", int64_t(4)}})); + ASSERT_TRUE(filter("[\"==\", \"two\", \"4\"]", {{"two", std::string("4")}})); + + ASSERT_FALSE(filter("[\"==\", \"two\", 4]", {{"two", int64_t(2)}})); ASSERT_FALSE(filter("[\"==\", \"two\", 4]", {{"two", std::string("2")}})); + ASSERT_FALSE(filter("[\"==\", \"two\", \"4\"]", {{"two", int64_t(2)}})); + ASSERT_FALSE(filter("[\"==\", \"two\", \"4\"]", {{"two", std::string("2")}})); + + ASSERT_FALSE(filter("[\"==\", \"one\", true]", {{"two", true}})); + ASSERT_FALSE(filter("[\"==\", \"one\", false]", {{"two", true}})); + ASSERT_TRUE(filter("[\"==\", \"two\", true]", {{"two", true}})); + ASSERT_FALSE(filter("[\"==\", \"two\", false]", {{"two", true}})); } TEST(Filter, Serialize) { @@ -254,6 +348,28 @@ TEST(Filter, ZoomExpressionNested) { R"(["==", ["get", "two"], ["+", ["zoom"], 1]])", {{"two", int64_t(2)}}, {}, FeatureType::Point, {}, 2.0f)); } +TEST(Filter, GeomType) { + ASSERT_TRUE(filter(R"(["==", ["geometry-type"], "Point"])", {{}}, {}, FeatureType::Point, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "LineString"])", {{}}, {}, FeatureType::Point, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Polygon"])", {{}}, {}, FeatureType::Point, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Unknown"])", {{}}, {}, FeatureType::Point, {}, 2.0f)); + + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Point"])", {{}}, {}, FeatureType::LineString, {}, 2.0f)); + ASSERT_TRUE(filter(R"(["==", ["geometry-type"], "LineString"])", {{}}, {}, FeatureType::LineString, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Polygon"])", {{}}, {}, FeatureType::LineString, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Unknown"])", {{}}, {}, FeatureType::LineString, {}, 2.0f)); + + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Point"])", {{}}, {}, FeatureType::Polygon, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "LineString"])", {{}}, {}, FeatureType::Polygon, {}, 2.0f)); + ASSERT_TRUE(filter(R"(["==", ["geometry-type"], "Polygon"])", {{}}, {}, FeatureType::Polygon, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Unknown"])", {{}}, {}, FeatureType::Polygon, {}, 2.0f)); + + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Point"])", {{}}, {}, FeatureType::Unknown, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "LineString"])", {{}}, {}, FeatureType::Unknown, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["geometry-type"], "Polygon"])", {{}}, {}, FeatureType::Unknown, {}, 2.0f)); + ASSERT_TRUE(filter(R"(["==", ["geometry-type"], "Unknown"])", {{}}, {}, FeatureType::Unknown, {}, 2.0f)); +} + TEST(Filter, Internal) { filter(R"(["filter-==","class","snow"])"); } diff --git a/test/style/property_expression.test.cpp b/test/style/property_expression.test.cpp index 45e14dc14..5811dc201 100644 --- a/test/style/property_expression.test.cpp +++ b/test/style/property_expression.test.cpp @@ -1,12 +1,11 @@ -#include -#include - -#include #include #include #include +#include #include - +#include +#include +#include #include #include @@ -19,6 +18,10 @@ using namespace mbgl::style::expression::dsl; using namespace std::string_literals; +namespace std { +using ::mbgl::operator<<; +} + static const StubGeometryTileFeature oneInteger{PropertyMap{{"property", uint64_t(1)}}}; static const StubGeometryTileFeature oneDouble{PropertyMap{{"property", 1.0}}}; @@ -52,19 +55,26 @@ TEST(PropertyExpression, Constant) { EXPECT_EQ(2.0f, evaluate(PropertyValue(2.0f), 22)); EXPECT_EQ(3.8f, evaluate(PropertyValue(3.8f), 22)); EXPECT_EQ(22.0f, evaluate(PropertyValue(22.0f), 22)); + + EXPECT_EQ(Dependency::None, PropertyValue(2.0f).getDependencies()); } TEST(PropertyExpression, Expression) { PropertyExpression expression(interpolate(linear(), zoom(), 0.0, literal(0.0), 1.0, literal(1.0))); EXPECT_EQ(0.0, evaluate(expression, 0.0)); EXPECT_EQ(0.5, evaluate(expression, 0.5)); + EXPECT_EQ(Dependency::Zoom, expression.getDependencies()); } TEST(PropertyExpression, Defaults) { - EXPECT_EQ(1.0f, PropertyExpression(number(get("property")), 0.0f).evaluate(oneInteger, 2.0f)); - EXPECT_EQ(1.0f, PropertyExpression(number(get("property")), 0.0f).evaluate(oneDouble, 2.0f)); - EXPECT_EQ(0.0f, PropertyExpression(number(get("property")), 0.0f).evaluate(oneString, 2.0f)); - EXPECT_EQ(2.0f, PropertyExpression(number(get("property"))).evaluate(oneString, 2.0f)); + const PropertyExpression noDefault(number(get("property"))); + const PropertyExpression withDefault(number(get("property")), 0.0f); + EXPECT_EQ(1.0f, withDefault.evaluate(oneInteger, 2.0f)); + EXPECT_EQ(1.0f, withDefault.evaluate(oneDouble, 2.0f)); + EXPECT_EQ(0.0f, withDefault.evaluate(oneString, 2.0f)); + EXPECT_EQ(2.0f, noDefault.evaluate(oneString, 2.0f)); + + EXPECT_EQ(Dependency::Feature, noDefault.getDependencies()); } TEST(PropertyExpression, ZoomInterpolation) { @@ -166,6 +176,8 @@ TEST(PropertyExpression, FormatSectionOverride) { PossiblyEvaluatedPropertyValue constantValueGreen(Color::green()); PossiblyEvaluatedPropertyValue ddsValueRed(toColor(string(get("color")))); + EXPECT_EQ(Dependency::Feature, ddsValueRed.getDependencies()); + // Evaluation test { auto override1 = createOverride(expression::type::Color, constantValueGreen, "text-color"); @@ -174,6 +186,9 @@ TEST(PropertyExpression, FormatSectionOverride) { EXPECT_EQ(Color::green(), propExpr.evaluate(oneDouble, Color())); EXPECT_EQ(Color::blue(), propExpr.evaluate(ctx)); + /// Override implies a feature dependency (See `::isFeatureConstant`) + EXPECT_EQ(Dependency::Override | Dependency::Feature, propExpr.getDependencies()); + auto override2 = createOverride(expression::type::Color, ddsValueRed, "text-color"); PropertyExpression propExprDDS(std::move(override2)); EXPECT_EQ(Color::red(), propExprDDS.evaluate(oneColor, Color())); @@ -203,6 +218,8 @@ TEST(PropertyExpression, ImageExpression) { EXPECT_FALSE(evaluatedImage.isAvailable()); EXPECT_EQ(evaluatedImage.id(), "airport-11"s); + EXPECT_EQ(Dependency::Image, propExpr.getDependencies()); + PropertyExpression ddPropExpr(image(get(literal("image_name"s)))); evaluatedImage = ddPropExpr.evaluate(oneImage, emptySet, expression::Image()); EXPECT_FALSE(evaluatedImage.isAvailable()); @@ -220,11 +237,15 @@ TEST(PropertyExpression, ImageExpression) { EXPECT_TRUE(evaluatedImage.isAvailable()); EXPECT_EQ(evaluatedImage.id(), "airport-11"s); + EXPECT_EQ(Dependency::Image, propExpr.getDependencies()); + PropertyExpression ddPropExpr(image(get(literal("image_name"s)))); evaluatedImage = ddPropExpr.evaluate(oneImage, availableImages, expression::Image()); EXPECT_TRUE(evaluatedImage.isAvailable()); EXPECT_EQ(evaluatedImage.id(), "maki-11"s); + EXPECT_EQ(Dependency::Image | Dependency::Feature, ddPropExpr.getDependencies()); + evaluatedImage = ddPropExpr.evaluate(emptyTileFeature, availableImages, expression::Image()); EXPECT_FALSE(evaluatedImage.isAvailable()); EXPECT_EQ(evaluatedImage.id(), ""s); @@ -240,6 +261,8 @@ TEST(PropertyExpression, ImageExpression) { evaluatedImage = propExpr.evaluate(18.0, emptyTileFeature, availableImages, expression::Image()); EXPECT_TRUE(evaluatedImage.isAvailable()); EXPECT_EQ(evaluatedImage.id(), "bicycle-15"s); + + EXPECT_EQ(Dependency::Image | Dependency::Zoom, propExpr.getDependencies()); } } @@ -287,6 +310,8 @@ TEST(PropertyExpression, WithinExpression) { ASSERT_TRUE(expression); PropertyExpression propExpr(std::move(expression)); + EXPECT_EQ(Dependency::Feature, propExpr.getDependencies()); + // evaluation test with valid geojson source but FeatureType is not Point/LineString // (currently only support FeatureType::Point and FeatureType::LineString) { @@ -536,6 +561,8 @@ TEST(PropertyExpression, DistanceExpression) { ASSERT_TRUE(expression); PropertyExpression propExpr(std::move(expression)); + EXPECT_EQ(Dependency::Feature, propExpr.getDependencies()); + auto evaluatedResult = propExpr.evaluate(EvaluationContext(&pointFeature).withCanonicalTileID(&canonicalTileID), invalidResult); EXPECT_NEAR(491.307, evaluatedResult, 0.01); diff --git a/test/style/style.test.cpp b/test/style/style.test.cpp index cfcb145a1..cd453691a 100644 --- a/test/style/style.test.cpp +++ b/test/style/style.test.cpp @@ -82,14 +82,12 @@ TEST(Style, DuplicateSource) { TEST(Style, RemoveSourceInUse) { util::RunLoop loop; - auto log = new FixtureLogObserver(); - Log::setObserver(std::unique_ptr(log)); + FixtureLog log; auto fileSource = std::make_shared(); Style::Impl style{fileSource, 1.0}; style.loadJSON(util::read_file("test/fixtures/resources/style-unused-sources.json")); - style.addSource(std::make_unique("sourceId", "mptiler://tiles/contours")); style.addLayer(std::make_unique("layerId", "sourceId")); @@ -109,7 +107,7 @@ TEST(Style, RemoveSourceInUse) { Sleep(1000); #endif - EXPECT_EQ(log->count(logMessage), 1u); + EXPECT_EQ(log.count(logMessage), 1u); } TEST(Style, SourceImplsOrder) { diff --git a/test/style/style_layer.test.cpp b/test/style/style_layer.test.cpp index 3ae368c32..0a4cae49a 100644 --- a/test/style/style_layer.test.cpp +++ b/test/style/style_layer.test.cpp @@ -353,7 +353,7 @@ void testHasOverrides(LayoutType& layout) { // Expression, overridden text-color. FormatExpressionSection section(literal("")); - section.setTextSectionOptions(std::nullopt, std::nullopt, toColor(literal("red"))); + section.setTextSectionOptions({}, {}, toColor(literal("red"))); auto formatExprOverride = std::make_unique(std::vector{section}); PropertyExpression propExprOverride(std::move(formatExprOverride)); layout.template get() = PropertyValueType(std::move(propExprOverride)); @@ -362,7 +362,7 @@ void testHasOverrides(LayoutType& layout) { // Nested expressions, overridden text-color. auto formattedExpr1 = format("first paragraph"); FormatExpressionSection secondParagraph(literal("second paragraph")); - secondParagraph.setTextSectionOptions(std::nullopt, std::nullopt, toColor(literal("blue"))); + secondParagraph.setTextSectionOptions({}, {}, toColor(literal("blue"))); std::vector sections{{std::move(secondParagraph)}}; auto formattedExpr2 = std::make_unique(std::move(sections)); std::unordered_map> branches{{"1st", std::move(formattedExpr1)}, diff --git a/test/style/style_parser.test.cpp b/test/style/style_parser.test.cpp index 8b44a70eb..00c7abbf4 100644 --- a/test/style/style_parser.test.cpp +++ b/test/style/style_parser.test.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -32,18 +33,18 @@ class StyleParserTest : public ::testing::TestWithParam {}; TEST_P(StyleParserTest, ParseStyle) { const std::string base = std::string("test/fixtures/style_parser/") + GetParam(); + using namespace std::string_literals; + SCOPED_TRACE("Loading: "s + base); + + FixtureLog log; + rapidjson::GenericDocument, rapidjson::CrtAllocator> infoDoc; infoDoc.Parse<0>(util::read_file(base + ".info.json").c_str()); ASSERT_FALSE(infoDoc.HasParseError()); ASSERT_TRUE(infoDoc.IsObject()); - auto observer = new FixtureLogObserver(); - Log::setObserver(std::unique_ptr(observer)); - style::Parser parser; - auto error = parser.parse(util::read_file(base + ".style.json")); - - if (error) { + if (auto error = parser.parse(util::read_file(base + ".style.json"))) { Log::Error(Event::ParseStyle, "Failed to parse style: " + util::toString(error)); } @@ -71,11 +72,14 @@ TEST_P(StyleParserTest, ParseStyle) { Sleep(10); #endif - EXPECT_EQ(count, observer->count(message)) << "Message: " << message << std::endl; + SCOPED_TRACE("Checking: "s + message.msg); + + const auto observedCount = log.count(message); + EXPECT_EQ(count, observedCount) << "Message: " << message << std::endl; } } - const auto& unchecked = observer->unchecked(); + const auto& unchecked = log.unchecked(); if (unchecked.size()) { std::cerr << "Unchecked Log Messages (" << base << "/" << name << "): " << std::endl << unchecked; } @@ -292,3 +296,21 @@ TEST(StyleParser, FontStacksGetExpression) { auto result = parser.fontStacks(); ASSERT_EQ(0u, result.size()); } + +TEST(StyleParser, ZoomCurve) { + using namespace mbgl::style; + using namespace mbgl::style::expression; + using namespace mbgl::style::expression::dsl; + + const auto zoomInterp = []() { + return interpolate(linear(), zoom(), 0.0, literal(0.0), 0.0, literal(0.0)); + }; + + auto expr1 = interpolate(linear(), literal(0.0), 0.0, zoomInterp(), 0.0, zoomInterp()); + ASSERT_TRUE(expr1); + ASSERT_TRUE(findZoomCurveChecked(*expr1).is()); + + auto expr2 = interpolate(linear(), zoom(), 0.0, literal(0.0), 0.0, zoomInterp()); + ASSERT_TRUE(expr2); + ASSERT_TRUE(findZoomCurveChecked(*expr2).is()); +}