diff --git a/CMakeLists.txt b/CMakeLists.txt index f65d7691525..c140af1e094 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -413,6 +413,7 @@ list(APPEND INCLUDE_FILES ${PROJECT_SOURCE_DIR}/include/mbgl/util/logging.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/lru_cache.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/noncopyable.hpp + ${PROJECT_SOURCE_DIR}/include/mbgl/util/padding.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/platform.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/premultiply.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/projection.hpp @@ -972,6 +973,7 @@ list(APPEND SRC_FILES ${PROJECT_SOURCE_DIR}/src/mbgl/util/mat4.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/util/mat4.hpp ${PROJECT_SOURCE_DIR}/src/mbgl/util/math.hpp + ${PROJECT_SOURCE_DIR}/src/mbgl/util/padding.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/util/premultiply.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/util/quaternion.cpp ${PROJECT_SOURCE_DIR}/src/mbgl/util/rapidjson.cpp diff --git a/bazel/core.bzl b/bazel/core.bzl index d168dd7e311..16e5281489e 100644 --- a/bazel/core.bzl +++ b/bazel/core.bzl @@ -613,6 +613,7 @@ MLN_CORE_SOURCE = [ "src/mbgl/util/mat4.cpp", "src/mbgl/util/mat4.hpp", "src/mbgl/util/math.hpp", + "src/mbgl/util/padding.cpp", "src/mbgl/util/premultiply.cpp", "src/mbgl/util/quaternion.cpp", "src/mbgl/util/quaternion.hpp", @@ -835,6 +836,7 @@ MLN_CORE_HEADERS = [ "include/mbgl/util/lru_cache.hpp", "include/mbgl/util/monotonic_timer.hpp", "include/mbgl/util/noncopyable.hpp", + "include/mbgl/util/padding.hpp", "include/mbgl/util/platform.hpp", "include/mbgl/util/premultiply.hpp", "include/mbgl/util/projection.hpp", diff --git a/include/mbgl/style/conversion/constant.hpp b/include/mbgl/style/conversion/constant.hpp index ae0a40a0bf5..58f6d5cef00 100644 --- a/include/mbgl/style/conversion/constant.hpp +++ b/include/mbgl/style/conversion/constant.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -44,6 +45,11 @@ struct Converter { std::optional operator()(const Convertible& value, Error& error) const; }; +template <> +struct Converter { + std::optional operator()(const Convertible& value, Error& error) const; +}; + template struct Converter> { std::optional> operator()(const Convertible& value, Error& error) const; diff --git a/include/mbgl/style/conversion_impl.hpp b/include/mbgl/style/conversion_impl.hpp index a45690d440d..7e7ccbfd7f9 100644 --- a/include/mbgl/style/conversion_impl.hpp +++ b/include/mbgl/style/conversion_impl.hpp @@ -315,6 +315,11 @@ struct ValueFactory { static Value make(const Color& color) { return color.serialize(); } }; +template <> +struct ValueFactory { + static Value make(const Padding& padding) { return padding.serialize(); } +}; + template struct ValueFactory && !is_linear_container::value)>> { static Value make(const T& arg) { return {arg}; } @@ -361,12 +366,14 @@ Value makeValue(T&& arg) { template StyleProperty makeStyleProperty(const PropertyValue& value) { - return value.match([](const Undefined&) -> StyleProperty { return {}; }, - [](const Color& c) -> StyleProperty { return {makeValue(c), StyleProperty::Kind::Expression}; }, - [](const PropertyExpression& fn) -> StyleProperty { - return {fn.getExpression().serialize(), StyleProperty::Kind::Expression}; - }, - [](const auto& t) -> StyleProperty { return {makeValue(t), StyleProperty::Kind::Constant}; }); + return value.match( + [](const Undefined&) -> StyleProperty { return {}; }, + [](const Color& c) -> StyleProperty { return {makeValue(c), StyleProperty::Kind::Expression}; }, + [](const Padding& p) -> StyleProperty { return {makeValue(p), StyleProperty::Kind::Expression}; }, + [](const PropertyExpression& fn) -> StyleProperty { + return {fn.getExpression().serialize(), StyleProperty::Kind::Expression}; + }, + [](const auto& t) -> StyleProperty { return {makeValue(t), StyleProperty::Kind::Constant}; }); } inline StyleProperty makeStyleProperty(const TransitionOptions& value) { diff --git a/include/mbgl/style/expression/dsl.hpp b/include/mbgl/style/expression/dsl.hpp index 9b6a157eb7d..cbd6996724a 100644 --- a/include/mbgl/style/expression/dsl.hpp +++ b/include/mbgl/style/expression/dsl.hpp @@ -42,6 +42,7 @@ std::unique_ptr string(std::unique_ptr, std::unique_ptr< std::unique_ptr boolean(std::unique_ptr, std::unique_ptr def = nullptr); std::unique_ptr toColor(std::unique_ptr, std::unique_ptr def = nullptr); +std::unique_ptr toPadding(std::unique_ptr value, std::unique_ptr def = nullptr); std::unique_ptr toString(std::unique_ptr, std::unique_ptr def = nullptr); std::unique_ptr toFormatted(std::unique_ptr, std::unique_ptr def = nullptr); std::unique_ptr toImage(std::unique_ptr, std::unique_ptr def = nullptr); diff --git a/include/mbgl/style/expression/type.hpp b/include/mbgl/style/expression/type.hpp index 14f18e225bc..4d26dac3ba7 100644 --- a/include/mbgl/style/expression/type.hpp +++ b/include/mbgl/style/expression/type.hpp @@ -44,6 +44,12 @@ struct ColorType { bool operator==(const ColorType&) const { return true; } }; +struct PaddingType { + constexpr PaddingType() = default; + std::string getName() const { return "padding"; } + bool operator==(const PaddingType&) const { return true; } +}; + struct ObjectType { constexpr ObjectType() = default; std::string getName() const { return "object"; } @@ -85,6 +91,7 @@ constexpr NumberType Number; constexpr StringType String; constexpr BooleanType Boolean; constexpr ColorType Color; +constexpr PaddingType Padding; constexpr ValueType Value; constexpr ObjectType Object; constexpr CollatorType Collator; @@ -99,6 +106,7 @@ using Type = variant, diff --git a/include/mbgl/style/expression/value.hpp b/include/mbgl/style/expression/value.hpp index 6bcc49f613b..d4f72d7af4e 100644 --- a/include/mbgl/style/expression/value.hpp +++ b/include/mbgl/style/expression/value.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -30,6 +31,7 @@ using ValueBase = variant>, mapbox::util::recursive_wrapper>>; struct Value : ValueBase { diff --git a/include/mbgl/style/layers/symbol_layer.hpp b/include/mbgl/style/layers/symbol_layer.hpp index da9da58123b..31fb64da984 100644 --- a/include/mbgl/style/layers/symbol_layer.hpp +++ b/include/mbgl/style/layers/symbol_layer.hpp @@ -52,9 +52,9 @@ class SymbolLayer final : public Layer { const PropertyValue& getIconOptional() const; void setIconOptional(const PropertyValue&); - static PropertyValue getDefaultIconPadding(); - const PropertyValue& getIconPadding() const; - void setIconPadding(const PropertyValue&); + static PropertyValue getDefaultIconPadding(); + const PropertyValue& getIconPadding() const; + void setIconPadding(const PropertyValue&); static PropertyValue getDefaultIconPitchAlignment(); const PropertyValue& getIconPitchAlignment() const; diff --git a/include/mbgl/util/interpolate.hpp b/include/mbgl/util/interpolate.hpp index 13b6ef3f482..8199bd944d6 100644 --- a/include/mbgl/util/interpolate.hpp +++ b/include/mbgl/util/interpolate.hpp @@ -125,6 +125,24 @@ struct Interpolator { } }; +template <> +struct Interpolator { +public: + Padding operator()(const Padding& a, const Padding& b, const float t) const noexcept { + return {interpolate(a.top, b.top, t), + interpolate(a.right, b.right, t), + interpolate(a.bottom, b.bottom, t), + interpolate(a.left, b.left, t)}; + } + + Padding operator()(const Padding& a, const Padding& b, const double t) const noexcept { + return {interpolate(a.top, b.top, t), + interpolate(a.right, b.right, t), + interpolate(a.bottom, b.bottom, t), + interpolate(a.left, b.left, t)}; + } +}; + template <> struct Interpolator { public: diff --git a/include/mbgl/util/padding.hpp b/include/mbgl/util/padding.hpp new file mode 100644 index 00000000000..79663542a95 --- /dev/null +++ b/include/mbgl/util/padding.hpp @@ -0,0 +1,72 @@ +#pragma once + +#include +#include +#include + +namespace mbgl { + +// A set of four numbers representing padding around a box. +struct Padding { + // Empty/zero padding on all sides. + Padding() = default; + + // Same padding on all sides. + Padding(float value) + : top(value), + right(value), + bottom(value), + left(value) {} + + Padding(float t_, float r_, float b_, float l_) + : top(t_), + right(r_), + bottom(b_), + left(l_) {} + + // Padding values are in CSS order: top, right, bottom, left. + Padding(const std::span& values) { + switch (values.size()) { + case 1: + top = right = bottom = left = values[0]; + break; + case 2: + top = bottom = values[0]; + right = left = values[1]; + break; + case 3: + top = values[0]; + left = right = values[1]; + bottom = values[2]; + break; + case 4: + top = values[0]; + right = values[1]; + bottom = values[2]; + left = values[3]; + break; + default: + throw std::invalid_argument("Padding must have between 1 and 4 elements"); + } + } + + float top = 0; + float right = 0; + float bottom = 0; + float left = 0; + + explicit operator bool() const { return top != 0 || right != 0 || bottom != 0 || left != 0; } + + bool operator==(const Padding&) const = default; + + std::array toArray() const; + + // Used by ValueFactory::make() + mbgl::Value serialize() const; +}; + +inline Padding operator*(const Padding& padding, float scale) { + return {padding.top * scale, padding.right * scale, padding.bottom * scale, padding.left * scale}; +} + +} // namespace mbgl diff --git a/metrics/integration/render-tests/icon-padding/databind/expected-mobile.png b/metrics/integration/render-tests/icon-padding/databind/expected-mobile.png new file mode 100644 index 00000000000..adff4cfd770 Binary files /dev/null and b/metrics/integration/render-tests/icon-padding/databind/expected-mobile.png differ diff --git a/metrics/integration/render-tests/icon-padding/databind/expected.png b/metrics/integration/render-tests/icon-padding/databind/expected.png new file mode 100644 index 00000000000..fdf164604b1 Binary files /dev/null and b/metrics/integration/render-tests/icon-padding/databind/expected.png differ diff --git a/metrics/integration/render-tests/icon-padding/databind/style.json b/metrics/integration/render-tests/icon-padding/databind/style.json new file mode 100644 index 00000000000..5dee29b2fb8 --- /dev/null +++ b/metrics/integration/render-tests/icon-padding/databind/style.json @@ -0,0 +1,57 @@ +{ + "version": 8, + "metadata": { + "test": { + "collisionDebug": true, + "width": 240, + "height": 100 + } + }, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [{ + "type": "Feature", + "properties": { }, + "geometry": { "type": "Point", "coordinates": [-60, 0] } + }, { + "type": "Feature", + "properties": { "padding": 5 }, + "geometry": { "type": "Point", "coordinates": [-30, 0] } + }, { + "type": "Feature", + "properties": { "padding": [-20, -5] }, + "geometry": { "type": "Point", "coordinates": [-0, 0] } + }, { + "type": "Feature", + "properties": { "padding": [2, 2, -10] }, + "geometry": { "type": "Point", "coordinates": [30, 0] } + }, { + "type": "Feature", + "properties": { "padding": [2, 10, -20, 2] }, + "geometry": { "type": "Point", "coordinates": [60, 0] } + } + ] + } + } + }, + "sprite": "local://sprites/2x", + "layers": [{ + "id": "bg", + "type": "background", + "paint": { + "background-color": "#ccc" + } + }, { + "id": "symbolA", + "type": "symbol", + "source": "geojson", + "layout": { + "icon-image": "icon", + "icon-padding": ["coalesce", ["get", "padding"], ["literal", [2]]] + } + } + ] +} diff --git a/metrics/integration/render-tests/icon-padding/default/expected-mobile.png b/metrics/integration/render-tests/icon-padding/default/expected-mobile.png new file mode 100644 index 00000000000..fa895f75c51 Binary files /dev/null and b/metrics/integration/render-tests/icon-padding/default/expected-mobile.png differ diff --git a/metrics/integration/render-tests/icon-padding/default/expected.png b/metrics/integration/render-tests/icon-padding/default/expected.png new file mode 100644 index 00000000000..99cf94f7a50 Binary files /dev/null and b/metrics/integration/render-tests/icon-padding/default/expected.png differ diff --git a/metrics/integration/render-tests/icon-padding/default/style.json b/metrics/integration/render-tests/icon-padding/default/style.json new file mode 100644 index 00000000000..78c2fec6f42 --- /dev/null +++ b/metrics/integration/render-tests/icon-padding/default/style.json @@ -0,0 +1,58 @@ +{ + "version": 8, + "metadata": { + "test": { + "collisionDebug": true, + "width": 240, + "height": 100 + } + }, + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [{ + "type": "Feature", + "properties": { }, + "geometry": { "type": "Point", "coordinates": [-60, 0] } + }, { + "type": "Feature", + "properties": { }, + "geometry": { "type": "Point", "coordinates": [-40, 0] } + }, { + "type": "Feature", + "properties": { }, + "geometry": { "type": "Point", "coordinates": [-20, 0] } + } + ] + } + } + }, + "sprite": "local://sprites/2x", + "layers": [{ + "id": "bg", + "type": "background", + "paint": { + "background-color": "#ccc" + } + }, { + "id": "symbolA", + "type": "symbol", + "source": "geojson", + "layout": { + "icon-image": "icon", + "icon-padding": [10] + } + }, { + "id": "symbolB", + "type": "symbol", + "source": "geojson", + "layout": { + "icon-image": "icon", + "icon-padding": [2, 2, -20], + "icon-offset": [120, 0] + } + } + ] +} diff --git a/metrics/linux-gcc8-release/render-tests/icon-padding/databind/metrics.json b/metrics/linux-gcc8-release/render-tests/icon-padding/databind/metrics.json new file mode 100644 index 00000000000..bc3a6a1b148 --- /dev/null +++ b/metrics/linux-gcc8-release/render-tests/icon-padding/databind/metrics.json @@ -0,0 +1,14 @@ +{ + "network": [ + [ + "probeNetwork - default - end", + 2, + 2094 + ], + [ + "probeNetwork - default - start", + 0, + 0 + ] + ] +} \ No newline at end of file diff --git a/metrics/linux-gcc8-release/render-tests/icon-padding/default/metrics.json b/metrics/linux-gcc8-release/render-tests/icon-padding/default/metrics.json new file mode 100644 index 00000000000..bc3a6a1b148 --- /dev/null +++ b/metrics/linux-gcc8-release/render-tests/icon-padding/default/metrics.json @@ -0,0 +1,14 @@ +{ + "network": [ + [ + "probeNetwork - default - end", + 2, + 2094 + ], + [ + "probeNetwork - default - start", + 0, + 0 + ] + ] +} \ No newline at end of file diff --git a/platform/android/MapLibreAndroid/src/cpp/conversion/constant.cpp b/platform/android/MapLibreAndroid/src/cpp/conversion/constant.cpp index 347b55b545b..915bc6dc753 100644 --- a/platform/android/MapLibreAndroid/src/cpp/conversion/constant.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/conversion/constant.cpp @@ -2,6 +2,7 @@ #include "collection.hpp" #include "../style/formatted.hpp" +#include #include namespace mbgl { @@ -33,6 +34,16 @@ Result>> Converter>, Color>::o return jni::Make(env, value.stringify()); } +Result>> Converter>, Padding>::operator()( + jni::JNIEnv& env, const Padding& value) const { + const auto values = value.toArray(); + auto result = jni::Array::New(env, values.size()); + for (size_t i = 0; i < values.size(); i++) { + result.Set(env, i, jni::Box(env, values[i])); + } + return result; +} + Result>> Converter>, style::expression::Formatted>::operator()( jni::JNIEnv& env, const style::expression::Formatted& value) const { return Formatted::New(env, value); diff --git a/platform/android/MapLibreAndroid/src/cpp/conversion/constant.hpp b/platform/android/MapLibreAndroid/src/cpp/conversion/constant.hpp index c8ff5228ba4..9a4e05e8e56 100644 --- a/platform/android/MapLibreAndroid/src/cpp/conversion/constant.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/conversion/constant.hpp @@ -3,6 +3,7 @@ #include "conversion.hpp" #include +#include #include #include @@ -57,6 +58,11 @@ struct Converter>, Color> { Result>> operator()(jni::JNIEnv& env, const Color& value) const; }; +template <> +struct Converter>, Padding> { + Result>> operator()(jni::JNIEnv& env, const Padding& value) const; +}; + template <> struct Converter>, style::expression::Formatted> { Result>> operator()(jni::JNIEnv& env, const style::expression::Formatted& value) const; diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/expressions/Expression.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/expressions/Expression.java index b103d2ff6af..dafd5b4f7b5 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/expressions/Expression.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/expressions/Expression.java @@ -3688,6 +3688,20 @@ public static Expression toColor(@NonNull Expression input) { return new Expression("to-color", input); } + /** + * Converts input value to a padding. + * + * If the input is a number or an array of numbers padding is created following + * the same pattern as CSS padding. See Style specification. + * Otherwise, the expression is an error. + * + * @param input expression input + * @return expression + */ + public static Expression toPadding(@NonNull Expression input) { + return new Expression("to-padding", input); + } + /** * Binds input to named variables, * which can then be referenced in the result expression using {@link #var(String)} or {@link #var(Expression)}. diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/PropertyFactory.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/PropertyFactory.java index 6e86a0c5faf..2fd97bff35d 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/PropertyFactory.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/PropertyFactory.java @@ -2050,18 +2050,18 @@ public static PropertyValue iconRotate(Expression value) { /** * Size of additional area round the icon bounding box used for detecting symbol collisions. * - * @param value a Float value - * @return property wrapper around Float + * @param value a Float[] value + * @return property wrapper around Float[] */ - public static PropertyValue iconPadding(Float value) { + public static PropertyValue iconPadding(Float[] value) { return new LayoutPropertyValue<>("icon-padding", value); } /** * Size of additional area round the icon bounding box used for detecting symbol collisions. * - * @param value a Float value - * @return property wrapper around Float + * @param value a Float[] value + * @return property wrapper around Float[] */ public static PropertyValue iconPadding(Expression value) { return new LayoutPropertyValue<>("icon-padding", value); diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/SymbolLayer.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/SymbolLayer.java index bdae6da13ba..adc8148e5cf 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/SymbolLayer.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/layers/SymbolLayer.java @@ -316,13 +316,13 @@ public PropertyValue getIconRotate() { /** * Get the IconPadding property * - * @return property wrapper value around Float + * @return property wrapper value around Float[] */ @NonNull @SuppressWarnings("unchecked") - public PropertyValue getIconPadding() { + public PropertyValue getIconPadding() { checkThread(); - return (PropertyValue) new PropertyValue("icon-padding", nativeGetIconPadding()); + return (PropertyValue) new PropertyValue("icon-padding", nativeGetIconPadding()); } /** diff --git a/platform/android/MapLibreAndroid/src/test/java/org/maplibre/android/style/expressions/ExpressionTest.kt b/platform/android/MapLibreAndroid/src/test/java/org/maplibre/android/style/expressions/ExpressionTest.kt index b3a8c24a07b..f4cadc19e3d 100644 --- a/platform/android/MapLibreAndroid/src/test/java/org/maplibre/android/style/expressions/ExpressionTest.kt +++ b/platform/android/MapLibreAndroid/src/test/java/org/maplibre/android/style/expressions/ExpressionTest.kt @@ -1124,6 +1124,14 @@ class ExpressionTest { Assert.assertTrue("expression should match", Arrays.deepEquals(expected, actual)) } + @Test + @Throws(Exception::class) + fun testToPadding() { + val expected = arrayOf("to-padding", "value") + val actual = Expression.toPadding(Expression.literal("value")).toArray() + Assert.assertTrue("expression should match", Arrays.deepEquals(expected, actual)) + } + @Test @Throws(Exception::class) fun testLet() { diff --git a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/ExpressionTest.java b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/ExpressionTest.java index 2fec96e9d4d..fe00cb99047 100644 --- a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/ExpressionTest.java +++ b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/ExpressionTest.java @@ -55,11 +55,13 @@ import static org.maplibre.android.style.expressions.Expression.string; import static org.maplibre.android.style.expressions.Expression.switchCase; import static org.maplibre.android.style.expressions.Expression.toColor; +import static org.maplibre.android.style.expressions.Expression.toPadding; import static org.maplibre.android.style.expressions.Expression.zoom; import static org.maplibre.android.style.layers.PropertyFactory.circleColor; import static org.maplibre.android.style.layers.PropertyFactory.fillAntialias; import static org.maplibre.android.style.layers.PropertyFactory.fillColor; import static org.maplibre.android.style.layers.PropertyFactory.fillOutlineColor; +import static org.maplibre.android.style.layers.PropertyFactory.iconPadding; import static org.maplibre.android.style.layers.PropertyFactory.textColor; import static org.maplibre.android.style.layers.PropertyFactory.textField; import static org.maplibre.android.testapp.action.MapLibreMapAction.invoke; @@ -779,6 +781,93 @@ public void testDoubleConversion() { }); } + @Test + public void testToPaddingExpression() { + validateTestSetup(); + invoke(maplibreMap, (uiController, maplibreMap) -> { + LatLng latLng = new LatLng(51, 17); + maplibreMap.getStyle().addSource( + new GeoJsonSource("source", Point.fromLngLat(latLng.getLongitude(), latLng.getLatitude())) + ); + + SymbolLayer layer = new SymbolLayer("layer", "source"); + maplibreMap.getStyle().addLayer(layer); + + // Automatic usage with iconPadding property, analogous to testGetExpressionWrapping() + { + Expression input = get("value"); + layer.setProperties(iconPadding(input)); + + Expression expectedOuput = toPadding(input); + Expression output = layer.getIconPadding().getExpression(); + assertNotNull(output); + assertArrayEquals("Expression should match", expectedOuput.toArray(), output.toArray()); + } + + // Same within interpolate expression + { + Expression input = interpolate( + exponential(0.5f), zoom(), + stop(-0.1, get("value")), + stop(0, get("value")) + ); + layer.setProperties(iconPadding(input)); + + Expression expectedOutput = interpolate( + exponential(0.5f), zoom(), + stop(-0.1, toPadding(get("value"))), + stop(0, toPadding(get("value"))) + ); + Expression output = layer.getIconPadding().getExpression(); + assertNotNull(output); + assertArrayEquals("Expression should match", expectedOutput.toArray(), output.toArray()); + } + }); + } + + @Test + public void testToPaddingResult() { + validateTestSetup(); + invoke(maplibreMap, (uiController, maplibreMap) -> { + LatLng latLng = new LatLng(51, 17); + maplibreMap.getStyle().addSource( + new GeoJsonSource("source", Point.fromLngLat(latLng.getLongitude(), latLng.getLatitude())) + ); + + SymbolLayer layer = new SymbolLayer("layer", "source"); + maplibreMap.getStyle().addLayer(layer); + + Expression input = Expression.toPadding(Expression.literal(new Float[] { 7.5f, 10.0f, 1.0f})); + layer.setProperties(iconPadding(input)); + + assertNull("Expression should be null", layer.getIconPadding().getExpression()); + assertArrayEquals( + "Padding value should match", + new Float[] { 7.5f, 10.0f, 1.0f, 10.0f }, + layer.getIconPadding().getValue()); + }); + } + + @Test + public void testToPaddingError() { + validateTestSetup(); + invoke(maplibreMap, (uiController, maplibreMap) -> { + LatLng latLng = new LatLng(51, 17); + maplibreMap.getStyle().addSource( + new GeoJsonSource("source", Point.fromLngLat(latLng.getLongitude(), latLng.getLatitude())) + ); + + SymbolLayer layer = new SymbolLayer("layer", "source"); + maplibreMap.getStyle().addLayer(layer); + + Expression input = toPadding(literal("invalid")); + layer.setProperties(iconPadding(input)); + + assertNull("Expression should be null", layer.getIconPadding().getExpression()); + assertNull("Padding value should be null", layer.getIconPadding().getValue()); + }); + } + private void setupStyle() { invoke(maplibreMap, (uiController, maplibreMap) -> { // Add a source diff --git a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/SymbolLayerTest.java b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/SymbolLayerTest.java index acdc16f9392..7987d674591 100644 --- a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/SymbolLayerTest.java +++ b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/SymbolLayerTest.java @@ -422,11 +422,24 @@ public void testIconPaddingAsConstant() { assertNull(layer.getIconPadding().getValue()); // Set and Get - Float propertyValue = 0.3f; + Float[] propertyValue = {2.0f, 2.0f, 2.0f, 2.0f}; layer.setProperties(iconPadding(propertyValue)); assertEquals(layer.getIconPadding().getValue(), propertyValue); } + @Test + @UiThreadTest + public void testIconPaddingAsExpression() { + Timber.i("icon-padding-expression"); + assertNotNull(layer); + assertNull(layer.getIconPadding().getExpression()); + + // Set and Get + Expression expression = toPadding(Expression.get("undefined")); + layer.setProperties(iconPadding(expression)); + assertEquals(layer.getIconPadding().getExpression(), expression); + } + @Test @UiThreadTest public void testIconKeepUprightAsConstant() { diff --git a/platform/android/scripts/generate-style-code.mjs b/platform/android/scripts/generate-style-code.mjs index 76965774af2..4afd01a804f 100644 --- a/platform/android/scripts/generate-style-code.mjs +++ b/platform/android/scripts/generate-style-code.mjs @@ -119,6 +119,8 @@ global.propertyType = function propertyType(property) { return 'String'; case 'color': return 'String'; + case 'padding': + return 'Float[]' case 'array': return `${propertyType({type:property.value, name: property.name})}[]`; default: @@ -193,6 +195,8 @@ global.propertyNativeType = function (property) { return `${camelize(property.name)}Type`; case 'color': return `Color`; + case 'padding': + return 'Padding'; case 'array': if (property.length) { return `std::array<${propertyType({type: property.value})}, ${property.length}>`; @@ -229,6 +233,8 @@ global.defaultExpressionJava = function(property) { return "string"; case 'color': return 'toColor'; + case 'padding': + return 'toPadding'; case 'array': return "array"; default: return "string"; @@ -261,6 +267,8 @@ global.defaultValueJava = function(property) { return snakeCaseUpper(property.name) + "_" + snakeCaseUpper(Object.keys(property.values)[0]); case 'color': return '"rgba(255,128,0,0.7)"'; + case 'padding': + return '{2.0f, 2.0f, 2.0f, 2.0f}'; case 'array': switch (property.value) { case 'string': diff --git a/platform/darwin/scripts/generate-style-code.mjs b/platform/darwin/scripts/generate-style-code.mjs index dec8cd4fcb9..dc1098448e5 100644 --- a/platform/darwin/scripts/generate-style-code.mjs +++ b/platform/darwin/scripts/generate-style-code.mjs @@ -175,6 +175,16 @@ global.testImplementation = function (property, layerType, isFunction) { global.objCTestValue = function (property, layerType, arraysAsStructs, indent) { let propertyName = originalPropertyName(property); + + const paddingTestValue = () => { + if (arraysAsStructs) { + let iosValue = '[NSValue valueWithUIEdgeInsets:UIEdgeInsetsMake(1, 1, 1, 1)]'.indent(indent * 4); + let macosValue = '[NSValue valueWithEdgeInsets:NSEdgeInsetsMake(1, 1, 1, 1)]'.indent(indent * 4); + return `@"%@",\n#if TARGET_OS_IPHONE\n${iosValue}\n#else\n${macosValue}\n#endif\n${''.indent((indent - 1) * 4)}`; + } + return '@"{1, 1, 1, 1}"'; + } + switch (property.type) { case 'boolean': return property.default ? '@"false"' : '@"true"'; @@ -196,6 +206,8 @@ global.objCTestValue = function (property, layerType, arraysAsStructs, indent) { return `@"'${_.last(_.keys(property.values))}'"`; case 'color': return '@"%@", [MLNColor redColor]'; + case 'padding': + return paddingTestValue(); case 'array': switch (arrayType(property)) { case 'dasharray': @@ -203,12 +215,7 @@ global.objCTestValue = function (property, layerType, arraysAsStructs, indent) { case 'font': return `@"{'${_.startCase(propertyName)}', '${_.startCase(_.reverse(propertyName.split('')).join(''))}'}"`; case 'padding': { - if (arraysAsStructs) { - let iosValue = '[NSValue valueWithUIEdgeInsets:UIEdgeInsetsMake(1, 1, 1, 1)]'.indent(indent * 4); - let macosValue = '[NSValue valueWithEdgeInsets:NSEdgeInsetsMake(1, 1, 1, 1)]'.indent(indent * 4); - return `@"%@",\n#if TARGET_OS_IPHONE\n${iosValue}\n#else\n${macosValue}\n#endif\n${''.indent((indent - 1) * 4)}`; - } - return '@"{1, 1, 1, 1}"'; + return paddingTestValue(); } case 'offset': case 'translate': { @@ -261,6 +268,8 @@ global.mbglTestValue = function (property, layerType) { } case 'color': return '{ 1, 0, 0, 1 }'; + case 'padding': + return '{ 1, 1, 1, 1 }'; case 'array': switch (arrayType(property)) { case 'dasharray': @@ -335,6 +344,8 @@ global.testHelperMessage = function (property, layerType, isFunction) { return `testEnum${fnSuffix}:${objCEnum} type:@encode(${objCType})`; case 'color': return 'testColor' + fnSuffix; + case 'padding': + return 'testPaddingType' + fnSuffix; case 'array': switch (arrayType(property)) { case 'dasharray': @@ -517,6 +528,8 @@ global.describeType = function (property) { return '`MLN' + camelize(property.name) + '`'; case 'color': return '`UIColor`'; + case 'padding': + return '`UIEdgeInsets`'; case 'array': switch (arrayType(property)) { case 'padding': @@ -540,7 +553,7 @@ global.describeType = function (property) { } global.describeValue = function (value, property, layerType) { - if (Array.isArray(value) && property.type !== 'array' && property.type !== 'enum') { + if (Array.isArray(value) && property.type !== 'array' && property.type !== 'enum' && property.type !== 'padding') { switch (value[0]) { case 'interpolate': { let curveType = value[1][0]; @@ -553,6 +566,21 @@ global.describeValue = function (value, property, layerType) { } } + const describePadding = () => { + let units = property.units || ''; + if (units) { + units = ` ${units}`.replace(/pixel/, 'point'); + } + + if (value.every(num => num === 0)) { + return 'an `NSValue` object containing `UIEdgeInsetsZero`'; + } + if (value.length === 1) { + return 'an `NSValue` object containing a `UIEdgeInsets` struct set to' + ` ${formatNumber(value[0])}${units} on all sides`; + } + return 'an `NSValue` object containing a `UIEdgeInsets` struct set to' + ` ${formatNumber(value[0])}${units} on the top, ${formatNumber(value[3])}${units} on the left, ${formatNumber(value[2])}${units} on the bottom, and ${formatNumber(value[1])}${units} on the right`; + } + switch (property.type) { case 'boolean': return value ? '`YES`' : '`NO`'; @@ -595,6 +623,10 @@ global.describeValue = function (value, property, layerType) { return '`UIColor.whiteColor`'; } return 'a `UIColor`' + ` object whose RGB value is ${formatNumber(color.r)}, ${formatNumber(color.g)}, ${formatNumber(color.b)} and whose alpha value is ${formatNumber(color.a)}`; + + case 'padding': + return describePadding(); + case 'array': let units = property.units || ''; if (units) { @@ -602,10 +634,7 @@ global.describeValue = function (value, property, layerType) { } switch (arrayType(property)) { case 'padding': - if (value[0] === 0 && value[1] === 0 && value[2] === 0 && value[3] === 0) { - return 'an `NSValue` object containing `UIEdgeInsetsZero`'; - } - return 'an `NSValue` object containing a `UIEdgeInsets` struct set to' + ` ${formatNumber(value[0])}${units} on the top, ${formatNumber(value[3])}${units} on the left, ${formatNumber(value[2])}${units} on the bottom, and ${formatNumber(value[1])}${units} on the right`; + return describePadding(); case 'offset': case 'translate': return 'an `NSValue` object containing a `CGVector` struct set to' + ` ${formatNumber(value[0])}${units} rightward and ${formatNumber(value[1])}${units} downward`; @@ -653,6 +682,8 @@ global.propertyType = function (property) { return 'NSValue *'; case 'color': return 'MLNColor *'; + case 'padding': + return 'NSValue *'; case 'array': switch (arrayType(property)) { case 'dasharray': @@ -701,6 +732,8 @@ global.valueTransformerArguments = function (property) { return [mbglType(property), 'NSValue *', mbglType(property), `MLN${camelize(property.name)}`]; case 'color': return ['mbgl::Color', objCType]; + case 'padding': + return ['mbgl::Padding', objCType]; case 'array': switch (arrayType(property)) { case 'dasharray': @@ -756,6 +789,8 @@ global.mbglType = function(property) { } case 'color': return 'mbgl::Color'; + case 'padding': + return 'mbgl::Padding'; case 'array': switch (arrayType(property)) { case 'dasharray': diff --git a/platform/darwin/src/MLNStyleValue_Private.h b/platform/darwin/src/MLNStyleValue_Private.h index e20ccb4fdac..50f2a846411 100644 --- a/platform/darwin/src/MLNStyleValue_Private.h +++ b/platform/darwin/src/MLNStyleValue_Private.h @@ -201,7 +201,7 @@ class MLNStyleValueTransformer { } } - // Padding + // Padding as array void getMBGLValue(id rawValue, std::array &mbglValue) { if ([rawValue isKindOfClass:[NSValue class]]) { mbglValue = [rawValue mgl_paddingArrayValue]; @@ -215,6 +215,27 @@ class MLNStyleValueTransformer { } } + // Padding type (supports numbers and float arrays w/ sizes 1 to 4) + void getMBGLValue(id rawValue, mbgl::Padding &mbglValue) { + if ([rawValue isKindOfClass:[NSNumber class]]) { + NSNumber *number = (NSNumber *)rawValue; + mbglValue = mbgl::Padding(number.floatValue); + } else if ([rawValue isKindOfClass:[NSArray class]]) { + NSArray *array = (NSArray *)rawValue; + if (array.count < 1 || array.count > 4) { + [NSException raise:NSInvalidArgumentException + format:@"Padding array should have from 1 to 4 elements."]; + } + std::array values; + for (size_t i = 0; i < array.count; ++i) { + getMBGLValue(array[i], values[i]); + } + mbglValue = mbgl::Padding(std::span(values.begin(), array.count)); + } else if ([rawValue isKindOfClass:[NSValue class]]) { + mbglValue = mbgl::Padding([rawValue mgl_paddingArrayValue]); + } + } + // Color void getMBGLValue(MLNColor *rawValue, mbgl::Color &mbglValue) { mbglValue = rawValue.mgl_color; } @@ -285,11 +306,16 @@ class MLNStyleValueTransformer { return [NSValue mgl_valueWithOffsetArray:mbglStopValue]; } - // Padding + // Padding as array static NSValue *toMLNRawStyleValue(const std::array &mbglStopValue) { return [NSValue mgl_valueWithPaddingArray:mbglStopValue]; } + // Padding type + static NSValue *toMLNRawStyleValue(const mbgl::Padding &mbglStopValue) { + return [NSValue mgl_valueWithPaddingArray:mbglStopValue.toArray()]; + } + // Color static MLNColor *toMLNRawStyleValue(const mbgl::Color mbglStopValue) { return [MLNColor mgl_colorWithColor:mbglStopValue]; diff --git a/platform/node/src/node_expression.cpp b/platform/node/src/node_expression.cpp index efd03370cf2..e37b689bd0c 100644 --- a/platform/node/src/node_expression.cpp +++ b/platform/node/src/node_expression.cpp @@ -45,6 +45,7 @@ type::Type parseType(v8::Local type) { {"boolean", type::Boolean}, {"object", type::Object}, {"color", type::Color}, + {"padding", type::Padding}, {"value", type::Value}, {"formatted", type::Formatted}, {"number-format", type::String}, @@ -216,6 +217,13 @@ struct ToValue { static_cast(color.a)}); } + v8::Local operator()(const mbgl::Padding& padding) { + return operator()(std::vector{static_cast(padding.top), + static_cast(padding.right), + static_cast(padding.bottom), + static_cast(padding.left)}); + } + v8::Local operator()(const std::unordered_map& map) { Nan::EscapableHandleScope scope; v8::Local result = Nan::New(); diff --git a/scripts/generate-style-code.mjs b/scripts/generate-style-code.mjs index 7471f5bff0b..196726056f3 100755 --- a/scripts/generate-style-code.mjs +++ b/scripts/generate-style-code.mjs @@ -56,6 +56,8 @@ function expressionType(property) { return 'StringType'; case 'color': return `ColorType`; + case 'padding': + return `PaddingType`; case 'formatted': return `FormattedType`; case 'array': @@ -104,6 +106,8 @@ function evaluatedType(property) { return (isLightProperty(property) ? 'Light' : '') + `${camelize(property.name)}Type`; case 'color': return `Color`; + case 'padding': + return `Padding`; case 'array': if (property.length) { return `std::array<${evaluatedType({type: property.value, name: property.name})}, ${property.length}>`; @@ -248,6 +252,7 @@ function defaultValue(property) { return `{ ${color} }`; } case 'array': + case 'padding': const defaults = (property.default || []).map((/** @type {any} **/ e) => defaultValue({ type: property.value, default: e })); if (property.length) { return `{{${defaults.join(', ')}}}`; diff --git a/scripts/style-spec.mjs b/scripts/style-spec.mjs index f2f5299d708..fc1abe9895b 100644 --- a/scripts/style-spec.mjs +++ b/scripts/style-spec.mjs @@ -1,13 +1,5 @@ import referenceSpec from './style-spec-reference/v8.json' with { type: "json" }; -// https://github.com/maplibre/maplibre-native/issues/2368 -referenceSpec['layout_symbol']['icon-padding']['type'] = 'number'; -// @ts-ignore -referenceSpec['layout_symbol']['icon-padding']['default'] = 2; - -// https://github.com/maplibre/maplibre-native/issues/2754 -referenceSpec['layout_symbol']['icon-padding']['property-type'] = 'data constant'; - /** @type {any} */ let modifiedReferenceSpec = referenceSpec; diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp index 5cc9e6aaecf..aa6156ace8f 100644 --- a/src/mbgl/layout/symbol_instance.cpp +++ b/src/mbgl/layout/symbol_instance.cpp @@ -87,7 +87,7 @@ SymbolInstance::SymbolInstance(Anchor& anchor_, const SymbolPlacementType textPlacement, const std::array& textOffset_, const float iconBoxScale, - const float iconPadding, + const Padding iconPadding, const std::array& iconOffset_, const RefIndexedSubfeature& indexedFeature, const std::size_t layoutFeatureIndex_, diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 30753229ff5..16f97e0ab44 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -110,7 +110,7 @@ class SymbolInstance { style::SymbolPlacementType textPlacement, const std::array& textOffset, float iconBoxScale, - float iconPadding, + Padding iconPadding, const std::array& iconOffset, const RefIndexedSubfeature& indexedFeature, std::size_t layoutFeatureIndex, diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 9ab466e13ad..b46096e0063 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -566,7 +566,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex, const float iconBoxScale = tilePixelRatio * layoutIconSize; const float symbolSpacing = tilePixelRatio * layout->get(); const float textPadding = layout->get() * tilePixelRatio; - const float iconPadding = layout->get() * tilePixelRatio; + const Padding iconPadding = layout->evaluate(zoom, feature, canonicalID) * tilePixelRatio; const float textMaxAngle = util::deg2radf(layout->get()); const float iconRotation = layout->evaluate(zoom, feature, canonicalID); const float textRotation = layout->evaluate(zoom, feature, canonicalID); diff --git a/src/mbgl/style/conversion/constant.cpp b/src/mbgl/style/conversion/constant.cpp index 55aa0d2648b..a52a155beef 100644 --- a/src/mbgl/style/conversion/constant.cpp +++ b/src/mbgl/style/conversion/constant.cpp @@ -113,6 +113,28 @@ std::optional Converter::operator()(const Convertible& value, Erro return *color; } +std::optional Converter::operator()(const Convertible& value, Error& error) const { + std::optional result; + if (isArray(value)) { + if (arrayLength(value) > 0 && arrayLength(value) <= 4) { + auto vector = Converter>{}(value, error); + if (vector) { + result = Padding(*vector); + } + } + } else { + std::optional number = toNumber(value); + if (number) { + result = Padding(*number); + } + } + + if (!result) { + error.message = "value must be a number or an array of numbers (between 1 and 4 elements)"; + } + return result; +} + template std::optional> Converter>::operator()(const Convertible& value, Error& error) const { diff --git a/src/mbgl/style/conversion/function.cpp b/src/mbgl/style/conversion/function.cpp index 75bd885bb72..cef1bf469a2 100644 --- a/src/mbgl/style/conversion/function.cpp +++ b/src/mbgl/style/conversion/function.cpp @@ -131,6 +131,9 @@ template std::optional> convertFunctionToExpres Error&, bool); template std::optional> convertFunctionToExpression(const Convertible&, Error&, bool); +template std::optional> convertFunctionToExpression(const Convertible&, + Error&, + bool); template std::optional> convertFunctionToExpression(const Convertible&, Error&, bool); @@ -209,6 +212,7 @@ enum class FunctionType { bool interpolatable(type::Type type) noexcept { return type.match([&](const type::NumberType&) { return true; }, [&](const type::ColorType&) { return true; }, + [&](const type::PaddingType&) { return true; }, [&](const type::Array& array) { return array.N && array.itemType == type::Number; }, [&](const auto&) { return false; }); } @@ -246,6 +250,13 @@ std::optional> convertLiteral(type::Type type, } return literal(*result); }, + [&](const type::PaddingType&) -> std::optional> { + auto result = convert(value, error); + if (!result) { + return std::nullopt; + } + return literal(*result); + }, [&](const type::Array& array) -> std::optional> { if (!isArray(value)) { error.message = "value must be an array"; @@ -779,6 +790,9 @@ std::optional> convertFunctionToExpression(type::Typ [&](const type::ColorType&) -> std::optional> { return toColor(get(literal(*property)), defaultExpr()); }, + [&](const type::PaddingType&) -> std::optional> { + return toPadding(get(literal(*property)), defaultExpr()); + }, [&](const type::Array& array) -> std::optional> { return assertion(array, get(literal(*property)), defaultExpr()); }, diff --git a/src/mbgl/style/conversion/property_value.cpp b/src/mbgl/style/conversion/property_value.cpp index e7cac3b4221..65d9b75c1b6 100644 --- a/src/mbgl/style/conversion/property_value.cpp +++ b/src/mbgl/style/conversion/property_value.cpp @@ -78,6 +78,8 @@ template std::optional> Converter>::op conversion::Error&, bool, bool) const; +template std::optional> Converter>::operator()( + conversion::Convertible const&, conversion::Error&, bool, bool) const; template std::optional> Converter>::operator()( conversion::Convertible const&, conversion::Error&, bool, bool) const; template std::optional>> diff --git a/src/mbgl/style/conversion/stringify.hpp b/src/mbgl/style/conversion/stringify.hpp index cd7c968b58f..0e198336098 100644 --- a/src/mbgl/style/conversion/stringify.hpp +++ b/src/mbgl/style/conversion/stringify.hpp @@ -85,6 +85,11 @@ void stringify(Writer& writer, const std::array& v) { writer.EndArray(); } +template +void stringify(Writer& writer, const Padding& v) { + stringify(writer, v.toArray()); +} + template void stringify(Writer&, const Value&); diff --git a/src/mbgl/style/expression/check_subtype.cpp b/src/mbgl/style/expression/check_subtype.cpp index 356c9061bbf..050e75db220 100644 --- a/src/mbgl/style/expression/check_subtype.cpp +++ b/src/mbgl/style/expression/check_subtype.cpp @@ -33,7 +33,8 @@ std::optional checkSubtype(const Type& expected, const Type& t) { [&](const ValueType&) -> std::optional { if (t.is()) return {}; - const Type members[] = {Null, Boolean, Number, String, Object, Color, Formatted, Image, Array(Value)}; + const Type members[] = { + Null, Boolean, Number, String, Object, Color, Padding, Formatted, Image, Array(Value)}; for (const auto& member : members) { const auto err = checkSubtype(member, t); diff --git a/src/mbgl/style/expression/coercion.cpp b/src/mbgl/style/expression/coercion.cpp index d3f82a1dfaa..10e5edfe6e4 100644 --- a/src/mbgl/style/expression/coercion.cpp +++ b/src/mbgl/style/expression/coercion.cpp @@ -72,6 +72,32 @@ EvaluationResult toColor(const Value& colorValue) { }); } +EvaluationResult toPadding(const Value& paddingValue) { + return paddingValue.match( + [](const Padding& padding) -> EvaluationResult { return padding; }, + [&](const std::vector& components) -> EvaluationResult { + const std::size_t len = components.size(); + const bool isNumeric = std::all_of(components.begin(), components.end(), [](const Value& item) -> bool { + return item.template is(); + }); + if ((len >= 1 && len <= 4) && isNumeric) { + float componentsAsFloats[4] = {0}; + for (std::size_t i = 0; i < len; i++) { + componentsAsFloats[i] = static_cast(components[i].template get()); + } + return Padding(std::span(componentsAsFloats, len)); + } else { + return EvaluationError{"Invalid padding value " + stringify(paddingValue) + + ": expected an array containing from one to four " + "numeric values."}; + } + }, + [](const double number) -> EvaluationResult { return Padding(static_cast(number)); }, + [&](const auto&) -> EvaluationResult { + return EvaluationError{"Could not parse padding from value '" + stringify(paddingValue) + "'"}; + }); +} + EvaluationResult toFormatted(const Value& formattedValue) { return Formatted(toString(formattedValue).c_str()); } @@ -89,6 +115,8 @@ CoerceFunction getCoerceFunction(const type::Type& t) { return toBoolean; } else if (t.is()) { return toColor; + } else if (t.is()) { + return toPadding; } else if (t.is()) { return toNumber; } else if (t.is()) { @@ -135,6 +163,7 @@ mbgl::Value Coercion::serialize() const { std::string Coercion::getOperator() const { auto s = getType().match([](const type::BooleanType&) -> std::string_view { return "to-boolean"; }, [](const type::ColorType&) -> std::string_view { return "to-color"; }, + [](const type::PaddingType&) -> std::string_view { return "to-padding"; }, [](const type::NumberType&) -> std::string_view { return "to-number"; }, [](const type::StringType&) -> std::string_view { return "to-string"; }, [](const auto&) noexcept -> std::string_view { @@ -148,6 +177,7 @@ using namespace mbgl::style::conversion; ParseResult Coercion::parse(const Convertible& value, ParsingContext& ctx) { static std::unordered_map types{{"to-boolean", type::Boolean}, {"to-color", type::Color}, + {"to-padding", type::Padding}, {"to-number", type::Number}, {"to-string", type::String}}; @@ -170,8 +200,9 @@ ParseResult Coercion::parse(const Convertible& value, ParsingContext& ctx) { /** * Special form for error-coalescing coercion expressions "to-number", - * "to-color". Since these coercions can fail at runtime, they accept - * multiple arguments, only evaluating one at a time until one succeeds. + * "to-color", "to-padding". Since these coercions can fail at runtime, + * they accept multiple arguments, only evaluating one at a time until + * one succeeds. */ std::vector> parsed; diff --git a/src/mbgl/style/expression/dsl.cpp b/src/mbgl/style/expression/dsl.cpp index 26657949e0d..dc518d38fd0 100644 --- a/src/mbgl/style/expression/dsl.cpp +++ b/src/mbgl/style/expression/dsl.cpp @@ -117,6 +117,10 @@ std::unique_ptr toColor(std::unique_ptr value, std::uniq return coercion(type::Color, std::move(value), std::move(def)); } +std::unique_ptr toPadding(std::unique_ptr value, std::unique_ptr def) { + return coercion(type::Padding, std::move(value), std::move(def)); +} + std::unique_ptr toString(std::unique_ptr value, std::unique_ptr def) { return coercion(type::String, std::move(value), std::move(def)); } diff --git a/src/mbgl/style/expression/interpolate.cpp b/src/mbgl/style/expression/interpolate.cpp index 2c49bca5118..889d6ea37c9 100644 --- a/src/mbgl/style/expression/interpolate.cpp +++ b/src/mbgl/style/expression/interpolate.cpp @@ -232,6 +232,10 @@ ParseResult createInterpolate(type::Type type, return ParseResult( std::make_unique>(type, interpolator, std::move(input), std::move(stops))); }, + [&](const type::PaddingType&) -> ParseResult { + return ParseResult( + std::make_unique>(type, interpolator, std::move(input), std::move(stops))); + }, [&](const type::Array& arrayType) -> ParseResult { if (arrayType.itemType != type::Number || !arrayType.N) { ctx.error("Type " + toString(type) + " is not interpolatable."); diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index cbeb9547455..12e11c4232b 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -134,6 +134,7 @@ constexpr const auto expressionRegistry = mapbox::eternal::hash_map())) { + parsed = {annotate( + std::move(*parsed), std::move(*expected), typeAnnotationOption.value_or(TypeAnnotationOption::coerce))}; } else { checkType((*parsed)->getType()); if (!errors->empty()) { diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp index 3bc06a4322f..0701d0dac32 100644 --- a/src/mbgl/style/expression/value.cpp +++ b/src/mbgl/style/expression/value.cpp @@ -12,6 +12,7 @@ type::Type typeOf(const Value& value) { [&](double) -> type::Type { return type::Number; }, [&](const std::string&) -> type::Type { return type::String; }, [&](const Color&) -> type::Type { return type::Color; }, + [&](const Padding&) -> type::Type { return type::Padding; }, [&](const Collator&) -> type::Type { return type::Collator; }, [&](const Formatted&) -> type::Type { return type::Formatted; }, [&](const Image&) -> type::Type { return type::Image; }, @@ -53,6 +54,7 @@ void writeJSON(rapidjson::Writer& writer, const Value& }, [&](const std::string& s) { writer.String(s); }, [&](const Color& c) { writer.String(c.stringify()); }, + [&](const Padding& p) { mbgl::style::conversion::stringify(writer, p); }, [&](const Collator&) { // Collators are excluded from constant folding and there's no Literal parser // for them so there shouldn't be any way to serialize this value. @@ -123,6 +125,7 @@ Value ValueConverter::toExpressionValue(const mbgl::Value& value) { mbgl::Value ValueConverter::fromExpressionValue(const Value& value) { return value.match( [&](const Color& color) -> mbgl::Value { return color.serialize(); }, + [&](const Padding& padding) -> mbgl::Value { return padding.serialize(); }, [&](const Collator&) -> mbgl::Value { // fromExpressionValue can't be used for Collator values, // because they have no meaningful representation as an mbgl::Value @@ -309,6 +312,10 @@ type::Type valueTypeToExpressionType() { return type::Color; } template <> +type::Type valueTypeToExpressionType() { + return type::Padding; +} +template <> type::Type valueTypeToExpressionType() { return type::Collator; } diff --git a/src/mbgl/style/layers/symbol_layer.cpp b/src/mbgl/style/layers/symbol_layer.cpp index a2ef653e3bd..949f571a1b0 100644 --- a/src/mbgl/style/layers/symbol_layer.cpp +++ b/src/mbgl/style/layers/symbol_layer.cpp @@ -168,15 +168,15 @@ void SymbolLayer::setIconOptional(const PropertyValue& value) { baseImpl = std::move(impl_); observer->onLayerChanged(*this); } -PropertyValue SymbolLayer::getDefaultIconPadding() { +PropertyValue SymbolLayer::getDefaultIconPadding() { return IconPadding::defaultValue(); } -const PropertyValue& SymbolLayer::getIconPadding() const { +const PropertyValue& SymbolLayer::getIconPadding() const { return impl().layout.get(); } -void SymbolLayer::setIconPadding(const PropertyValue& value) { +void SymbolLayer::setIconPadding(const PropertyValue& value) { if (value == getIconPadding()) return; auto impl_ = mutableImpl(); impl_->layout.get() = value; @@ -1628,39 +1628,15 @@ std::optional SymbolLayer::setPropertyInternal(const std::string& name, c return std::nullopt; } } - if (property == Property::IconPadding || property == Property::SymbolSpacing || - property == Property::TextLineHeight || property == Property::TextMaxAngle || - property == Property::TextPadding) { + if (property == Property::IconPadding) { Error error; - const auto& typedValue = convert>(value, error, false, false); + const auto& typedValue = convert>(value, error, true, false); if (!typedValue) { return error; } - if (property == Property::IconPadding) { - setIconPadding(*typedValue); - return std::nullopt; - } - - if (property == Property::SymbolSpacing) { - setSymbolSpacing(*typedValue); - return std::nullopt; - } - - if (property == Property::TextLineHeight) { - setTextLineHeight(*typedValue); - return std::nullopt; - } - - if (property == Property::TextMaxAngle) { - setTextMaxAngle(*typedValue); - return std::nullopt; - } - - if (property == Property::TextPadding) { - setTextPadding(*typedValue); - return std::nullopt; - } + setIconPadding(*typedValue); + return std::nullopt; } if (property == Property::IconPitchAlignment || property == Property::IconRotationAlignment || property == Property::TextPitchAlignment || property == Property::TextRotationAlignment) { @@ -1720,6 +1696,34 @@ std::optional SymbolLayer::setPropertyInternal(const std::string& name, c setSymbolPlacement(*typedValue); return std::nullopt; } + if (property == Property::SymbolSpacing || property == Property::TextLineHeight || + property == Property::TextMaxAngle || property == Property::TextPadding) { + Error error; + const auto& typedValue = convert>(value, error, false, false); + if (!typedValue) { + return error; + } + + if (property == Property::SymbolSpacing) { + setSymbolSpacing(*typedValue); + return std::nullopt; + } + + if (property == Property::TextLineHeight) { + setTextLineHeight(*typedValue); + return std::nullopt; + } + + if (property == Property::TextMaxAngle) { + setTextMaxAngle(*typedValue); + return std::nullopt; + } + + if (property == Property::TextPadding) { + setTextPadding(*typedValue); + return std::nullopt; + } + } if (property == Property::SymbolZOrder) { Error error; const auto& typedValue = convert>(value, error, false, false); diff --git a/src/mbgl/style/layers/symbol_layer_properties.hpp b/src/mbgl/style/layers/symbol_layer_properties.hpp index 296fc9e6369..cdc1fdf9b8f 100644 --- a/src/mbgl/style/layers/symbol_layer_properties.hpp +++ b/src/mbgl/style/layers/symbol_layer_properties.hpp @@ -51,9 +51,9 @@ struct IconOptional : LayoutProperty { static bool defaultValue() { return false; } }; -struct IconPadding : LayoutProperty { +struct IconPadding : DataDrivenLayoutProperty { static constexpr const char *name() { return "icon-padding"; } - static float defaultValue() { return 2.f; } + static Padding defaultValue() { return {2}; } }; struct IconPitchAlignment : LayoutProperty { diff --git a/src/mbgl/text/collision_feature.cpp b/src/mbgl/text/collision_feature.cpp index 8687425b61f..9bc06384d5d 100644 --- a/src/mbgl/text/collision_feature.cpp +++ b/src/mbgl/text/collision_feature.cpp @@ -24,7 +24,7 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line, shapedText.right, std::nullopt, boxScale, - padding, + {padding}, overscaling, rotate); } @@ -33,7 +33,7 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line, const Anchor& anchor, std::optional shapedIcon, const float boxScale, - const float padding, + const Padding& padding, const RefIndexedSubfeature& indexedFeature_, const float rotate) : indexedFeature(std::move(indexedFeature_)), @@ -68,15 +68,15 @@ void CollisionFeature::initialize(const GeometryCoordinates& line, float right, const std::optional& collisionPadding, float boxScale, - float padding, + const Padding& padding, float overscaling, float rotate) { if (top == 0 && bottom == 0 && left == 0 && right == 0) return; - float y1 = top * boxScale - padding; - float y2 = bottom * boxScale + padding; - float x1 = left * boxScale - padding; - float x2 = right * boxScale + padding; + float y1 = top * boxScale - padding.top; + float y2 = bottom * boxScale + padding.bottom; + float x1 = left * boxScale - padding.left; + float x2 = right * boxScale + padding.right; if (collisionPadding) { x1 -= collisionPadding->left * boxScale; diff --git a/src/mbgl/text/collision_feature.hpp b/src/mbgl/text/collision_feature.hpp index 819cca508ba..048003aaef3 100644 --- a/src/mbgl/text/collision_feature.hpp +++ b/src/mbgl/text/collision_feature.hpp @@ -103,7 +103,7 @@ class CollisionFeature { const Anchor& anchor, std::optional shapedIcon, const float boxScale, - const float padding, + const Padding& padding, const RefIndexedSubfeature& indexedFeature_, const float rotate); @@ -120,7 +120,7 @@ class CollisionFeature { float right, const std::optional& collisionPadding, float boxScale, - float padding, + const Padding& padding, float overscaling, float rotate); void bboxifyLabel(const GeometryCoordinates& line, diff --git a/src/mbgl/text/shaping.hpp b/src/mbgl/text/shaping.hpp index eb9a95f22cb..83a33c8943d 100644 --- a/src/mbgl/text/shaping.hpp +++ b/src/mbgl/text/shaping.hpp @@ -28,20 +28,6 @@ style::TextJustifyType getAnchorJustification(style::SymbolAnchorType anchor); class SymbolFeature; class BiDi; -class Padding { -public: - float left = 0; - float top = 0; - float right = 0; - float bottom = 0; - - explicit operator bool() const { return left != 0 || top != 0 || right != 0 || bottom != 0; } - - bool operator==(const Padding& rhs) const { - return left == rhs.left && top == rhs.top && right == rhs.right && bottom == rhs.bottom; - } -}; - class PositionedIcon { private: PositionedIcon( diff --git a/src/mbgl/util/padding.cpp b/src/mbgl/util/padding.cpp new file mode 100644 index 00000000000..afe2fb74268 --- /dev/null +++ b/src/mbgl/util/padding.cpp @@ -0,0 +1,12 @@ +#include +#include + +namespace mbgl { +std::array Padding::toArray() const { + return {{top, right, bottom, left}}; +} + +mbgl::Value Padding::serialize() const { + return std::vector{top, right, bottom, left}; +} +} // namespace mbgl diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 814b31c1dc4..5c70fc95170 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -48,6 +48,7 @@ add_library( ${PROJECT_SOURCE_DIR}/test/style/conversion/geojson_options.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/layer.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/light.test.cpp + ${PROJECT_SOURCE_DIR}/test/style/conversion/padding.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/property_value.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/stringify.test.cpp ${PROJECT_SOURCE_DIR}/test/style/conversion/tileset.test.cpp @@ -95,6 +96,7 @@ add_library( ${PROJECT_SOURCE_DIR}/test/util/memory.test.cpp ${PROJECT_SOURCE_DIR}/test/util/merge_lines.test.cpp ${PROJECT_SOURCE_DIR}/test/util/number_conversions.test.cpp + ${PROJECT_SOURCE_DIR}/test/util/padding.test.cpp ${PROJECT_SOURCE_DIR}/test/util/position.test.cpp ${PROJECT_SOURCE_DIR}/test/util/projection.test.cpp ${PROJECT_SOURCE_DIR}/test/util/rotation.test.cpp diff --git a/test/android/.gitignore b/test/android/.gitignore index ac96206f319..58cb249b9e2 100644 --- a/test/android/.gitignore +++ b/test/android/.gitignore @@ -1 +1,3 @@ -.gradle/ \ No newline at end of file +.gradle/ +**/.cxx/ +app/build \ No newline at end of file diff --git a/test/style/conversion/padding.test.cpp b/test/style/conversion/padding.test.cpp new file mode 100644 index 00000000000..00723bc89ed --- /dev/null +++ b/test/style/conversion/padding.test.cpp @@ -0,0 +1,102 @@ +#include + +#include +#include +#include +#include + +#include + +using namespace mbgl; +using namespace mbgl::style; +using namespace mbgl::style::conversion; + +TEST(StyleConversion, Padding) { + Error error; + auto parsePadding = [&](const std::string& src) { + return convertJSON(src, error); + }; + + { + // Single value is applied to all sides + auto padding = parsePadding("3"); + ASSERT_TRUE(padding.has_value()); + ASSERT_EQ(padding->top, 3); + ASSERT_EQ(padding->right, 3); + ASSERT_EQ(padding->bottom, 3); + ASSERT_EQ(padding->left, 3); + } + { + // Two values apply to [top/bottom, left/right] + auto padding = parsePadding("[3.5, 4.5]"); + ASSERT_TRUE(padding.has_value()); + ASSERT_EQ(padding->top, 3.5); + ASSERT_EQ(padding->bottom, 3.5); + ASSERT_EQ(padding->left, 4.5); + ASSERT_EQ(padding->right, 4.5); + } + { + // Three values apply to [top, left/right, bottom] e.g. [2, 3, 1]; + auto padding = parsePadding("[2, 3, 1]"); + ASSERT_TRUE(padding.has_value()); + ASSERT_EQ(padding->top, 2); + ASSERT_EQ(padding->left, 3); + ASSERT_EQ(padding->right, 3); + ASSERT_EQ(padding->bottom, 1); + } + { + // Four values apply to [top, right, bottom, left], e.g. [2, 3, 1, 0]. + auto padding = parsePadding("[-2.5, -3.5, -1.0, -0.5]"); + ASSERT_TRUE(padding.has_value()); + ASSERT_EQ(padding->top, -2.5); + ASSERT_EQ(padding->right, -3.5); + ASSERT_EQ(padding->bottom, -1.0); + ASSERT_EQ(padding->left, -0.5); + } + + // Invalid values + + const auto expectedError = "value must be a number or an array of numbers (between 1 and 4 elements)"; + { + // null + auto padding = parsePadding("null"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // bool + auto padding = parsePadding("false"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // empty array + auto padding = parsePadding("[]"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // too long array + auto padding = parsePadding("[1, 2, 3, 4, 5]"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // array of wrong types + auto padding = parsePadding("[true, false]"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // array with null + auto padding = parsePadding("[null]"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } + { + // object + auto padding = parsePadding("{\"foo\": 1}"); + ASSERT_FALSE(padding.has_value()); + ASSERT_EQ(error.message, expectedError); + } +} \ No newline at end of file diff --git a/test/style/conversion/stringify.test.cpp b/test/style/conversion/stringify.test.cpp index cc79b753383..a16ab45a7aa 100644 --- a/test/style/conversion/stringify.test.cpp +++ b/test/style/conversion/stringify.test.cpp @@ -128,6 +128,10 @@ TEST(Stringify, Layout) { SymbolLayoutProperties::Unevaluated layout; layout.get() = true; - layout.get() = 2.0; - ASSERT_EQ(stringify(layout), "{\"icon-padding\":2.0,\"symbol-avoid-edges\":true}"); + layout.get() = {2.f}; + ASSERT_EQ(stringify(layout), "{\"icon-padding\":[2.0,2.0,2.0,2.0],\"symbol-avoid-edges\":true}"); } + +TEST(Stringify, Padding) { + ASSERT_EQ("[3.5,7.0,9.0,11.0]", stringify(Padding(3.5, 7, 9, 11))); +} \ No newline at end of file diff --git a/test/util/padding.test.cpp b/test/util/padding.test.cpp new file mode 100644 index 00000000000..26f42816a79 --- /dev/null +++ b/test/util/padding.test.cpp @@ -0,0 +1,115 @@ +#include + +#include +#include + +#include +#include +#include +#include + +#include +#include +#include + +#include + +using namespace mbgl; +using namespace style; +using namespace style::expression; +using namespace style::expression::dsl; +using namespace std::string_literals; + +TEST(Padding, InterpolateExpression) { + // Use DLS methods to create an expression + { + auto expr = PropertyExpression( + interpolate(linear(), zoom(), 0.0, literal(Padding(0.0)), 1.0, literal(Padding(8.0)))); + + auto result = expr.evaluate(0.5f); + EXPECT_EQ(Padding(4.0), result); + } + + // Parse expression from JSON, verify that expansion from number/array is handled correctly. + { + auto json = + R"(["interpolate", ["linear"], ["zoom"], 0, ["to-padding", 0], 1, ["to-padding", ["literal",[8, 16, -32]]]])"; + PropertyExpression expr(createExpression(json)); + + auto result = expr.evaluate(0.5f); + EXPECT_EQ(Padding(4.0, 8, -16, 8), result); + } +} + +TEST(Padding, Function) { + auto fromFunction = [](const char* json) { + conversion::Error error; + auto function = conversion::convertJSON>( + json, error, /*allowDataExpressions*/ true, /*convertTokens*/ false); + return function->asExpression(); + }; + + auto evalInContext = [](const auto& expr, const PropertyMap& properties) { + StubGeometryTileFeature feature{{}, FeatureType::Unknown, {}, properties}; + EvaluationContext context{0, &feature}; + auto a = expr.evaluate(context); + return a; + }; + + // exponential + { + auto expr = fromFunction(R"({"type": "exponential", "stops": [[1, 2], [11, [2, 5, 2, 7]]]})"); + EXPECT_EQ(Padding(2, 2, 2, 2), expr.evaluate(0)); + EXPECT_EQ(Padding(2, 3.2, 2, 4), expr.evaluate(5)); + EXPECT_EQ(Padding(2, 5, 2, 7), expr.evaluate(11)); + } + // interval + { + auto expr = fromFunction(R"({"type": "interval", "stops": [[1, 2], [11, 4]]})"); + EXPECT_EQ(Padding(2), expr.evaluate(0)); + EXPECT_EQ(Padding(2), expr.evaluate(10)); + EXPECT_EQ(Padding(4), expr.evaluate(11)); + } + // categorical + { + auto expr = fromFunction( + R"({"type": "categorical", "property": "foo", "stops": [[0, 2], [1, 4]], "default": 6})"); + EXPECT_EQ(Padding(2), evalInContext(expr, {{"foo", 0}})); + EXPECT_EQ(Padding(4), evalInContext(expr, {{"foo", 1}})); + EXPECT_EQ(Padding(6), evalInContext(expr, {{"foo", 2}})); + EXPECT_EQ(Padding(6), evalInContext(expr, {{"bar", 0}})); + } + // identity + { + auto expr = fromFunction(R"({"type": "identity", "property": "foo", "default": [3, 7, 9, 11]})"); + EXPECT_EQ(Padding(2, 4, 6, 4), evalInContext(expr, {{"foo", std::vector({2, 4, 6})}})); + EXPECT_EQ(Padding(3), evalInContext(expr, {{"foo", 3}})); + EXPECT_EQ(Padding(3, 7, 9, 11), evalInContext(expr, {{"bar", 3}})); + } +} + +TEST(Padding, OperatorBool) { + Padding padding(0, 0, 0, 0); + EXPECT_FALSE(padding); + padding.left = 1; + EXPECT_TRUE(padding); +} + +TEST(Padding, OperatorEqual) { + auto a = Padding({{3.5f, 9}}); + auto b = Padding(3.5, 9, 3.5, 9); + EXPECT_TRUE(a == b); + EXPECT_FALSE(a != b); + + a.left = 7; + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); +} + +TEST(Padding, OperatorMultiply) { + auto padding = Padding(1, 2, 3, 4) * 2; + EXPECT_EQ(Padding(2, 4, 6, 8), padding); + + padding = padding * 0.0; + EXPECT_EQ(Padding(0), padding); +} \ No newline at end of file