Improve line labels using text-offset

[ChrisLoer]

Summary

The status quo for line labels in MapLibre is that if a vertical offset is applied via text-offset, the glyph location is just moved away from its line in the direction of the normal vector. The problem with this approach is that at convex line vertices, this opens up gaps in the glyph placement, and at concave vertices, it causes glyphs to be jumbled on top of each other. (@1ec5 reported this as #2170)

This PR improves the behavior by offsetting the actual line geometry and then placing glyphs along that geometry with no further offset. In practice, this means shifting the line segments based on the offset, and then extending or shortening adjacent segments towards their point of intersection. Here's a clumsy sketch:

There are no public API changes with this PR: I view this as functionality that's implicitly promised by the style spec but not yet delivered.

Motivation

This change is important to us at Felt because we want to make it really easy to add line data to a map and style it well. Defaulting to having line labels offset from their lines is a much more flexible choice with this goal (you don't have to coordinate your text/halo color/width with your line color/width in order to be legible). Here are some examples:

Felt railway layer w/ default text-max-angle: line labels look bad at curves

Felt railway layer w/ default text-max-angle: 25: worst offenders removed, but we lose density (this is the compromise we're currently using)

Felt railway layer w/ this PR: 🎉

Testing and Performance

Look at the test update commit diff to see the change of behavior as captured in the tests. For a gently curving line, you can see the effect is just more even spacing. The "sharp" line tests expose some of the broken behavior of the previous implementation (although this behavior would normally be hidden by the default text-max-angle of 45 degrees). The point of the sharp tests is not that we want line labels that look like that, but just to exercise some extreme values. Note that the line-sharp-outside-center test is still showing the same behavior as before because the line extension algorithm doesn't handle the special case where the anchor point is exactly on a line vertex. This is a solvable problem but doesn't fit in easily with the algorithm as currently written. I decided it was OK to leave as-is because (1) it's not a regression from current behavior, and (2) in most maps it should be pretty rare for anchor points to line up exactly with line vertices.

For non-offset line labels, this should have a pretty trivial effect on performance. For line labels with offsets, this definitely adds cost that scales with the number of label-covered line vertices, and this is a performance hot spot. Still, the costs should be pretty modest (some point geometry arithmetic and the line intersection math), on the order of what we currently incur for line vertex projection. For someone trying to use this functionality, I think it's a pretty good bet that they'd prefer to have the slightly slower but also less-broken behavior from this PR.

I ran the "Paint" benchmark, which uses MapTiler Streets, so in theory this should be a test of non-offset performance, which shouldn't be noticeably changed. I didn't see any red flags in the benchmark, but it also seemed to me like results weren't very reproducible between runs, so I don't have a ton of confidence in this as a signal.

License Compliance

I was at Mapbox when @rreusser was working on an alternative solution to this problem in which actual offset curves are computed to place glyphs along as they go around line vertices. That approach definitely looks better, but also has more code complexity (and possibly perf impact? I haven't evaluated). My approach here is simpler, just trying to get legibility parity with non-offset line labels. In terms of license compliance -- it's been ~two years since I looked at his PR, and this is following a different approach, so I think this should be clean.

cc @ibesora

Launch Checklist

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[ChrisLoer]

I just took a second look at #2171

I'm not seeing that behavior with collision circles using this branch -- it looks like they're following what's expected:

I updated the "pathVertices" used for collision detection to follow the offset line, so that may have incidentally fixed that issue, but I haven't specifically tested the "two layers following same line" case.

[ChrisLoer]

This PR also includes some un-related formatting changes that I think are just coming from the .eslintrc settings in the project, but let me know if I should strip those out.

One thing -- I did a bunch of my interactive testing using my own local copy of debug-pages/debug.html, but it doesn't seem like it's in out-of-the-box working shape? (asks for fonts that aren't available, etc.) What are the go-to local debug options these days? Should I push some changes to debug.html to get it to render better, or put that in a separate PR?

In terms of which release to target, I assume it makes sense to merge this to main and then maybe if necessary cherry-pick to the 2.4 branch?

[HarelM]

Thanks for taking the time to open this PR!
I'll review it more thoroughly in the coming days.
A few minor comments from an initial scan:
Would be good to have some unit tests added as well to cover the code changes your made (unless I missed them...)
would be nice to improve projection cache typea bit more to indicate what the objects hold.

If I understand the idea correctly you take the offset vectors and cut them or prolong them as needed to create a new line that doesn't cut itself and doesn't have gaps, right? And only afterwards place the text.

[ChrisLoer]

Would be good to have some unit tests added as well to cover the code changes your made (unless I missed them...)

The added tests are all render tests under text-offset. Do you have a strong preference for "unit" vs "render" test here? My preference is "render", because:

• Way less "mocking" done, instead you use a real style
• Specification of the test is much more intelligible, both in the style.json and in the expected output
• It's still pretty well isolated to the specific thing we're testing, in the style of a unit test.

would be nice to improve projection cache typea bit more to indicate what the objects hold.

👍

If I understand the idea correctly you take the offset vectors and cut them or prolong them as needed to create a new line that doesn't cut itself and doesn't have gaps, right? And only afterwards place the text.

That's right. For historical background, in the original implementation there was no CPU projection (line label positioning and offsets were done entirely in the vertex shader). That's the constraint that led to the weird offset behavior in the first place, and we just never re-visited once we switched to CPU projection.

One thing about the algorithm is that it traverses the line from the anchor point for every single glyph to find a location. There's probably a somewhat more efficient algorithm in which we only the traverse the line once as we place glyphs, but IIRC when we did the original profiling the expensive part of re-traversing the line was just re-doing the projection work, so we just cached that as we went. Placing the first and last glyph before doing the rest of them is an important optimization, though, because it allows you to tell if the label is going to fit (and avoid collision) before you actually do the rest of the layout.

[HarelM]

Render tests are great, but you can cover a lot more with unit tests. Especially with all this math logic and edge cases.
I think that this PR deserves both render and unit tests.
Generally speaking, I would like to get to around 80% code coverage, so we need to make sure we cover the files we change well...

[1ec5]

Thanks for fixing this issue! It’s going to make a big difference for styles that adopt the North American convention of labeling roads, rivers, and boundaries as simple strokes with offset labels.

Before	After

When the map is tilted and the layer has text-pitch-alignment set to viewport, some symbols that were previously poorly kerned have better kerning, but individual characters aren’t being rotated to match the new positions:

Before1	After

The changes don’t seem to improve the kerning on layers that have text-anchor set to bottom:

All the screenshots above link to coordinates in OSM Americana that you can play with, but let me know if you need a reduced test case.

Footnotes

1. To see this in action, run: map.setLayoutProperty("boundary_country_label_left", "text-pitch-alignment", "viewport"); map.setLayoutProperty("boundary_country_label_right", "text-pitch-alignment", "viewport") ↩

[1ec5]

This is a fun test. Fortunately, most styles would apply a text-max-angle to roads and rivers to avoid letting this sort of thing get out of hand, but other things like boundaries and power lines could legitimately have very sharp angles while needing to be labeled.

Do you think there’s an opportunity to be consistent with word-wrapping on point-placed labels, trying to align these very sharp corners with word boundaries instead of arbitrary characters? If this isn’t straightforward, it would be a nice feature to track separately.

[ChrisLoer]

Just to make sure it's clear: the problem this test exercises is that the anchor point for the label is directly on a vertex. The fact that it's a sharp angle just makes the break more dramatic. In this special case, we degenerate to the same behavior we had before the PR, for glyphs on either side of the starting vertex.

Do you think there’s an opportunity to be consistent with word-wrapping on point-placed labels, trying to align these very sharp corners with word boundaries instead of arbitrary characters? If this isn’t straightforward, it would be a nice feature to track separately.

That's an interesting idea! The heuristic would be something like "if you hit a vertex that's sharper than text-max-angle, but can back up to a space that's less than X units away from the vertex, try breaking the glyph stream there, jumping ahead to past the vertex and then if you can place at least one word before having to jump over vertices again, continue"? But I think it definitely makes sense to treat as a separate feature.

[ChrisLoer]

When the map is tilted and the layer has text-pitch-alignment set to viewport, some symbols that were previously poorly kerned have better kerning, but individual characters aren’t being rotated to match the new positions:

I'm not sure if "not being rotated" is the right description of what's going on? The glyphs are oriented to the normal of the offset line that they're place on. I think one thing that's changed is with a sharp convex angle, the intersection point is on the "inside" of the curve, which means switching to the next line happens sooner. This is the reason you see that last "D" rotated in the Ireland example -- it's shifted on to the next line. This behavior is the same as with glyphs placed along the centerline, but maybe it looks worse because it's not directly on top of the visual reference that kind of explains why it's happening (it looks bad either way).

I experimented a couple years back with interpolating the glyph rotation as you approached a sharp angle, and I don't have the screenshots anymore, but I remember that it looked surprisingly weird. The better solution (for both offset and centered text) is what I mentioned @rreusser was working on -- define a curve that goes around every vertex, and place points along it. I think that might be the best solution! But it adds more complexity and cost, which might be part of why it never merged to Mapbox GL.

One "cheap" experiment I haven't tried is just boosting the spacing in proportion to how sharp a curve is (e.g. normal spacing up to 25 degrees, and then maybe linearly scale to 1em of extra spacing on the way to 90 degrees). I doubt it will look great, but it might lessen the worst parts of things like that I - RELAN - D with near-90-degree angles. I may try this tomorrow.

The changes don’t seem to improve the kerning on layers that have text-anchor set to bottom:

Yeah unfortunately text-anchor has a totally different implementation -- the "offset" in that case is applied in the shaping process on the worker and is baked into the glyph quads (not the smaller x-only "glyphOffsetArray" that's passed to the foreground projection code). You could untangle this, but there would be some pretty annoying parts. I think this makes sense to treat as a separate feature as well (the shape of the feature would be something like "get the text-anchor effect for lines out of the shaping and passed forward to use in the foreground line offset behavior).

Thank you for the screenshots with links -- very clear!

[1ec5]

I just took a second look at #2171

I'm not seeing that behavior with collision circles using this branch -- it looks like they're following what's expected:

Yes, I can confirm in this area that the collision circles are in the expected positions. Unfortunately, that leaves me searching for a new theory as to why the boundary edge labels in https://github.com/ZeLonewolf/openstreetmap-americana/pull/753#issuecomment-1413315442 keep going missing, but it’s unlikely to be due to what you’re touching.

I'm not sure if "not being rotated" is the right description of what's going on? The glyphs are oriented to the normal of the offset line that they're place on. I think one thing that's changed is with a sharp convex angle, the intersection point is on the "inside" of the curve, which means switching to the next line happens sooner. This is the reason you see that last "D" rotated in the Ireland example -- it's shifted on to the next line. This behavior is the same as with glyphs placed along the centerline, but maybe it looks worse because it's not directly on top of the visual reference that kind of explains why it's happening (it looks bad either way).

Ah, I missed that little kink in the line beside the “D”. I’m not sure how the symbol is being placed here, since that angle has got to be more than the 20 degrees specified in text-max-angle for that layer. I can keep investigating and file a separate issue if you think it’s unrelated.

Yeah unfortunately text-anchor has a totally different implementation -- the "offset" in that case is applied in the shaping process on the worker and is baked into the glyph quads (not the smaller x-only "glyphOffsetArray" that's passed to the foreground projection code). You could untangle this, but there would be some pretty annoying parts. I think this makes sense to treat as a separate feature as well (the shape of the feature would be something like "get the text-anchor effect for lines out of the shaping and passed forward to use in the foreground line offset behavior).

Makes sense. Fortunately, the workaround in many cases could be to keep text-anchor as center and instead increase the text-offset by roughly half of text-size.

[HarelM]

@ChrisLoer thanks a lot for taking the time to do this!
I know I added some annoying comments but I believe this is for the greater good. :-)

[ChrisLoer]

@1ec5 It's maybe worth noting along with the text-anchor gotcha -- text-max-angle is calculated entirely in tile space (see check_max_angle.ts), so if you're in a pitched map the angles you're seeing on the screen may be different than the ones used for the calculation.