Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] Traverse expression tree when checking for property overrides
Browse files Browse the repository at this point in the history
Before this change, symbol layer was only checking whether top level
'text-field' layout property expression is FormatExpression and if it
has paint property overrides. This change takes into account that
'text-field' might have nested expressions, thus, requires traversal
over child expressions.

Fixes: #14254
  • Loading branch information
alexshalamov committed Mar 28, 2019
1 parent 2115577 commit 4e335d0
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 14 deletions.
5 changes: 5 additions & 0 deletions src/mbgl/style/expression/value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ mbgl::Value ValueConverter<mbgl::Value>::fromExpressionValue(const Value& value)
}
options.emplace("text-font", std::vector<mbgl::Value>{ std::string("literal"), fontStack });
}

if (section.textColor) {
options.emplace("text-color", fromExpressionValue(*section.textColor));
}

serialized.push_back(options);
}
return serialized;
Expand Down
62 changes: 48 additions & 14 deletions src/mbgl/style/layers/symbol_layer_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
#include <mbgl/style/layer_impl.hpp>
#include <mbgl/style/layers/symbol_layer.hpp>
#include <mbgl/style/layers/symbol_layer_properties.hpp>
#include <mbgl/style/expression/literal.hpp>
#include <mbgl/style/expression/format_expression.hpp>
#include <mbgl/style/expression/formatted.hpp>
#include <mbgl/style/expression/format_section_override.hpp>
#include <mbgl/style/expression/value.hpp>

namespace mbgl {
namespace style {
Expand Down Expand Up @@ -56,25 +58,57 @@ struct FormatSectionOverrides<TypeList<PaintProperty...>> {

template<typename Property, typename FormattedProperty>
static bool hasOverride(const FormattedProperty& formatted) {

const auto checkLiteral = [] (const TextField::Type& literal) {
for (const auto& section : literal.sections) {
if (Property::hasOverride(section)) {
return true;
}
}
return false;
};

return formatted.match(
[] (const TextField::Type& t) {
for (const auto& section : t.sections) {
if (Property::hasOverride(section)) {
return true;
}
}
return false;
[&checkLiteral] (const TextField::Type& literal) {
return checkLiteral(literal);
},
[] (const PropertyExpression<TextField::Type>& t) {
if (t.getExpression().getKind() == expression::Kind::FormatExpression) {
const auto* e = static_cast<const expression::FormatExpression*>(&t.getExpression());
for (const auto& section : e->getSections()) {
if (Property::hasOverride(section)) {
return true;
[&checkLiteral] (const PropertyExpression<TextField::Type>& property) {
bool expressionHasOverrides = false;
const auto checkExpression = [&](const expression::Expression& e) {
if (expressionHasOverrides) {
return;
}

if (e.getKind() == expression::Kind::Literal &&
e.getType() == expression::type::Formatted) {
const auto* literalExpr = static_cast<const expression::Literal*>(&e);
const auto formattedValue = expression::fromExpressionValue<expression::Formatted>(literalExpr->getValue());
if (formattedValue && checkLiteral(*formattedValue)) {
expressionHasOverrides = true;
}
return;
}

if (e.getKind() == expression::Kind::FormatExpression) {
const auto* formatExpr = static_cast<const expression::FormatExpression*>(&e);
for (const auto& section : formatExpr->getSections()) {
if (Property::hasOverride(section)) {
expressionHasOverrides = true;
break;
}
}
}
};

// Check root property expression and return early.
checkExpression(property.getExpression());
if (expressionHasOverrides) {
return true;
}
return false;

// Traverse thru children and check whether any of them have overrides.
property.getExpression().eachChild(checkExpression);
return expressionHasOverrides;
},
[] (const auto&) {
return false;
Expand Down
12 changes: 12 additions & 0 deletions test/style/style_layer.test.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <mbgl/style/expression/dsl.hpp>
#include <mbgl/style/expression/match.hpp>
#include <mbgl/style/expression/format_expression.hpp>
#include <mbgl/style/style_impl.hpp>
#include <mbgl/style/layers/background_layer.hpp>
Expand Down Expand Up @@ -330,6 +331,17 @@ void testHasOverrides(LayoutType& layout) {
PropertyExpression<Formatted> propExprOverride(std::move(formatExprOverride));
layout.template get<TextField>() = PropertyValueType<Formatted>(std::move(propExprOverride));
EXPECT_TRUE(MockOverrides::hasOverrides(layout.template get<TextField>()));

// Nested expressions, overridden text-color.
auto formattedExpr1 = format("first paragraph");
std::vector<FormatExpressionSection> sections{ { literal("second paragraph"), nullopt, nullopt, toColor(literal("blue")) } };
auto formattedExpr2 = std::make_unique<FormatExpression>(std::move(sections));
std::unordered_map<std::string, std::shared_ptr<Expression>> branches{ { "1st", std::move(formattedExpr1) },
{ "2nd", std::move(formattedExpr2) } };
auto match = std::make_unique<Match<std::string>>(type::Formatted, literal("input"), std::move(branches), format("otherwise"));
PropertyExpression<Formatted> nestedPropExpr(std::move(match));
layout.template get<TextField>() = PropertyValueType<Formatted>(std::move(nestedPropExpr));
EXPECT_TRUE(MockOverrides::hasOverrides(layout.template get<TextField>()));
}

} // namespace
Expand Down

0 comments on commit 4e335d0

Please sign in to comment.