Glyph visibility culling

[kvark]

Helps #2890 quite a bit, includes #2894.
Reduces the vertex count from 6.8M to ~10K
TODO:

• try push
• profile in actual release
• reviews!

cc @gw3583 @lsalzman

---

This change is 

[gw3583]

I'm a little confused about why #2890 is so slow - I haven't looked in detail yet.

Since we split text runs at ~2048 glyphs per run, and each run should be visibility culled, how do we end up drawing so many vertices in this case?

Oh, perhaps it is because they are supplied in a single text run and the local bounding box for each run we split is the same?

If that's the case, we may be better off special casing that to calculate bounding boxes for each split - then we would only pay an extra cost in these edge cases where splits occur, and not on every glyph / run?

[kvark]

If that's the case, we may be better off special casing that to calculate bounding boxes for each split - then we would only pay an extra cost in these edge cases where splits occur, and not on every glyph / run?

Would that be better than the approach of this PR? To me, the cost of intersecting a rect (for visible glyphs) is completely shadowed by the work we do per glyph (accessing the resource map, requesting texture cache update, receiving it, etc), so doing it seems more universal and simple. Speaking of which, I think we should optimize the hell out of rectangle intersection, if we haven't already (cc @nical ).

[kvark]

Did a bit of benchmarking:

rev	backend	compositor
base	500ms	35ms
32bit glyph key	430ms	35ms
plus glyph culling	280ms	28ms

Notably, the compositor times are not that much better, despite 500x less vertices being sent. I believe the main cost here is transferring the vertex data, not actual rendering, and the driver eventually gets used to those :)

The backend time is still not nearly acceptable, so more changes are needed.

@gw3583 I checked the text run splitting code, it's fairly straightforward:

        for split_glyphs in glyphs.chunks(MAX_TEXT_RUN_LENGTH) {
            self.push_item(item, info);
            self.push_iter(split_glyphs);
        }

So clearly they inherit the same primitive info and can't be culled off independently. Not sure what the split gives us in this case.

How about this: we sub-divide the local clip rectangle as much as needed to ensure that the number of glyphs within each division is below the MAX_TEXT_RUN_LENGTH?

[kvark]

The problem with the approach I suggested is that it requires us to have an upper bound on the glyph size for a font (at display list population time) in order to do the local rect subdivision. I don't see a way to get that bound just yet. cc @lsalzman

[gw3583]

The reason I'm a bit unsure of visibility culling each glyph is somewhat hypothetical. If / when we get to a point where we are caching surfaces more aggressively for power saving, we probably want the culling to be a bit coarser / rarer (i.e. at the level of a glyph run split), since that would mean we wouldn't need to invalidate the cached surface as often (e.g. when it scrolls a bit and reveals a few new glyphs at a time). Happy to discuss on irc / vidyo in detail to see if that makes sense or is not really relevant here!

[kvark]

@gw3583 yes, that makes sense. To clarify: I'd prefer to not merge this code as is and instead fix the actual primitive run segmentation so that we can cull those as usual.

[bors-servo]

☔ The latest upstream changes (presumably #2965) made this pull request unmergeable. Please resolve the merge conflicts.

[kvark]

@lsalzman is there a way to get an upper bound on the glyph size/bounds per font instance?

[gw3583]

@kvark For freetype, https://www.freetype.org/freetype2/docs/tutorial/step2.html references the bbox field in global glyph metrics. It's defined:

'The global bounding box is defined as the smallest rectangle that can enclose all the glyphs in a font face.'

I'm not sure if other platform font APIs have something similar, but I suspect they do.

[kvark]

@gw3583 thank you! This bbox is looking like precisely what I need here.

[gw3583]

@kvark Should we close this until there is time to re-visit is? Or worth keeping open?