From 1e620d3bd890f7efdf76ce7bff4cf57d41570bb2 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 2 Apr 2019 15:55:21 +0300 Subject: [PATCH] [core] Fix legacy/{interval,exponential}/*-default expression tests Consider the function default value in interval and exponential functions conversion. --- platform/node/test/ignores.json | 8 --- src/mbgl/style/conversion/function.cpp | 83 +++++++++++++++++++------- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 4cdd7795e..ec79ed898 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -15,20 +15,12 @@ "expression-tests/legacy/categorical/string": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/categorical/string-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/categorical/tokens": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/exponential/array-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/exponential/color-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/exponential/color-hcl": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/exponential/color-lab": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/exponential/composite-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/exponential/number-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/identity/color-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/identity/enum-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/interval/array-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/interval/color-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/interval/composite": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/interval/composite-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/interval/number-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", - "expression-tests/legacy/interval/string-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "expression-tests/legacy/interval/tokens-zoom": "https://github.com/mapbox/mapbox-gl-native/issues/12747", "query-tests/geometry/multilinestring": "needs investigation", "query-tests/geometry/multipolygon": "needs investigation", diff --git a/src/mbgl/style/conversion/function.cpp b/src/mbgl/style/conversion/function.cpp index 33ed7c985..f00aa670b 100644 --- a/src/mbgl/style/conversion/function.cpp +++ b/src/mbgl/style/conversion/function.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -474,23 +475,43 @@ std::unique_ptr categorical(type::Type type, const std::string return std::make_unique(type, std::move(trueBranch), std::move(falseCase)); } +static std::unique_ptr numberOrDefault(type::Type type, + std::unique_ptr get, + std::unique_ptr expr, + optional> def) { + if (!def) { + return expr; + } + + std::vector branches; + branches.emplace_back(eq(compound("typeof", std::move(get)), literal("number")), + std::move(expr)); + return std::make_unique(type, std::move(branches), std::move(*def)); +} + static optional> convertIntervalFunction(type::Type type, const Convertible& value, Error& error, - std::unique_ptr input, + std::function (bool)> makeInput, + optional> def, bool convertTokens = false) { auto stops = convertStops(type, value, error, convertTokens); if (!stops) { return nullopt; } omitFirstStop(*stops); - return step(type, std::move(input), std::move(*stops)); + + return numberOrDefault(type, + makeInput(false), + step(type, makeInput(true), std::move(*stops)), + std::move(def)); } static optional> convertExponentialFunction(type::Type type, const Convertible& value, Error& error, - std::unique_ptr input, + std::function (bool)> makeInput, + optional> def, bool convertTokens = false) { auto stops = convertStops(type, value, error, convertTokens); if (!stops) { @@ -500,7 +521,11 @@ static optional> convertExponentialFunction(type::Ty if (!base) { return nullopt; } - return interpolate(type, exponential(*base), std::move(input), std::move(*stops)); + + return numberOrDefault(type, + makeInput(false), + interpolate(type, exponential(*base), makeInput(true), std::move(*stops)), + std::move(def)); } static optional> convertCategoricalFunction(type::Type type, @@ -672,18 +697,21 @@ optional> convertFunctionToExpression(type::Type typ } } - optional> defaultExpr; - if (objectMember(value, "default")) { - defaultExpr = convertLiteral(type, *objectMember(value, "default"), err); - } + auto defaultExpr = [&]() -> optional> { + auto member = objectMember(value, "default"); + if (member) { + return convertLiteral(type, *member, err); + } + return {}; + }; if (!objectMember(value, "property")) { // Camera function. switch (functionType) { case FunctionType::Interval: - return convertIntervalFunction(type, value, err, zoom(), convertTokens); + return convertIntervalFunction(type, value, err, [](bool) { return zoom(); }, defaultExpr(), convertTokens); case FunctionType::Exponential: - return convertExponentialFunction(type, value, err, zoom(), convertTokens); + return convertExponentialFunction(type, value, err, [](bool) { return zoom(); }, defaultExpr(), convertTokens); default: err.message = "unsupported function type"; return nullopt; @@ -705,22 +733,22 @@ optional> convertFunctionToExpression(type::Type typ if (functionType == FunctionType::Identity) { return type.match( [&] (const type::StringType&) -> optional> { - return string(get(literal(*property)), std::move(defaultExpr)); + return string(get(literal(*property)), defaultExpr()); }, [&] (const type::NumberType&) -> optional> { - return number(get(literal(*property)), std::move(defaultExpr)); + return number(get(literal(*property)), defaultExpr()); }, [&] (const type::BooleanType&) -> optional> { - return boolean(get(literal(*property)), std::move(defaultExpr)); + return boolean(get(literal(*property)), defaultExpr()); }, [&] (const type::ColorType&) -> optional> { - return toColor(get(literal(*property)), std::move(defaultExpr)); + return toColor(get(literal(*property)), defaultExpr()); }, [&] (const type::Array& array) -> optional> { - return assertion(array, get(literal(*property)), std::move(defaultExpr)); + return assertion(array, get(literal(*property)), defaultExpr()); }, [&] (const type::FormattedType&) -> optional> { - return toFormatted(get(literal(*property)), std::move(defaultExpr)); + return toFormatted(get(literal(*property)), defaultExpr()); }, [&] (const auto&) -> optional> { assert(false); // No properties use this type. @@ -759,13 +787,21 @@ optional> convertFunctionToExpression(type::Type typ const auto& stop = arrayMember(first, 0); + const auto getProperty = [&](bool coerce) { + if (coerce) { + return number(get(literal(*property))); + } else { + return get(literal(*property)); + } + }; + if (!isObject(stop)) { // Source function. switch (functionType) { case FunctionType::Interval: - return convertIntervalFunction(type, value, err, number(get(literal(*property)))); + return convertIntervalFunction(type, value, err, getProperty, defaultExpr()); case FunctionType::Exponential: - return convertExponentialFunction(type, value, err, number(get(literal(*property)))); + return convertExponentialFunction(type, value, err, getProperty, defaultExpr()); case FunctionType::Categorical: return convertCategoricalFunction(type, value, err, *property); default: @@ -796,11 +832,18 @@ optional> convertFunctionToExpression(type::Type typ switch (functionType) { case FunctionType::Interval: return composite(type, value, err, [&] (type::Type type_, double, std::map> stops) { - return step(type_, number(get(literal(*property))), std::move(stops)); + omitFirstStop(stops); + return numberOrDefault(type, + getProperty(false), + step(type_, getProperty(true), std::move(stops)), + defaultExpr()); }); case FunctionType::Exponential: return composite(type, value, err, [&] (type::Type type_, double base, std::map> stops) { - return interpolate(type_, exponential(base), number(get(literal(*property))), std::move(stops)); + return numberOrDefault(type, + getProperty(false), + interpolate(type_, exponential(base), getProperty(true), std::move(stops)), + defaultExpr()); }); case FunctionType::Categorical: return composite(type, value, err, [&] (type::Type type_, double, std::map> stops) {