1710778 - Testcase is much more slower with animated text and faster if animated text is removed

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Mayank Bansal]

Attachment: Fast-No Text.html

Run both testcases individually.

The Fast with no text version is quite smooth, taking 8-10% CPU.
The slower testcase with text is slower, and uses a lot of CPU 240%-300% .

Profiles :
Fast testcase: https://share.firefox.dev/3y3MAbn
Slow testcase :https://share.firefox.dev/3tJzoVQ

If the perf difference is expected, feel free to INVALID.

Comment 1

[Mayank Bansal]

Attachment: Slow- With Text.html

Comment 2

[Mayank Bansal]

Original demo is from https://codepen.io/amit_sheen/pen/JjEQywZ

Comment 3

[Sotaro Ikeda [:sotaro]]

When I run Slow- With Text.html on my PC, WRWorker threads were very busy for rendering glyph.

https://share.firefox.dev/2Qiu8uP

Comment 4

[Sotaro Ikeda [:sotaro]]

:gw, :lsalzman, can you comment to the bug?

Comment 5

[Glenn Watson [:gw]]

We have some code in WR that avoids excessive glyph rasterization during pinch-zoom, which sounds like what we want to do here as well.

Jamie, do you think it'd be reasonable to extend this to apply the same logic for text runs that are animated? We could probably apply similar logic in the spatial tree, adding a method or flags to detect if a spatial node is attached to a node that has an animated property binding?

Comment 6

[Jamie Nicol [:jnicol]]

Yeah I think that does sound reasonable, in fact I thought we already did that: https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/layout/painting/nsDisplayList.cpp#8086

It would be interesting to know if that flag is being set. If it is not, then why not? And if it is, then why is it not resulting in a local raster space?

Comment 7

[Glenn Watson [:gw]]

I investigated this a bit, and it's definitely that glyphs are being re-rasterized. However, even when I change the code to reuse the quantizing logic applied when pinch-zoom is active, lots of glyphs are still being rasterized.

I need to investigate further - it looks like the quantizing breaks something to do with the caching logic in update_font_instance.

Comment 8

[Mayank Bansal]

Will this be addressed by bug 1719238 ?

Comment 9

[Glenn Watson [:gw]]

No, that patch only handles when a pinch-zoom is active. However, the fix for this will be somewhat similar.

Comment 10

[Glenn Watson [:gw]]

I think that this is working correctly now.

The profiler overlay shows 0 glyph rasterizations, and the WR glyph worker threads look idle on a firefox profile.

Mayank, could you check if it seems to working as expected on your environment?

Comment 11

[Mayank Bansal]

Slow testcase: https://share.firefox.dev/3vk1pH4
Fast testcase: https://share.firefox.dev/3PRJ7Vq

Visually, both the slow and fast versions appear the same.

Comment 12

[Glenn Watson [:gw]]

Great, thanks for confirming. Most likely fixed as part of the work in #1749380

Comment 13

[Mayank Bansal]

heres the regression range I came up with :

2022-07-27T08:40:13.178000: DEBUG : Found commit message:
Bug 1730710 - Propagate raster spaces in GPU process r=gw

This actually does two things. One: it lets child stacking contexts inherit out
of process raster spaces by checking the parent stacking context's raster space
inside WR itself, and Two: it mirrors the logic in get_raster_space_for_prim
which undoes the device_pixel_scale for pinch zooms in order to get a
consistent glyph rasterization scale. This second thing won't affect the test
page linked in the bug, but it will affect the performance of, say, animating a
whole browser's scale, which is useful for reasons.

Differential Revision: https://phabricator.services.mozilla.com/D125562

2022-07-27T08:40:13.178000: DEBUG : Did not find a branch, checking all integration branches
2022-07-27T08:40:13.180000: INFO : The bisection is done.
2022-07-27T08:40:13.181000: INFO : Stopped