setStyle updates text-variable-anchor inconsistently

[samanpwbb]

mapbox-gl-js version: 1.0.0

browser: any

Steps to Trigger Behavior

1. Render a mapbox gl map with a text-variable-anchor value
2. Use setStyle to apply new text-variable-anchor values

The result is that Studio cannot provide a realistic preview of the user's style as the user changes the text-variable-anchor value.

Link to Demonstration

https://jsbin.com/riwasomula/1/edit?html,output

Expected Behavior

Expect label placement to change when preferred text anchor changes.

Actual Behavior

text placement does not change.

[samanpwbb]

Note that passing { diff: false } to setStyle does properly update text position – but this shouldn't be necessary.

[peterqliu]

@samanpwbb I can't reproduce the bug in the linked test case 🤔 changes are subtle, but if we disable the color change and bump the interval to 1000, labels are clearly updating:

[samanpwbb]

@samanpwbb I can't reproduce the bug in the linked test case 🤔 changes are subtle, but if we disable the color change and bump the interval to 1000, labels are clearly updating:

Yes, there are some subtle changes. I would expect label placement after a setStyle to be deterministic though. Right now, results of setStyle vary based on past label placement calculations. It appears as though once a given label finds a position that works, it will "prefer" that position in the future. Would it be possible to reset the "memory" of past positions after setStyle calls that effect text-variable-anchor?

[dereklieu]

This is a pretty simplistic example, but I think it helps illustrate why someone using this property in Studio might think the current behavior is buggy. I made the screengrab on 0.54.x but confirmed it on 1.1.0 as well.

Regular text-anchor

text-variable-anchor

[asheemmamoowala]

The styleDiff is working correctly. It generates the correct setLayoutProperty change operation in the jsbin provided.

The problem (I think) stems from preferring previous placement in:

mapbox-gl-js/src/symbol/placement.js

Lines 342 to 350 in 2ce452d

	// If we this symbol was in the last placement, shift the previously used
	// anchor to the front of the anchor list.
	if (this.prevPlacement && this.prevPlacement.variableOffsets[symbolInstance.crossTileID]) {
	const prevOffsets = this.prevPlacement.variableOffsets[symbolInstance.crossTileID];
	if (anchors[0] !== prevOffsets.anchor) {
	anchors = anchors.filter(anchor => anchor !== prevOffsets.anchor);
	anchors.unshift(prevOffsets.anchor);
	}
	}

If the previous placement anchor is not in the current set of anchors it should not be preferred. Currently it is added as long as it is not the first value in anchors.

[dereklieu]

@asheemmamoowala ah thanks for that reference. This would validate the experience of labels seeming to bias the current anchor, when the current anchor is among the list of selected anchors.

However, I'm having trouble understanding why setting a single anchor for text-variable-anchor, and then changing that anchor, does not cause the placement to change without zooming or panning the map.

text-variable-anchor: [ "center" ] -> [ "top-right" ]

[dereklieu]

Oops, just re-read it. Am I right in understanding that any anchor with a previous placement will always bias for that previous placement, even if it's no longer a favored placement? If so, that seems like unexpected behavior.

[asheemmamoowala]

Am I right in understanding that any anchor with a previous placement will always bias for that previous placement, even if it's no longer a favored placement? If so, that seems like unexpected behavior.

That's correct, this behavior is a bug. The previous placement should only be preferred if the chosen anchor continues to be in the text-variable-anchor property.

[asheemmamoowala]

Closed by #8473