[mikemorris]

To minimize label clipping.

/cc @bsudekum @ansis @miccolis

[mikemorris]

This seems to fix some of the egregious clipping issues, but I'm still seeing some minor clipping.

• "Washington Monument & Grounds" clipped by "Maine Ave SW"
  
• Clipping from tiles with missing labels
  
  

[mikemorris]

Note that there are failing tests in the visual render suite because of this change. Is there an equivalent change that should be applied to mapbox-gl-js to generate updated fixtures, or is there an alternate workaround here @jfirebaugh?

Current

New

Diff

[jfirebaugh]

@mikemorris I believe the "Current" and "New" images are swapped. Should be:

EDIT: Swapped images in comment above.

In the "New" image, you can see that many of the labels (those that come from points within tile buffer area) are drawn twice, or maybe more if they're near a tile corner. This is a problem for static renders.

To fix this, I believe we need to clip labels by tile boundaries, and at that point I think we should not have different layout and rendering pathways for Still vs Continuous mode:

• Always addToBuffers during layout
• Always clip labels to tile boundaries during painting

cc @ansis

[kkaefer]

Ah dang, totally missed that the static API can render more than just individual tiles.

[ansis]

I believe we need to clip labels by tile boundaries

Yep, they need to be clipped by enabling the stencil test here.

Is there an equivalent change that should be applied to mapbox-gl-js to generate updated fixtures

Yes, whatever changes are made in -native need to be added to -js.

and at that point I think we should not have different layout and rendering pathways for Still vs Continuous mode

Separate paths allow for a pretty significant optimization. If labels are always clipped to tile boundaries, the quads used for rendering need to be included in multiple tiles not just one. Without clipping we can drop all the labels with anchors outside that tile's boundaries. Usually 50% - 66% of all labels are outside and can be dropped.

A single render path with clipping at tile boundaries would mean:

• 2x to 3x the amount of labels we need to generate quads for (slower initial loading)
• 2x to 3x bigger symbol buffers (how does this impact processed tile caching costs on mobile?)
• 2x to 3x more symbol geometries that are rendered each frame

[jfirebaugh]

My concern with having a separate rendering pathway is that even if we fix this specific issue, it will still be possible to make a style where labels look fine in MapMode::Continuous but then get clipped when rendering static tiles. With large enough fonts or text spacing, that's always going to be an issue, right? So it would be better if you could spot style issues like that right away in Mapbox Studio or when testing your mobile app.

[mb12]

@jfirebaugh Is still mode used to render vector tiles as PNG/JPG/raster tiles?

[ansis]

@jfirebaugh that's a very good point. I hadn't thought about that

[jfirebaugh]

My current thinking is that this approach is good enough as an expedient fix for 90% of the label rendering issues with static tiles. Let's get a test case for this in the test suite (hopefully City of London is a good one) and merge.

I expect that some cases will still have a rendering issue -- where a label is so big/long that it crosses over a tile despite not being anchored in the buffer area. This is probably what's happening with Colorado Springs in #3357. These will need to be dealt with by tweaking the style.

[lucaswoj]

These will need to be dealt with by tweaking the style.

Agreed. This is the whole point of vector tile buffers. We could go as far as to issue a warning if text-max-width > buffer width.

This doesn't capture all cases but maybe the remaining 90% of the 90% 😬

[jfirebaugh]

Thinking about the Colorado Springs case again, I realized that it's probably the MapMode::Continuous analog of this same issue, and not a "label too big/wide" issue. When the tile containing Colorado Springs is offscreen, we don't render the label even if it's within the buffer of the tile immediately to the east, which is onscreen. So then when the containing tile does get loaded, the label pops into view over into the eastern tile.

So I think we should merge this for the expedient fix, but then go back and make the more reliable fix, where we always clip labels to tile boundaries during painting. In order to get some of the nice performance characteristics of the existing approach perhaps we can cull the symbol geometries to those that actually intersect the tile.

[kkaefer]

Hmm, overall we'd like to move to a model that doesn't use clipping. In an interactive context, we can load adjacent tiles by widening the invisible buffer from the viewport to X pixels around it.

[mikemorris]

Rebased this onto master, test suite failures all look like fixed clipped symbols (yay!) or overdrawing issues that would be fixed by Always clip labels to tile boundaries during painting, which still needs to be implemented.

[mikemorris]

After clipping labels to tiles, I'm still seeing a single failing test case (overdrawn labels) locally for text-rotate function, wondering if the label clipping math fails to account for rotation properly.

[ansis]

All tests are passing now. There was a small bug in the condition that determines when clipping happens. Merging

[jfirebaugh]

Was there a test-suite test case added for this?

[ansis]

Was there a test-suite test case added for this?

No, most of the existing text tests have four tiles and already test tile crosstile clipping. I'm not sure how a clipping test would differ from any of the existing text tests. Do you have any ideas for tests that should be added?

[jfirebaugh]

We should have a test case that was failing before the change, and passing after, like one of the regions from the screenshots above.

[ansis]

pr for test: #3457