Fix behaviour when using keep-text-upright in combination with text-offset places text

[danpat]

GL-JS version: Whatever was in use in Mapbox Studio on 2016-03-21:

I have direction-specific line geometry in some tiles. I want to offset labels to the right of the lines.

If I specify text-offset: [[0,1]], it works, but a bunch of the labels are upside-down (as expected):

If I enable "Keep text upright", then the text is upright, but the Y offset is interpreted incorrectly:

The expected behaviour would be for the labels to rotate around their offset anchor until they're "upright". It seems that the rotation to keep upright needs to be done before the label is offset, rather than after.

[andrewharvey]

Also applies to polygons where you want to keep labels either inside or out side the polygon.

Here is a minimal test case.

http://jsbin.com/lowekid/1/edit?html,output

The labels are in the right place, outside but some labels are upside down.

---

http://jsbin.com/danexuj/1/edit?html,output

All labels are the right way up, but some are in the wrong place.

[andrewharvey]

this also affects gl-native (confirmed via raster tiles api) so tagging as cross-platform.

[anandthakker]

I agree that it makes sense for text-offset to be interpreted relative to the line / polygon geometry (forward direction of the line coordinates being the positive x axis, and y axis 90 degrees clockwise), and for the keep-upright rotation to be applied after the anchor is offset. @ChrisLoer @ansis do you concur?

[ChrisLoer]

Yeah makes sense to me.

[anandthakker]

Copying question from @ansis from the PR #4779:

I think this change makes sense but are there any cases where the current behavior is the desired one? Is this a breaking change?

[anandthakker]

@ansis yeah, it's definitely not totally clear cut.

The style spec documentation for text-offset says:

Offset distance of text from its anchor. Positive values indicate right and down, while negative values indicate left and up

Now, this doesn't speak to the symbol-placement: line case, but to me it does seem to imply an intention that text-offset is with respect to the placement but not the orientation of the text. (For instance, it's applied before text-rotate, too.)

I'm inclined to call this a bug fix rather than a breaking change, because of the above and because it seems somewhat unlikely that many/any users would be relying upon the weirdly inconsistent broken behavior that would be changed here.

[andrewharvey]

My hunch is it's a breaking change, but I haven't taken the time to layout all possible scenarios.

I can think of cases where you always want the label above or below the line regardless of direction of the line, and convesley other times when you want it on the right or left of the line. There are also cases where you want the labels upright and othertimes to left as is.

https://jsbin.com/nihogoxope/edit?html,output

I'm sure there are use cases where instead of text your label is something like ">>>" to indicate direction, or even a icon font symbol.

[jfirebaugh]

I think I'm comfortable calling this a bug fix. It wasn't intended that text-keep-upright: true, in addition to its obvious effect, also acts as a toggle for keeping the label above or below the line regardless of direction of the line. If we need such a feature, it should have an independent property such as text-offset-anchor.

cc @mapbox/cartography-cats for further visibility about the key use case that's affected:

where you always want the label above or below the line regardless of direction of the line

Anyone know of concrete cases where removing the unintended ability of text-keep-upright: true to accomplish this would pose a problem?

---

I'm sure there are use cases where instead of text your label is something like ">>>" to indicate direction, or even a icon font symbol.

Those cases would be using text-keep-upright: false, so wouldn't be affected by this change, right?

[danpat]

I'm sure there are use cases where instead of text your label is something like ">>>" to indicate direction, or even a icon font symbol.

Those cases would be using text-keep-upright: false, so wouldn't be affected by this change, right?

Yes, that's a meta-bug in my original report - the arrows indicating direction are part of the label text, but there's no way to make the text upright without ending up with half the arrows pointing a different way.

[ansis]

I think I'm comfortable calling this a bug fix. It wasn't intended that text-keep-upright: true, in addition to its obvious effect, also acts as a toggle for keeping the label above or below the line regardless of direction of the line.

Historical note: text-offset was intended as a way to offset a point label in it's text shaping coordinate space. But I really never really thought about behavior for labels that follow lines since we couldn't render offset text around corners properly. We still can't, but #4781 is a step towards that. I'd say there wasn't any intended behavior for text-offset and line labels at all.

I'm also comfortable calling this a bug fix or defining undefined behavior.

After this change, how will text-keep-upright interact with text-anchor? For example, if you have "text-keep-upright": true, "text-anchor": "left" and rotate so that the text flips, which side of the label will it be on?

[anandthakker]

After this change, how will text-keep-upright interact with text-anchor? For example, if you have "text-keep-upright": true, "text-anchor": "left" and rotate so that the text flips, which side of the label will it be on?

@ansis good question. Added an integration test to #4779 that confirms that the fix therein doesn't change this behavior. (And it looks right to me: regardless of whether or not the text-keep-upright flipping is in effect, text-anchor: left results in the label's left side being aligned with the anchor.)

[ansis]

regardless of whether or not the text-keep-upright flipping is in effect, text-anchor: left results in the label's left side being aligned with the anchor

Is text-anchor: bottom supported for line labels? If it is, after this change, the label will sometimes be below the anchor and sometimes above?

[anandthakker]

It's supported, and also behaves as expected. I'll add it to the integration test.

[mb12]

@ansis and @anandthakker Can you please clarify if the following is a correct (or not) description of text-anchor for text placement on line geometries?

1. The line is re-sampled into anchor points based on symbol spacing value
   (plus other criterion).
2. text-anchor/text-offset etc are interpreted based on anchor points generated in step 1.

Other than the symbol spacing value, there aren't any knobs available to the end user that would impact 1.

[nickidlugash]

where you always want the label above or below the line regardless of direction of the line

Anyone know of concrete cases where removing the unintended ability of text-keep-upright: true to accomplish this would pose a problem?

I'm fairly sure that none of our Mapbox styles at least use this combination of style properties in this way; none of our styles should be affected if this fix/update is implemented. In the abstract, it seems like this would be a potentially useful styling technique but right now I cannot think of any concrete applications.

The carto team has tested out offset road labels (which for certain use cases can help improve legibility by not overlapping the labels and the geometries underneath). For this use case, having all the labels offset in the same direction – regardless of direction of traffic, i.e. the direction of the line – seems cleaner and more readable than offsets in the direction of the traffic at high zooms. At low zooms, offsets in the direction of traffic is probably better, so it's a bit of a toss up. However, the current jump in label positions when the orientation of the text flips is very jarring, especially with dense layers like road labels. For that reason, we deemed this styling technique impractical to use in a production style with the current behavior.

[anandthakker]

@mb12 yes, that sounds right. You can see this behavior in action in this integration test: https://github.com/mapbox/mapbox-gl-js/pull/4779/files#diff-16c07158e3ff79c74c4f937c9847d9a4