[core] Variable label placement #14184

pozdnyakov · 2019-03-21T09:26:51Z

Port of mapbox/mapbox-gl-js#7596

platform/node/test/ignores.json

src/mbgl/layout/symbol_layout.cpp

src/mbgl/renderer/layers/render_symbol_layer.cpp

src/mbgl/text/placement.cpp

src/mbgl/layout/symbol_layout.cpp

… part

kkaefer · 2019-03-21T16:46:38Z

src/mbgl/style/conversion/constant.cpp

+namespace {
+
+}  // namespace
+


remove unused?

kkaefer · 2019-03-21T16:49:19Z

src/mbgl/text/shaping.cpp

@@ -360,12 +363,10 @@ const Shaping getShaping(const TaggedString& formattedString,
                         const style::TextJustifyType textJustify,
                         const float spacing,
                         const Point<float>& translate,
-                         const float verticalHeight,
+                         //const float verticalHeight,


julianrex · 2019-03-21T19:48:10Z

/cc @fabian-guerra for related platform/darwin/scripts/generate-style-code.js changes.

src/mbgl/layout/symbol_layout.cpp

pozdnyakov · 2019-03-22T18:53:45Z

@alexshalamov @kkaefer Thanks for your comments! Fixed now in the latest patch set. Also, an optimization for single-line labels is added.

chloekraw · 2019-03-22T20:06:51Z

@pozdnyakov - does the implementation in this PR still leave out runtime styling support?

pozdnyakov · 2019-03-25T11:59:50Z

@pozdnyakov - does the implementation in this PR still leave out runtime styling support?

This patch contains auto-generated runtime API & tests for Android.

alexshalamov · 2019-03-25T15:01:55Z

src/mbgl/layout/symbol_layout.cpp

+                    // but doesn't actually specify what happens if you use both. We go with the radial offset.
+                    textOffset = evaluateRadialOffset(textAnchor, radialOffset * util::ONE_EM);
+                } else {
+                    textOffset = { layout.evaluate<TextOffset>(zoom, feature)[0] * util::ONE_EM,


const auto evaluatedOffset = layout.evaluate<TextOffset>(zoom, feature); textOffset = { evaluatedOffset[0] * util::ONE_EM, evaluatedOffset[1] * util::ONE_EM };

@pozdnyakov wdyt? simple fix, avoids extra evaluation of text offset.

fabian-guerra · 2019-03-25T20:37:41Z

platform/android/scripts/generate-style-code.js

@@ -197,7 +197,12 @@ global.defaultValueJava = function(property) {
      case 'array':
             switch (property.value) {
              case 'string':
-                return '[' + property['default'] + "]";
+              case 'enum':
+                if (property['default'] !== undefined) {


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.

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

platform/darwin/src/MGLSymbolStyleLayer.mm

platform/darwin/src/MGLSymbolStyleLayer.h

platform/darwin/test/MGLSymbolStyleLayerTests.mm

fabian-guerra · 2019-03-28T00:33:49Z

platform/darwin/src/MGLStyleValue_Private.h

-        auto str = mbgl::Enum<MBGLEnum>::toString(value);
-        MGLEnum mglType = *mbgl::Enum<MGLEnum>::toEnum(str);
-        return [NSValue value:&mglType withObjCType:@encode(MGLEnum)];
+    static NSString *toMGLRawStyleValue(const MBGLEnum &value) {


Why are we changing NSValue to an NSString? Is there any MGLTextAnchor conversion error?

The previously existing method was never invoked - the currently removed

template <typename MBGLEnum = MBGLType, class = typename std::enable_if<std::is_enum<MBGLEnum>::value>::type, typename MGLEnum = ObjCEnum, class = typename std::enable_if<std::is_enum<MGLEnum>::value>::type> NSExpression *operator()(const MBGLEnum &value) const {...}

was used instead. It worked fine for a single MGLTextAnchor instance, however std::vector< MGLTextAnchor> could not be handled.

fabian-guerra · 2019-03-28T01:41:32Z

platform/darwin/src/MGLSymbolStyleLayer.h

+
+ You can set this property to an expression containing any of the following:
+
+ * Constant array values


If this is accepts MGLTextAnchor values, the documentation should be similar to:

mapbox-gl-native/platform/darwin/src/MGLSymbolStyleLayer.h

Lines 1145 to 1159 in 0ec8945

* Constant `MGLTextAnchor` values

* Any of the following constant string values:

* `center`: The center of the text is placed closest to the anchor.

* `left`: The left side of the text is placed closest to the anchor.

* `right`: The right side of the text is placed closest to the anchor.

* `top`: The top of the text is placed closest to the anchor.

* `bottom`: The bottom of the text is placed closest to the anchor.

* `top-left`: The top left corner of the text is placed closest to the

anchor.

* `top-right`: The top right corner of the text is placed closest to the

anchor.

* `bottom-left`: The bottom left corner of the text is placed closest to the

anchor.

* `bottom-right`: The bottom right corner of the text is placed closest to

the anchor.

tobrun

alexshalamov reviewed Mar 21, 2019

View reviewed changes

pozdnyakov added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Mar 21, 2019

pozdnyakov and others added 6 commits March 21, 2019 14:59

[core] Enable text-variable-anchor property

9616f89

[core] Enable 'text-radial-offset' property

2960381

[core] auto value for 'text-justify' field

8c95140

[core] Introduce variable text placement for point labels - Layout part

8fd6717

Export hideGlyphs and addDynamicAttributes symbols

9711b9d

[core] Introduce variable text placement for point labels - Render part

cefa35b

pozdnyakov force-pushed the variable_label_placement branch from 19df93a to c6f2cc7 Compare March 21, 2019 13:48

pozdnyakov removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Mar 21, 2019

pozdnyakov force-pushed the variable_label_placement branch from c6f2cc7 to ef803e1 Compare March 21, 2019 14:27

pozdnyakov requested a review from kkaefer March 21, 2019 14:28

pozdnyakov added 2 commits March 21, 2019 16:42

[core] Introduce variable text placement for point labels - Placement…

ab1f820

… part

Enable text-variable-anchor render tests

9b1def0

pozdnyakov force-pushed the variable_label_placement branch from ef803e1 to 9b1def0 Compare March 21, 2019 14:43

[android] Fix style code generation if array default value is missing

26d0a74

kkaefer reviewed Mar 21, 2019

View reviewed changes

alexshalamov reviewed Mar 22, 2019

View reviewed changes

src/mbgl/layout/symbol_layout.cpp Outdated Show resolved Hide resolved

src/mbgl/layout/symbol_layout.cpp Show resolved Hide resolved

[core] Single line optimization for variable label placement

cc6cc89

alexshalamov approved these changes Mar 25, 2019

View reviewed changes

fabian-guerra suggested changes Mar 25, 2019

View reviewed changes

[darwin] Support for variable text placement API

9bc4994

pozdnyakov requested a review from 1ec5 as a code owner March 27, 2019 12:24

pozdnyakov requested review from a team and fabian-guerra March 27, 2019 12:24

alexshalamov reviewed Mar 27, 2019

View reviewed changes

platform/darwin/src/MGLSymbolStyleLayer.mm Show resolved Hide resolved

platform/darwin/src/MGLSymbolStyleLayer.mm Show resolved Hide resolved

frederoni reviewed Mar 27, 2019

View reviewed changes

platform/darwin/src/MGLSymbolStyleLayer.h Show resolved Hide resolved

frederoni reviewed Mar 27, 2019

View reviewed changes

platform/darwin/test/MGLSymbolStyleLayerTests.mm Show resolved Hide resolved

fabian-guerra reviewed Mar 28, 2019

View reviewed changes

[Darwin] Improve documentation for enum array properties

4cad71a

fabian-guerra approved these changes Mar 28, 2019

View reviewed changes

tobrun approved these changes Mar 29, 2019

View reviewed changes

pozdnyakov merged commit 2cfdd1a into master Mar 29, 2019

pozdnyakov deleted the variable_label_placement branch March 29, 2019 14:18

pozdnyakov mentioned this pull request Apr 1, 2019

CP [core] Variable label placement #14280

Merged

osana mentioned this pull request Apr 3, 2019

[android] Updated Changelog for 7.4.0-alpha.1 release #14317

Merged

fabian-guerra mentioned this pull request May 9, 2019

[ios] Add example for variable label placement. mapbox/ios-sdk-examples#286

Merged

chloekraw added Core The cross-platform C++ core, aka mbgl feature labels Jul 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Variable label placement #14184

[core] Variable label placement #14184

pozdnyakov commented Mar 21, 2019

kkaefer Mar 21, 2019

kkaefer Mar 21, 2019

julianrex commented Mar 21, 2019

pozdnyakov commented Mar 22, 2019

chloekraw commented Mar 22, 2019

pozdnyakov commented Mar 25, 2019

alexshalamov Mar 25, 2019

alexshalamov Mar 27, 2019

fabian-guerra Mar 25, 2019

pozdnyakov Mar 27, 2019

fabian-guerra Mar 28, 2019

pozdnyakov Mar 28, 2019

fabian-guerra Mar 28, 2019

tobrun left a comment


		You can set this property to an expression containing any of the following:

		* Constant array values

	* Constant `MGLTextAnchor` values
	* Any of the following constant string values:
	* `center`: The center of the text is placed closest to the anchor.
	* `left`: The left side of the text is placed closest to the anchor.
	* `right`: The right side of the text is placed closest to the anchor.
	* `top`: The top of the text is placed closest to the anchor.
	* `bottom`: The bottom of the text is placed closest to the anchor.
	* `top-left`: The top left corner of the text is placed closest to the
	anchor.
	* `top-right`: The top right corner of the text is placed closest to the
	anchor.
	* `bottom-left`: The bottom left corner of the text is placed closest to the
	anchor.
	* `bottom-right`: The bottom right corner of the text is placed closest to
	the anchor.

[core] Variable label placement #14184

[core] Variable label placement #14184

Conversation

pozdnyakov commented Mar 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianrex commented Mar 21, 2019

pozdnyakov commented Mar 22, 2019

chloekraw commented Mar 22, 2019

pozdnyakov commented Mar 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobrun left a comment

Choose a reason for hiding this comment