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

[core] Variable label placement #14184

Merged
merged 12 commits into from
Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/mbgl/style/conversion/constant.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ struct Converter<T, typename std::enable_if_t<std::is_enum<T>::value>> {
optional<T> operator()(const Convertible& value, Error& error) const;
};

template <class T>
struct Converter<std::vector<T>, typename std::enable_if_t<std::is_enum<T>::value>> {
optional<std::vector<T>> operator()(const Convertible& value, Error& error) const;
};

template <>
struct Converter<Color> {
optional<Color> operator()(const Convertible& value, Error& error) const;
Expand Down
8 changes: 8 additions & 0 deletions include/mbgl/style/layers/symbol_layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ class SymbolLayer : public Layer {
PropertyValue<TextJustifyType> getTextJustify() const;
void setTextJustify(PropertyValue<TextJustifyType>);

static PropertyValue<float> getDefaultTextRadialOffset();
PropertyValue<float> getTextRadialOffset() const;
void setTextRadialOffset(PropertyValue<float>);

static PropertyValue<std::vector<TextVariableAnchorType>> getDefaultTextVariableAnchor();
PropertyValue<std::vector<TextVariableAnchorType>> getTextVariableAnchor() const;
void setTextVariableAnchor(PropertyValue<std::vector<TextVariableAnchorType>>);

static PropertyValue<SymbolAnchorType> getDefaultTextAnchor();
PropertyValue<SymbolAnchorType> getTextAnchor() const;
void setTextAnchor(PropertyValue<SymbolAnchorType>);
Expand Down
3 changes: 3 additions & 0 deletions include/mbgl/style/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ enum class AlignmentType : uint8_t {
};

enum class TextJustifyType : uint8_t {
Auto,
Center,
Left,
Right
Expand All @@ -98,6 +99,8 @@ enum class SymbolAnchorType : uint8_t {
BottomRight
};

using TextVariableAnchorType = SymbolAnchorType;

enum class TextTransformType : uint8_t {
None,
Uppercase,
Expand Down
3 changes: 3 additions & 0 deletions include/mbgl/util/constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ constexpr float MIN_ZOOM_F = MIN_ZOOM;
constexpr float MAX_ZOOM_F = MAX_ZOOM;
constexpr uint8_t DEFAULT_MAX_ZOOM = 22;

// ONE_EM constant used to go between "em" units used in style spec and "points" used internally for layout.
constexpr float ONE_EM = 24.0f;

constexpr uint8_t DEFAULT_PREFETCH_ZOOM_DELTA = 4;

constexpr uint64_t DEFAULT_MAX_CACHE_SIZE = 50 * 1024 * 1024;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ public final class Property {

// TEXT_JUSTIFY: Text justification options.

/**
* The text is aligned towards the anchor position.
*/
public static final String TEXT_JUSTIFY_AUTO = "auto";
/**
* The text is aligned to the left.
*/
Expand All @@ -339,6 +343,7 @@ public final class Property {
* Text justification options.
*/
@StringDef({
TEXT_JUSTIFY_AUTO,
TEXT_JUSTIFY_LEFT,
TEXT_JUSTIFY_CENTER,
TEXT_JUSTIFY_RIGHT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2275,6 +2275,46 @@ public static PropertyValue<Expression> textJustify(Expression value) {
return new LayoutPropertyValue<>("text-justify", value);
}

/**
* Radial offset of text, in the direction of the symbol's anchor. Useful in combination with {@link PropertyFactory#textVariableAnchor}, which doesn't support the two-dimensional {@link PropertyFactory#textOffset}.
*
* @param value a Float value
* @return property wrapper around Float
*/
public static PropertyValue<Float> textRadialOffset(Float value) {
return new LayoutPropertyValue<>("text-radial-offset", value);
}

/**
* Radial offset of text, in the direction of the symbol's anchor. Useful in combination with {@link PropertyFactory#textVariableAnchor}, which doesn't support the two-dimensional {@link PropertyFactory#textOffset}.
*
* @param value a Float value
* @return property wrapper around Float
*/
public static PropertyValue<Expression> textRadialOffset(Expression value) {
return new LayoutPropertyValue<>("text-radial-offset", value);
}

/**
* To increase the chance of placing high-priority labels on the map, you can provide an array of {@link Property.TEXT_ANCHOR} locations: the render will attempt to place the label at each location, in order, before moving onto the next label. Use `text-justify: auto` to choose justification based on anchor position. To apply an offset, use the {@link PropertyFactory#textRadialOffset} instead of the two-dimensional {@link PropertyFactory#textOffset}.
*
* @param value a String[] value
* @return property wrapper around String[]
*/
public static PropertyValue<String[]> textVariableAnchor(String[] value) {
return new LayoutPropertyValue<>("text-variable-anchor", value);
}

/**
* To increase the chance of placing high-priority labels on the map, you can provide an array of {@link Property.TEXT_ANCHOR} locations: the render will attempt to place the label at each location, in order, before moving onto the next label. Use `text-justify: auto` to choose justification based on anchor position. To apply an offset, use the {@link PropertyFactory#textRadialOffset} instead of the two-dimensional {@link PropertyFactory#textOffset}.
*
* @param value a String[] value
* @return property wrapper around String[]
*/
public static PropertyValue<Expression> textVariableAnchor(Expression value) {
return new LayoutPropertyValue<>("text-variable-anchor", value);
}

/**
* Part of the text placed closest to the anchor.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,30 @@ public PropertyValue<String> getTextJustify() {
return (PropertyValue<String>) new PropertyValue("text-justify", nativeGetTextJustify());
}

/**
* Get the TextRadialOffset property
*
* @return property wrapper value around Float
*/
@NonNull
@SuppressWarnings("unchecked")
public PropertyValue<Float> getTextRadialOffset() {
checkThread();
return (PropertyValue<Float>) new PropertyValue("text-radial-offset", nativeGetTextRadialOffset());
}

/**
* Get the TextVariableAnchor property
*
* @return property wrapper value around String[]
*/
@NonNull
@SuppressWarnings("unchecked")
public PropertyValue<String[]> getTextVariableAnchor() {
checkThread();
return (PropertyValue<String[]>) new PropertyValue("text-variable-anchor", nativeGetTextVariableAnchor());
}

/**
* Get the TextAnchor property
*
Expand Down Expand Up @@ -1185,6 +1209,14 @@ public PropertyValue<String> getTextTranslateAnchor() {
@Keep
private native Object nativeGetTextJustify();

@NonNull
@Keep
private native Object nativeGetTextRadialOffset();

@NonNull
@Keep
private native Object nativeGetTextVariableAnchor();

@NonNull
@Keep
private native Object nativeGetTextAnchor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ public void testTextJustifyAsConstant() {
assertNull(layer.getTextJustify().getValue());

// Set and Get
String propertyValue = TEXT_JUSTIFY_LEFT;
String propertyValue = TEXT_JUSTIFY_AUTO;
layer.setProperties(textJustify(propertyValue));
assertEquals(layer.getTextJustify().getValue(), propertyValue);
}
Expand All @@ -577,6 +577,32 @@ public void testTextJustifyAsExpression() {
assertEquals(layer.getTextJustify().getExpression(), expression);
}

@Test
@UiThreadTest
public void testTextRadialOffsetAsConstant() {
Timber.i("text-radial-offset");
assertNotNull(layer);
assertNull(layer.getTextRadialOffset().getValue());

// Set and Get
Float propertyValue = 0.3f;
layer.setProperties(textRadialOffset(propertyValue));
assertEquals(layer.getTextRadialOffset().getValue(), propertyValue);
}

@Test
@UiThreadTest
public void testTextVariableAnchorAsConstant() {
Timber.i("text-variable-anchor");
assertNotNull(layer);
assertNull(layer.getTextVariableAnchor().getValue());

// Set and Get
String[] propertyValue = new String[0];
layer.setProperties(textVariableAnchor(propertyValue));
assertEquals(layer.getTextVariableAnchor().getValue(), propertyValue);
}

@Test
@UiThreadTest
public void testTextAnchorAsConstant() {
Expand Down
7 changes: 6 additions & 1 deletion platform/android/scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,12 @@ global.defaultValueJava = function(property) {
case 'array':
switch (property.value) {
case 'string':
return '[' + property['default'] + "]";
case 'enum':
if (property['default'] !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has to be applied to darwin/scripts/generate-style-code.js and please run make darwin-style-code after. That's why the iOS and macOS tests fail in this branch.

Let me know if you have any questions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit more than this, pls see 9bc4994

return '[' + property['default'] + ']';
} else {
return 'new String[0]';
}
case 'number':
var result ='new Float[] {';
for (var i = 0; i < property.length; i++) {
Expand Down
11 changes: 11 additions & 0 deletions platform/android/src/conversion/constant.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ struct Converter<jni::Local<jni::Object<>>, T, typename std::enable_if_t<std::is
}
};

template <class T>
struct Converter<jni::Local<jni::Object<>>, std::vector<T>, typename std::enable_if_t<std::is_enum<T>::value>> {
Result<jni::Local<jni::Object<>>> operator()(jni::JNIEnv& env, const std::vector<T>& value) const {
auto result = jni::Array<jni::String>::New(env, value.size());
for (std::size_t i = 0; i < value.size(); ++i) {
result.Set(env, i, jni::Make<jni::String>(env, Enum<T>::toString(value.at(i))));
}
return result;
}
};

} // namespace conversion
} // namespace android
} // namespace mbgl
12 changes: 12 additions & 0 deletions platform/android/src/style/layers/symbol_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,16 @@ namespace android {
return std::move(*convert<jni::Local<jni::Object<>>>(env, toSymbolLayer(layer).getTextJustify()));
}

jni::Local<jni::Object<>> SymbolLayer::getTextRadialOffset(jni::JNIEnv& env) {
using namespace mbgl::android::conversion;
return std::move(*convert<jni::Local<jni::Object<>>>(env, toSymbolLayer(layer).getTextRadialOffset()));
}

jni::Local<jni::Object<>> SymbolLayer::getTextVariableAnchor(jni::JNIEnv& env) {
using namespace mbgl::android::conversion;
return std::move(*convert<jni::Local<jni::Object<>>>(env, toSymbolLayer(layer).getTextVariableAnchor()));
}

jni::Local<jni::Object<>> SymbolLayer::getTextAnchor(jni::JNIEnv& env) {
using namespace mbgl::android::conversion;
return std::move(*convert<jni::Local<jni::Object<>>>(env, toSymbolLayer(layer).getTextAnchor()));
Expand Down Expand Up @@ -514,6 +524,8 @@ namespace android {
METHOD(&SymbolLayer::getTextLineHeight, "nativeGetTextLineHeight"),
METHOD(&SymbolLayer::getTextLetterSpacing, "nativeGetTextLetterSpacing"),
METHOD(&SymbolLayer::getTextJustify, "nativeGetTextJustify"),
METHOD(&SymbolLayer::getTextRadialOffset, "nativeGetTextRadialOffset"),
METHOD(&SymbolLayer::getTextVariableAnchor, "nativeGetTextVariableAnchor"),
METHOD(&SymbolLayer::getTextAnchor, "nativeGetTextAnchor"),
METHOD(&SymbolLayer::getTextMaxAngle, "nativeGetTextMaxAngle"),
METHOD(&SymbolLayer::getTextRotate, "nativeGetTextRotate"),
Expand Down
4 changes: 4 additions & 0 deletions platform/android/src/style/layers/symbol_layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ class SymbolLayer : public Layer {

jni::Local<jni::Object<jni::ObjectTag>> getTextJustify(jni::JNIEnv&);

jni::Local<jni::Object<jni::ObjectTag>> getTextRadialOffset(jni::JNIEnv&);

jni::Local<jni::Object<jni::ObjectTag>> getTextVariableAnchor(jni::JNIEnv&);

jni::Local<jni::Object<jni::ObjectTag>> getTextAnchor(jni::JNIEnv&);

jni::Local<jni::Object<jni::ObjectTag>> getTextMaxAngle(jni::JNIEnv&);
Expand Down
21 changes: 3 additions & 18 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -149,24 +149,9 @@
"render-tests/symbol-sort-key/text-expression": "https://github.com/mapbox/mapbox-gl-native/issues/14028",
"render-tests/symbol-sort-key/text-placement": "https://github.com/mapbox/mapbox-gl-native/issues/14028",
"render-tests/fill-opacity/opaque-fill-over-symbol-layer": "skip - port https://github.com/mapbox/mapbox-gl-js/pull/7612",
"render-tests/text-radial-offset/basic": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/all-anchors-offset": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/all-anchors": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/icon-image-all-anchors": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/icon-image": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/no-animate-zoom": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/pitched-offset": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/pitched-rotated-debug": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/pitched-with-map": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/pitched": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/remember-last-placement": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/rotated-offset": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/rotated-with-map": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/rotated": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/single-justification": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/single-line": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/top-bottom-left-right": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-justify/auto": "skip - port of https://github.com/mapbox/mapbox-gl-js/pull/7596",
"render-tests/text-variable-anchor/pitched-rotated-debug": "skip - fails on gl-native due to worse accuracy at collision detection (float vs double) - needs issue",
alexshalamov marked this conversation as resolved.
Show resolved Hide resolved
"render-tests/text-variable-anchor/rotated-offset": "skip - fails on gl-native due to worse accuracy at collision detection (float vs double) - needs issue",
"render-tests/text-variable-anchor/remember-last-placement": "skip - fails on gl-native, as symbol index is not functional at static map mode - needs issue",
"render-tests/geojson/clustered-properties": "https://github.com/mapbox/mapbox-gl-native/issues/14043",
"render-tests/remove-feature-state/composite-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12413",
"render-tests/remove-feature-state/data-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12413",
Expand Down
6 changes: 5 additions & 1 deletion scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ global.evaluatedType = function (property) {
if (property.length) {
return `std::array<${evaluatedType({type: property.value})}, ${property.length}>`;
} else {
return `std::vector<${evaluatedType({type: property.value})}>`;
return `std::vector<${evaluatedType({type: property.value, name: property.name})}>`;
}
default: throw new Error(`unknown type for ${property.name}`)
}
Expand Down Expand Up @@ -153,7 +153,11 @@ global.defaultValue = function (property) {

switch (property.type) {
case 'number':
if (property.default === undefined) {
return 0;
} else {
return property.default;
}
case 'formatted':
case 'string':
return JSON.stringify(property.default || "");
Expand Down
7 changes: 1 addition & 6 deletions scripts/style-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,4 @@ var spec = module.exports = require('../mapbox-gl-js/src/style-spec/reference/v8
// Make temporary modifications here when Native doesn't have all features that JS has.
delete spec.layout_symbol['symbol-sort-key'];
delete spec.layout_symbol['symbol-z-order'].values['auto'];
spec.layout_symbol['symbol-z-order'].default = 'viewport-y';

delete spec.layout_symbol['text-variable-anchor'];
delete spec.layout_symbol['text-radial-offset'];
delete spec.layout_symbol['text-justify'].values['auto'];
spec.layout_symbol['text-offset'].requires.splice(1, 1); // { "!": "text-radial-offset" }
spec.layout_symbol['symbol-z-order'].default = 'viewport-y';
Loading