diff --git a/src/mbgl/style/conversion/property_value.cpp b/src/mbgl/style/conversion/property_value.cpp index d86641f9576e..2a03aa7fb618 100644 --- a/src/mbgl/style/conversion/property_value.cpp +++ b/src/mbgl/style/conversion/property_value.cpp @@ -3,6 +3,10 @@ #include #include #include +#include + +#include +#include namespace mbgl { namespace style { @@ -22,11 +26,27 @@ std::optional> Converter>::operator()(const Co std::optional> expression; if (isExpression(value)) { - ParsingContext ctx(valueTypeToExpressionType()); - ParseResult parsed = ctx.parseLayerPropertyExpression(value); - if (!parsed) { - error.message = ctx.getCombinedErrors(); - return std::nullopt; + ParseResult parsed; + if constexpr (std::is_same_v> || std::is_same_v>) { + using Element = typename T::value_type; + const auto scalarType = valueTypeToExpressionType(); + ParsingContext scalarCtx(scalarType); + parsed = scalarCtx.parseLayerPropertyExpression(value); + if (!parsed) { + ParsingContext arrayCtx{type::Array(scalarType)}; + parsed = arrayCtx.parseLayerPropertyExpression(value); + if (!parsed) { + error.message = arrayCtx.getCombinedErrors(); + return std::nullopt; + } + } + } else { + ParsingContext ctx(valueTypeToExpressionType()); + parsed = ctx.parseLayerPropertyExpression(value); + if (!parsed) { + error.message = ctx.getCombinedErrors(); + return std::nullopt; + } } expression = PropertyExpression(std::move(*parsed)); } else if (isObject(value)) { diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp index afd691f2ad4b..0af8eb554d22 100644 --- a/src/mbgl/style/expression/value.cpp +++ b/src/mbgl/style/expression/value.cpp @@ -258,6 +258,25 @@ std::optional> ValueConverter>::fromExpressionValu [&](const auto&) { return std::optional>(); }); } +template <> +std::optional> ValueConverter>::fromExpressionValue(const Value& value) { + if (value.is>()) { + const auto& values = value.get>(); + std::vector result; + result.reserve(values.size()); + for (const auto& v : values) { + auto num = ValueConverter::fromExpressionValue(v); + if (!num) return std::nullopt; + result.push_back(*num); + } + return result; + } + + auto num = ValueConverter::fromExpressionValue(value); + if (!num) return std::nullopt; + return std::vector{*num}; +} + Value ValueConverter::toExpressionValue(const mbgl::style::Position& value) { return ValueConverter>::toExpressionValue(value.getSpherical()); } diff --git a/test/style/conversion/property_value.test.cpp b/test/style/conversion/property_value.test.cpp index f6281034b6a0..8996b42a3494 100644 --- a/test/style/conversion/property_value.test.cpp +++ b/test/style/conversion/property_value.test.cpp @@ -2,8 +2,12 @@ #include #include +#include +#include #include +#include + using namespace mbgl; using namespace mbgl::style; using namespace mbgl::style::conversion; @@ -20,3 +24,111 @@ TEST(StyleConversion, PropertyValue) { ASSERT_TRUE(result->isConstant()); ASSERT_EQ(result->asConstant(), expected); } + +// Regression: PR #3965 promoted hillshade-shadow-color and hillshade-highlight-color +// from PaintProperty to PaintProperty> to support +// multidirectional hillshade. The style-spec types them as `colorArray` with +// single-color defaults ("#000000" / "#FFFFFF"), and maplibre-gl-js accepts both +// `color` and `array` expression outputs for these properties. +// +// Before the fix in src/mbgl/style/conversion/property_value.cpp, parsing an +// `interpolate` expression over single-color outputs against a +// `PropertyValue>` failed with +// "Expected array but found string instead." — breaking nearly every +// published topographic basemap that animates hillshade color over zoom. + +TEST(StyleConversion, PropertyValueVectorColor_SingleColorInterpolate) { + Error error; + JSDocument doc; + doc.Parse<0>(R"(["interpolate", ["linear"], ["zoom"], 0, "#000000", 17, "#666666"])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << "single-color interpolate must parse for colorArray properties: " << error.message; + ASSERT_TRUE(result->isExpression()); +} + +TEST(StyleConversion, PropertyValueVectorColor_MultidirectionalConstant) { + // Spec-defined multidirectional shape: a JSON array of color strings used as + // a constant. Flows through the non-expression conversion path; must still + // parse correctly after the single-color expression fallback was added. + Error error; + JSDocument doc; + doc.Parse<0>(R"(["#000000", "#222222"])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << "multidirectional constant array must parse: " << error.message; + ASSERT_TRUE(result->isConstant()); + ASSERT_EQ(result->asConstant().size(), 2u); +} + +// PR #3965 also promoted hillshade-illumination-direction and +// hillshade-illumination-altitude from PaintProperty to +// PaintProperty>. The style-spec types them as `numberArray` +// with single-number defaults (335 and 45). The same compile-time check that +// broke colorArray expressions also rejects spec-valid single-number +// interpolate expressions for these properties. + +TEST(StyleConversion, PropertyValueVectorFloat_SingleNumberInterpolate) { + Error error; + JSDocument doc; + doc.Parse<0>(R"(["interpolate", ["linear"], ["zoom"], 0, 335, 17, 200])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << "single-number interpolate must parse for numberArray properties: " << error.message; + ASSERT_TRUE(result->isExpression()); +} + +TEST(StyleConversion, PropertyValueVectorFloat_MultidirectionalConstant) { + Error error; + JSDocument doc; + doc.Parse<0>(R"([335, 45])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << "multidirectional constant array must parse: " << error.message; + ASSERT_TRUE(result->isConstant()); + ASSERT_EQ(result->asConstant().size(), 2u); +} + +// Evaluation smoke tests: a parse-time success isn't useful if the runtime +// conversion path silently returns nullopt at every frame. These exercise the +// ValueConverter>::fromExpressionValue wrap for scalar outputs. + +TEST(StyleConversion, PropertyValueVectorColor_SingleColorInterpolate_Evaluates) { + Error error; + JSDocument doc; + doc.Parse<0>(R"(["interpolate", ["linear"], ["zoom"], 0, "#000000", 10, "#FFFFFF"])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << error.message; + ASSERT_TRUE(result->isExpression()); + + const auto& expr = result->asExpression(); + auto v0 = expr.evaluate(0.0f); + ASSERT_EQ(v0.size(), 1u); + EXPECT_FLOAT_EQ(v0[0].r, 0.0f); + EXPECT_FLOAT_EQ(v0[0].g, 0.0f); + EXPECT_FLOAT_EQ(v0[0].b, 0.0f); + + auto v10 = expr.evaluate(10.0f); + ASSERT_EQ(v10.size(), 1u); + EXPECT_FLOAT_EQ(v10[0].r, 1.0f); + EXPECT_FLOAT_EQ(v10[0].g, 1.0f); + EXPECT_FLOAT_EQ(v10[0].b, 1.0f); +} + +TEST(StyleConversion, PropertyValueVectorFloat_SingleNumberInterpolate_Evaluates) { + Error error; + JSDocument doc; + doc.Parse<0>(R"(["interpolate", ["linear"], ["zoom"], 0, 335, 10, 200])"); + auto result = convert>>(doc, error, false, false); + ASSERT_TRUE(result) << error.message; + ASSERT_TRUE(result->isExpression()); + + const auto& expr = result->asExpression(); + auto v0 = expr.evaluate(0.0f); + ASSERT_EQ(v0.size(), 1u); + EXPECT_FLOAT_EQ(v0[0], 335.0f); + + auto v5 = expr.evaluate(5.0f); + ASSERT_EQ(v5.size(), 1u); + EXPECT_FLOAT_EQ(v5[0], 267.5f); + + auto v10 = expr.evaluate(10.0f); + ASSERT_EQ(v10.size(), 1u); + EXPECT_FLOAT_EQ(v10[0], 200.0f); +}