Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track expression dependencies #2113

Merged
merged 35 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
c8dc9cc
1
TimSylvester Feb 14, 2024
27e1fc5
warning
TimSylvester Feb 16, 2024
3079d78
warning
TimSylvester Feb 16, 2024
24143b0
warnings
TimSylvester Feb 16, 2024
3ca788c
uninitialized access bug
TimSylvester Feb 19, 2024
4a4316b
Merge branch 'main' into expr-deps
TimSylvester Feb 19, 2024
71e1204
Merge branch 'main' into expr-deps
TimSylvester Feb 23, 2024
064907e
sync with noexcept branch to reduce conflicts
TimSylvester Feb 23, 2024
80ce2cd
Fix generated code, warnings
TimSylvester Feb 23, 2024
7799da1
Merge branch 'main' into expr-deps
TimSylvester Feb 23, 2024
9786960
Add more dependency types, show layer summary on style load.
TimSylvester Feb 26, 2024
8a39773
Merge branch 'main' into expr-deps
TimSylvester Feb 26, 2024
acb99b9
warnings, more benchmark precision
TimSylvester Feb 27, 2024
7aca922
Merge branch 'main' into expr-deps
TimSylvester Feb 27, 2024
d5fc87d
refactor
TimSylvester Feb 27, 2024
06aad57
remove `make_tuple`
TimSylvester Feb 27, 2024
8bae842
Merge branch 'main' into expr-deps
TimSylvester Feb 27, 2024
c6aeecb
Simplify template iteration
TimSylvester Feb 28, 2024
4b56aea
Add streaming formatter for better test output
TimSylvester Feb 28, 2024
05bd61d
remove redundant call to `isZoomConstant`
TimSylvester Feb 28, 2024
4a402cb
improve coverage
TimSylvester Feb 28, 2024
5a6434e
Use `FixtureLog` instead of using `FixtureLogObserver` directly, to g…
TimSylvester Feb 28, 2024
62205b6
Check dependencies.
TimSylvester Feb 28, 2024
f47f62f
expression dependency tests
TimSylvester Feb 28, 2024
9b210ce
Merge branch 'main' into expr-deps
TimSylvester Feb 28, 2024
8c24adc
fix warnings
TimSylvester Feb 29, 2024
469b286
warning
TimSylvester Feb 29, 2024
4d00fbf
Use `FixtureLog` to avoid threading issues
TimSylvester Feb 29, 2024
3d6367b
improve coverage
TimSylvester Feb 29, 2024
58cb8e2
improve coverage
TimSylvester Feb 29, 2024
ab1100b
improve coverage
TimSylvester Mar 1, 2024
cd713d7
Merge branch 'main' into expr-deps
TimSylvester Mar 4, 2024
1da0031
Merge branch 'main' into expr-deps
TimSylvester Mar 5, 2024
61c0898
Merge branch 'main' into expr-deps
TimSylvester Mar 6, 2024
c0d719a
Merge branch 'main' into expr-deps
TimSylvester Mar 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions include/mbgl/style/color_ramp_property_value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ class ColorRampPropertyValue {
}

public:
ColorRampPropertyValue()
: value(nullptr) {}
ColorRampPropertyValue() noexcept = default;
ColorRampPropertyValue(std::shared_ptr<expression::Expression> value_)
: value(std::move(value_)) {}

