tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't enforce ["zoom"] constraints for filters (#11672)

Browse Source 
* Don't enforce ["zoom"] constraints for filters

Fixes #11594

* Add a couple of comments

* Fix ambiguous constructor call

* ParsingContext(optional<Type>) => ParsingContext(Type)
This commit is contained in:
Anand Thakker
2018-04-12 16:02:48 -04:00
committed by GitHub
parent 646cbc98dd
commit 352d63d2ce
11 changed files with 88 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
cmake/core-files.cmake
View File

@@ -410,7 +410,6 @@ set(MBGL_CORE_FILES
include/mbgl/style/conversion/coordinate.hpp
include/mbgl/style/conversion/custom_geometry_source_options.hpp
include/mbgl/style/conversion/data_driven_property_value.hpp
include/mbgl/style/conversion/expression.hpp
include/mbgl/style/conversion/filter.hpp
include/mbgl/style/conversion/function.hpp
include/mbgl/style/conversion/geojson.hpp

14
include/mbgl/style/conversion/data_driven_property_value.hpp
View File

@@ -4,12 +4,12 @@
#include <mbgl/style/conversion.hpp>
#include <mbgl/style/conversion/constant.hpp>
#include <mbgl/style/conversion/function.hpp>
#include <mbgl/style/conversion/expression.hpp>
#include <mbgl/style/expression/is_expression.hpp>
#include <mbgl/style/expression/is_constant.hpp>
#include <mbgl/style/expression/find_zoom_curve.hpp>
#include <mbgl/style/expression/literal.hpp>
#include <mbgl/style/expression/value.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
#include <unordered_set>
@@ -22,15 +22,15 @@ template <class T>
struct Converter<DataDrivenPropertyValue<T>> {
optional<DataDrivenPropertyValue<T>> operator()(const Convertible& value, Error& error) const {
using namespace mbgl::style::expression;
if (isUndefined(value)) {
return DataDrivenPropertyValue<T>();
} else if (expression::isExpression(value)) {
optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(
value,
error,
valueTypeToExpressionType<T>());
} else if (isExpression(value)) {
ParsingContext ctx(valueTypeToExpressionType<T>());
ParseResult expression = ctx.parseLayerPropertyExpression(value);
if (!expression) {
error = { ctx.getCombinedErrors() };
return {};
}

39
include/mbgl/style/conversion/expression.hpp
View File

@@ -1,39 +0,0 @@
#pragma once
#include <mbgl/style/expression/parsing_context.hpp>
#include <mbgl/style/expression/type.hpp>
#include <mbgl/style/conversion.hpp>
#include <memory>
namespace mbgl {
namespace style {
namespace conversion {
using namespace mbgl::style::expression;
template<> struct Converter<std::unique_ptr<Expression>> {
optional<std::unique_ptr<Expression>> operator()(const Convertible& value, Error& error, type::Type expected) const {
ParsingContext ctx(optional<type::Type> {expected});
ParseResult parsed = ctx.parse(value);
if (parsed) {
return std::move(*parsed);
}
std::string combinedError;
for (const ParsingError& parsingError : ctx.getErrors()) {
if (combinedError.size() > 0) {
combinedError += "\n";
}
if (parsingError.key.size() > 0) {
combinedError += parsingError.key + ": ";
}
combinedError += parsingError.message;
}
error = { combinedError };
return {};
};
};
} // namespace conversion
} // namespace style
} // namespace mbgl

7
include/mbgl/style/conversion/heatmap_color_property_value.hpp
View File

@@ -4,11 +4,11 @@
#include <mbgl/style/conversion.hpp>
#include <mbgl/style/conversion/constant.hpp>
#include <mbgl/style/conversion/function.hpp>
#include <mbgl/style/conversion/expression.hpp>
#include <mbgl/style/expression/value.hpp>
#include <mbgl/style/expression/is_constant.hpp>
#include <mbgl/style/expression/is_expression.hpp>
#include <mbgl/style/expression/find_zoom_curve.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
namespace mbgl {
namespace style {
@@ -17,11 +17,14 @@ namespace conversion {
template <>
struct Converter<HeatmapColorPropertyValue> {
optional<HeatmapColorPropertyValue> operator()(const Convertible& value, Error& error) const {
using namespace mbgl::style::expression;
if (isUndefined(value)) {
return HeatmapColorPropertyValue();
} else if (isExpression(value)) {
optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, expression::type::Color);
ParsingContext ctx(type::Color);
ParseResult expression = ctx.parseLayerPropertyExpression(value);
if (!expression) {
error = { ctx.getCombinedErrors() };
return {};
}
assert(*expression);

9
include/mbgl/style/conversion/property_value.hpp
View File

@@ -4,11 +4,11 @@
#include <mbgl/style/conversion.hpp>
#include <mbgl/style/conversion/constant.hpp>
#include <mbgl/style/conversion/function.hpp>
#include <mbgl/style/conversion/expression.hpp>
#include <mbgl/style/expression/value.hpp>
#include <mbgl/style/expression/is_constant.hpp>
#include <mbgl/style/expression/is_expression.hpp>
#include <mbgl/style/expression/find_zoom_curve.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
namespace mbgl {
namespace style {
@@ -17,13 +17,18 @@ namespace conversion {
template <class T>
struct Converter<PropertyValue<T>> {
optional<PropertyValue<T>> operator()(const Convertible& value, Error& error) const {
using namespace mbgl::style::expression;
if (isUndefined(value)) {
return PropertyValue<T>();
} else if (isExpression(value)) {
optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, valueTypeToExpressionType<T>());
ParsingContext ctx(valueTypeToExpressionType<T>());
ParseResult expression = ctx.parseLayerPropertyExpression(value);
if (!expression) {
error = { ctx.getCombinedErrors() };
return {};
}
if (isFeatureConstant(**expression)) {
return { CameraFunction<T>(std::move(*expression)) };
} else {

34
include/mbgl/style/expression/parsing_context.hpp
View File

@@ -55,7 +55,7 @@ class ParsingContext {
public:
ParsingContext() : errors(std::make_shared<std::vector<ParsingError>>()) {}
ParsingContext(std::string key_) : key(std::move(key_)), errors(std::make_shared<std::vector<ParsingError>>()) {}
explicit ParsingContext(optional<type::Type> expected_)
explicit ParsingContext(type::Type expected_)
: expected(std::move(expected_)),
errors(std::make_shared<std::vector<ParsingError>>())
{}
@@ -67,6 +67,7 @@ public:
std::string getKey() const { return key; }
optional<type::Type> getExpected() const { return expected; }
const std::vector<ParsingError>& getErrors() const { return *errors; }
const std::string getCombinedErrors() const;
enum TypeAnnotationOption {
includeTypeAnnotations,
@@ -74,16 +75,21 @@ public:
};
/*
Parse the given style-spec JSON value into an Expression object.
Specifically, this function is responsible for determining the expression
type (either Literal, or the one named in value[0]) and dispatching to the
appropriate ParseXxxx::parse(const V&, ParsingContext) method.
Parse the given style-spec JSON value as an expression.
*/
ParseResult parse(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
ParseResult parseExpression(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
/*
Parse a child expression.
Parse the given style-spec JSON value as an expression intended to be used
in a layout or paint property. This entails checking additional constraints
that exist in that context but not, e.g., for filters.
*/
ParseResult parseLayerPropertyExpression(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
/*
Parse a child expression. For use by individual Expression::parse() methods.
*/
ParseResult parse(const mbgl::style::conversion::Convertible&,
std::size_t,
@@ -91,7 +97,7 @@ public:
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
/*
Parse a child expression.
Parse a child expression. For use by individual Expression::parse() methods.
*/
ParseResult parse(const mbgl::style::conversion::Convertible&,
std::size_t index,
@@ -141,6 +147,16 @@ private:
errors(std::move(errors_))
{}
/*
Parse the given style-spec JSON value into an Expression object.
Specifically, this function is responsible for determining the expression
type (either Literal, or the one named in value[0]) and dispatching to the
appropriate ParseXxxx::parse(const V&, ParsingContext) method.
*/
ParseResult parse(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
std::string key;
optional<type::Type> expected;
std::shared_ptr<detail::Scope> scope;

4
platform/node/src/node_expression.cpp
View File

@@ -75,8 +75,8 @@ void NodeExpression::Parse(const Nan::FunctionCallbackInfo<v8::Value>& info) {
auto expr = info[0];
try {
ParsingContext ctx(expected);
ParseResult parsed = ctx.parse(mbgl::style::conversion::Convertible(expr));
ParsingContext ctx = expected ? ParsingContext(*expected) : ParsingContext();
ParseResult parsed = ctx.parseLayerPropertyExpression(mbgl::style::conversion::Convertible(expr));
if (parsed) {
assert(ctx.getErrors().size() == 0);
auto nodeExpr = new NodeExpression(std::move(*parsed));

7
src/mbgl/style/conversion/filter.cpp
View File

@@ -2,7 +2,7 @@
#include <mbgl/util/geometry.hpp>
#include <mbgl/style/expression/expression.hpp>
#include <mbgl/style/expression/type.hpp>
#include <mbgl/style/conversion/expression.hpp>
#include <mbgl/style/expression/parsing_context.hpp>
namespace mbgl {
namespace style {
@@ -236,10 +236,13 @@ optional<Filter> convertCompoundFilter(const Convertible& value, Error& error) {
}
optional<Filter> convertExpressionFilter(const Convertible& value, Error& error) {
optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, expression::type::Boolean);
expression::ParsingContext ctx(expression::type::Boolean);
expression::ParseResult expression = ctx.parseExpression(value);
if (!expression) {
error = { ctx.getCombinedErrors() };
return {};
}
return { ExpressionFilter { std::move(*expression) } };
}

27
src/mbgl/style/expression/parsing_context.cpp
View File

@@ -206,8 +206,16 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption
}
}
// if this is the root expression, enforce constraints on the use ["zoom"].
if (key.size() == 0 && !isZoomConstant(**parsed)) {
return parsed;
}
ParseResult ParsingContext::parseExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
return parse(value, typeAnnotationOption);
}
ParseResult ParsingContext::parseLayerPropertyExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
ParseResult parsed = parse(value, typeAnnotationOption);
if (parsed && !isZoomConstant(**parsed)) {
optional<variant<const InterpolateBase*, const Step*, ParsingError>> zoomCurve = findZoomCurve(parsed->get());
if (!zoomCurve) {
error(R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)");
@@ -217,10 +225,23 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption
return ParseResult();
}
}
return parsed;
}
const std::string ParsingContext::getCombinedErrors() const {
std::string combinedError;
for (const ParsingError& parsingError : *errors) {
if (combinedError.size() > 0) {
combinedError += "\n";
}
if (parsingError.key.size() > 0) {
combinedError += parsingError.key + ": ";
}
combinedError += parsingError.message;
}
return combinedError;
}
optional<std::string> ParsingContext::checkType(const type::Type& t) {
assert(expected);
optional<std::string> err = type::checkSubtype(*expected, t);

2
test/style/expression/expression.test.cpp
View File

@@ -49,7 +49,7 @@ TEST_P(ExpressionEqualityTest, ExpressionEquality) {
assert(!document.HasParseError());
const JSValue* expression = &document;
expression::ParsingContext ctx;
expression::ParseResult parsed = ctx.parse(conversion::Convertible(expression));
expression::ParseResult parsed = ctx.parseExpression(conversion::Convertible(expression));
if (!parsed) {
error_ = ctx.getErrors().size() > 0 ? ctx.getErrors()[0].message : "failed to parse";
};

14
test/style/filter.test.cpp
View File

@@ -11,14 +11,19 @@
using namespace mbgl;
using namespace mbgl::style;
bool filter(const char * json, const PropertyMap& featureProperties = {{}}, optional<FeatureIdentifier> featureId = {}, FeatureType featureType = FeatureType::Point, GeometryCollection featureGeometry = {}) {
bool filter(const char * json,
const PropertyMap& featureProperties = {{}},
optional<FeatureIdentifier> featureId = {},
FeatureType featureType = FeatureType::Point,
GeometryCollection featureGeometry = {},
float zoom = 0.0f) {
conversion::Error error;
optional<Filter> filter = conversion::convertJSON<Filter>(json, error);
EXPECT_TRUE(bool(filter));
EXPECT_EQ(error.message, "");
StubGeometryTileFeature feature { featureId, featureType, featureGeometry, featureProperties };
expression::EvaluationContext context = { &feature };
expression::EvaluationContext context = { zoom, &feature };
return (*filter)(context);
}
@@ -109,3 +114,8 @@ TEST(Filter, PropertyExpression) {
ASSERT_TRUE(filter("[\"==\", [\"get\", \"two\"], 2]", {{"two", int64_t(2)}}));
ASSERT_FALSE(filter("[\"==\", [\"get\", \"two\"], 4]", {{"two", int64_t(2)}}));
}
TEST(Filter, ZoomExpressionNested) {
ASSERT_TRUE(filter(R"(["==", ["get", "two"], ["zoom"]])", {{"two", int64_t(2)}}, {}, FeatureType::Point, {}, 2.0f));
ASSERT_FALSE(filter(R"(["==", ["get", "two"], ["+", ["zoom"], 1]])", {{"two", int64_t(2)}}, {}, FeatureType::Point, {}, 2.0f));
}