1595768 - Investigate why we rasterize glyphs so often

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task, P3)

Description

[Nicolas Silva [:nical]]

We rasterize a few glyphs almost every frame even when it doesn't look like text is changing on the page.

For example while watching a youtube video I see 4 glyphs rasterized every frame.

Comment 1

[Nicolas Silva [:nical]]

It looks like we can easily get into a loop of requesting a few glyphs which error during rasterization for some reason, and re-request them every frame.

I added some log and I am getting this continuously without having to go further than the tiles from about:newtab.

!! Error for key GlyphKey(3)
!! Error for key GlyphKey(536870915)
!! Error for key GlyphKey(268435459)
>> rasterize glyphs [GlyphKey(3), GlyphKey(536870915)]
>> rasterize glyphs [GlyphKey(268435459)]
!! Error for key GlyphKey(3)
!! Error for key GlyphKey(536870915)
!! Error for key GlyphKey(268435459)
>> rasterize glyphs [GlyphKey(3), GlyphKey(536870915)]
>> rasterize glyphs [GlyphKey(268435459)]
!! Error for key GlyphKey(3)
!! Error for key GlyphKey(536870915)
!! Error for key GlyphKey(268435459)
>> rasterize glyphs [GlyphKey(3), GlyphKey(536870915)]
>> rasterize glyphs [GlyphKey(268435459)]
!! Error for key GlyphKey(268435459)
!! Error for key GlyphKey(3)
!! Error for key GlyphKey(536870915)
>> rasterize glyphs [GlyphKey(3), GlyphKey(536870915)]
>> rasterize glyphs [GlyphKey(268435459)]
``

What's particularly bad about this is that it causes us to block on rayon for nothing which takes a millisecond or more from our precious frame budget.

Comment 2

[Nicolas Silva [:nical]]

The problem partly has to do with how eagerly we evict glyph cache entries with no size (which is what happens for errored glyphs). We should only evict them when they haven't been requested for a number of frame to avoid continuously requesting them each frame.

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 1595768 - Keep empty items in the texture cache.

Comment 4

[Nicolas Silva [:nical]]

Attachment: Bug 1595768 - Keep empty items in the texture cache.

https://phabricator.services.mozilla.com/D56938

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0112c670bd65
Keep empty items in the texture cache. r=gw

Comment 6

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/0112c670bd65

Comment 7

[Connor Brewster [:cbrewster]]

Hey Nical! I was helping someone fix a bug in Servo where Servo was uploading a 0x0 image to WR which causes a panic due to a an invalid cache texture lookup. (https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/texture_cache.rs#164, https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/renderer.rs#3648)

This happens because empty entries are given an invalid texture cache id. I ran into this patch, and was puzzled that Firefox isn't also hitting this same cache texture lookup panic in the case of invalid glyphs, but I think its because this wasn't entirely fixed.

Gecko doesn't create texture cache entries for invalid/zero-sized glyphs: https://searchfox.org/mozilla-central/rev/3a9dbc403f33cca1ff43326d78340add7d996350/gfx/wr/webrender/src/glyph_rasterizer/mod.rs#213-216

It looks like an earlier revision you made invalid/zero-sized glyphs create a texture cache entry, but that bit was removed in the final revision (earlier revision: https://phabricator.services.mozilla.com/D56938).

I could be missing something, but from a bit of testing it looks like invalid glyphs are still getting a rasterization request every frame.

Comment 8

[Nicolas Silva [:nical]]

Indeed that's very odd.

Comment 9

[Nicolas Silva [:nical]]

Attachment: Bug 1595768 - Don't evict blank glyphs to avoid re-rasterizing them continuously. r=gw

Depends on D68856

Comment 10

[Nicolas Silva [:nical]]

Attachment: Bug 1595768 - Don't track empty items in the texture cache. r=gw

This was added in a previous patch but it seems that we don't need it if invalid glyphs can be treated as blank glyph cache items.

Depends on D68857

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f777b2dc1223
Don't evict blank glyphs to avoid re-rasterizing them continuously. r=gw
https://hg.mozilla.org/integration/autoland/rev/e3f5c752e57f
Don't track empty items in the texture cache. r=gw

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/f777b2dc1223
https://hg.mozilla.org/mozilla-central/rev/e3f5c752e57f