Expand All @@ -46,6 +45,9 @@ class ColorRampPropertyValue {
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
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/at.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace expression {
class At : public Expression {
public:
At(std::unique_ptr<Expression> index_, std::unique_ptr<Expression> input_)
: Expression(Kind::At, input_->getType().get<type::Array>().itemType),
: Expression(Kind::At, input_->getType().get<type::Array>().itemType, depsOf(index_) | depsOf(input_)),
index(std::move(index_)),
input(std::move(input_)) {}

Expand Down
4 changes: 2 additions & 2 deletions include/mbgl/style/expression/boolean_operator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace expression {
class Any : public Expression {
public:
Any(std::vector<std::unique_ptr<Expression>> 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);
Expand All @@ -31,7 +31,7 @@ class Any : public Expression {
class All : public Expression {
public:
All(std::vector<std::unique_ptr<Expression>> 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);
Expand Down
8 changes: 7 additions & 1 deletion include/mbgl/style/expression/case.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Case : public Expression {
using Branch = std::pair<std::unique_ptr<Expression>, std::unique_ptr<Expression>>;

Case(type::Type type_, std::vector<Branch> branches_, std::unique_ptr<Expression> 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_)) {}

Expand All @@ -32,6 +32,12 @@ class Case : public Expression {

std::string getOperator() const override { return "case"; }

protected:
using Expression::collectDependencies;
static Dependency collectDependencies(const std::vector<Branch>& branches) {
return collectDependencies(branches, [](const Branch& v) { return depsOf(v.first) | depsOf(v.second); });
}

private:
std::vector<Branch> branches;
std::unique_ptr<Expression> otherwise;
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/coalesce.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Coalesce : public Expression {
public:
using Args = std::vector<std::unique_ptr<Expression>>;
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);
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/collator_expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class CollatorExpression : public Expression {

void eachChild(const std::function<void(const Expression&)>&) const override;

bool operator==(const Expression& e) const override;
bool operator==(const Expression&) const override;

std::vector<std::optional<Value>> possibleOutputs() const override {
// Technically the set of possible outputs is the combinatoric set of
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void(const Expression&)>&) const override {}
Expand Down
99 changes: 82 additions & 17 deletions include/mbgl/style/expression/expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
#include <mbgl/style/expression/value.hpp>
#include <mbgl/tile/tile_id.hpp>
#include <mbgl/util/color.hpp>
#include <mbgl/util/traits.hpp>
#include <mbgl/util/variant.hpp>

#include <array>
#include <vector>
#include <memory>
#include <numeric>
#include <optional>
#include <vector>

namespace mbgl {

Expand Down Expand Up @@ -135,23 +137,23 @@ class EvaluationResult : public Result<Value> {
};

/**
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<U> 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<U> 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,
Expand Down Expand Up @@ -183,10 +185,29 @@ 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) {
return Dependency{mbgl::underlying_type(x) | mbgl::underlying_type(y)};
}
inline constexpr static Dependency operator&(Dependency x, Dependency y) {
return Dependency{mbgl::underlying_type(x) & mbgl::underlying_type(y)};
}

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;

Expand Down Expand Up @@ -229,6 +250,8 @@ class Expression {

virtual std::string getOperator() const = 0;

const Dependency dependencies;

protected:
template <typename T>
static bool childrenEqual(const T& lhs, const T& rhs) {
Expand Down Expand Up @@ -261,11 +284,53 @@ class Expression {
return *(lhs.first) == *(rhs.first) && *(lhs.second) == *(rhs.second);
}

static Dependency depsOf(const std::shared_ptr<Expression>& ex) { return ex ? ex->dependencies : Dependency::None; }
static Dependency depsOf(const std::unique_ptr<Expression>& ex) { return ex ? ex->dependencies : Dependency::None; }

template <typename T>
static Dependency depsOf(const std::optional<T>& 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 <typename T>
static constexpr Dependency collectDependencies(const std::vector<T>& container) {
return std::accumulate(container.begin(), container.end(), Dependency::None, [](auto a, const auto& ex) {
return a | depsOf(ex);
});
}

template <typename T, typename F>
static constexpr Dependency collectDependencies(const std::vector<T>& container, F op) {
return std::accumulate(container.begin(), container.end(), Dependency::None, [&](auto a, const auto& item) {
return a | op(item);
});
}

template <typename K, typename T>
static constexpr Dependency collectDependencies(const std::map<K, T>& container) {
return std::accumulate(container.begin(), container.end(), Dependency::None, [](auto a, const auto& v) {
return a | depsOf(v.second);
});
}

template <typename K, typename T>
static constexpr Dependency collectDependencies(const std::unordered_map<K, T>& 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;
};

} // namespace expression
} // namespace style

namespace util {
std::string toString(style::expression::Dependency);
} // namespace util

} // namespace mbgl
17 changes: 11 additions & 6 deletions include/mbgl/style/expression/format_expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ namespace expression {
struct FormatExpressionSection {
explicit FormatExpressionSection(std::unique_ptr<Expression> content_);

void setTextSectionOptions(std::optional<std::unique_ptr<Expression>> fontScale_,
std::optional<std::unique_ptr<Expression>> textFont_,
std::optional<std::unique_ptr<Expression>> textColor_);
void setTextSectionOptions(std::unique_ptr<Expression>&& fontScale_,
std::unique_ptr<Expression>&& textFont_,
std::unique_ptr<Expression>&& textColor_);

// Content can be expression that evaluates to String or Image.
std::shared_ptr<Expression> content;

// Text related section options.
std::optional<std::shared_ptr<Expression>> fontScale;
std::optional<std::shared_ptr<Expression>> textFont;
std::optional<std::shared_ptr<Expression>> textColor;
std::shared_ptr<Expression> fontScale;
std::shared_ptr<Expression> textFont;
std::shared_ptr<Expression> textColor;
};

class FormatExpression final : public Expression {
Expand All @@ -45,6 +45,11 @@ class FormatExpression final : public Expression {
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<FormatExpressionSection> sections;
};
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/format_section_override.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class FormatSectionOverride final : public Expression {
FormatSectionOverride(const type::Type& type_,
PossiblyEvaluatedPropertyValue<T> defaultValue_,
std::string propertyName_)
: Expression(Kind::FormatSectionOverride, type_),
: Expression(Kind::FormatSectionOverride, type_, defaultValue_.getDependencies() | Dependency::Override),
defaultValue(std::move(defaultValue_)),
propertyName(std::move(propertyName_)) {}

Expand Down
4 changes: 2 additions & 2 deletions include/mbgl/style/expression/let.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Let : public Expression {
using Bindings = std::map<std::string, std::shared_ptr<Expression>>;

Let(Bindings bindings_, std::unique_ptr<Expression> result_)
: Expression(Kind::Let, result_->getType()),
: Expression(Kind::Let, result_->getType(), collectDependencies(bindings_) | Dependency::Bind),
bindings(std::move(bindings_)),
result(std::move(result_)) {}

Expand Down Expand Up @@ -49,7 +49,7 @@ class Let : public Expression {
class Var : public Expression {
public:
Var(std::string name_, std::shared_ptr<Expression> value_)
: Expression(Kind::Var, value_->getType()),
: Expression(Kind::Var, value_->getType(), depsOf(value_) | Dependency::Var),
name(std::move(name_)),
value(std::move(value_)) {}

Expand Down
4 changes: 2 additions & 2 deletions include/mbgl/style/expression/literal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> value_)
: Expression(Kind::Literal, type_),
: Expression(Kind::Literal, type_, Dependency::None),
value(std::move(value_)) {}

EvaluationResult evaluate(const EvaluationContext&) const override { return value; }
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/style/expression/match.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Match : public Expression {
std::unique_ptr<Expression> input_,
Branches branches_,
std::unique_ptr<Expression> 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_)) {}
Expand Down
6 changes: 6 additions & 0 deletions include/mbgl/style/layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

namespace mbgl {
namespace style {
namespace expression {
enum class Dependency : uint32_t;
} // namespace expression

class LayerObserver;
class Filter;
Expand Down Expand Up @@ -159,6 +162,9 @@ class Layer {

mapbox::base::WeakPtr<Layer> makeWeakPtr() { return weakFactory.makeWeakPtr(); }

/// Collect dependencies
expression::Dependency getDependencies() const noexcept;

protected:
virtual Mutable<Impl> mutableBaseImpl() const = 0;
void serializeProperty(Value&, const StyleProperty&, const char* propertyName, bool isPaint) const;
Expand Down
6 changes: 4 additions & 2 deletions include/mbgl/style/layer_properties.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,20 @@ class LayerProperties {
/// Contains render passes used by the renderer, see `mbgl::RenderPass`.
uint8_t renderPasses = 0u;

virtual expression::Dependency getDependencies() const noexcept = 0;

protected:
LayerProperties(Immutable<Layer::Impl> impl)
: baseImpl(std::move(impl)) {}
};

template <class Derived>
inline const auto& getEvaluated(const Immutable<LayerProperties>& properties) {
inline const auto& getEvaluated(const Immutable<LayerProperties>& properties) noexcept {
return static_cast<const Derived&>(*properties).evaluated;
}

template <class Derived>
inline const auto& getCrossfade(const Immutable<LayerProperties>& properties) {
inline const auto& getCrossfade(const Immutable<LayerProperties>& properties) noexcept {
return static_cast<const Derived&>(*properties).crossfade;
}

Expand Down