1603637 - Lots of glyph invalidation on https://lwn.net/ml/oss-security/[e-mail]/ during pinch to zoom

Status: RESOLVED DUPLICATE of bug 1661135

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

Description

[Jeff Muizelaar [:jrmuizel]]

STR: turn on gfx.webrender.debug.glyph-flashing and pinch to zoom on https://lwn.net/ml/oss-security/[e-mail]/.

This side bar seems to rerasterize it's glyphs every paint.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jbonisteel, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Jamie Nicol [:jnicol]]

Attachment: testcase.html

Here's a reduced test case

Comment 3

[Jamie Nicol [:jnicol]]

I unfortunately cannot print the clip scroll tree easily on android (because our tree printer uses stderr, which doesn't show up in logcat. Very sad.)

But from logging I can see that SpatialNode 4 is the one being pinch-zoomed, and 5 and 6 are its children. The text run is attached to SpatialNode 6, but Picture::get_raster_space() is only called for self.spatial_node_index == 0. So it returns self.requested_raster_space which means we invalidate the glyphs every frame. (Rather than rounding the raster space which is what we want to be doing to avoid the invalidation.)

Glenn, is this the expected behaviour for fixed-position items? That their Picture is attached to SpatialNode 0? If so, can we tell that it's being zoomed by another spatial node, so that get_raster_space() can make the correct decision?

Comment 4

[Glenn Watson [:gw]]

Yea, it's expected that fixed position pictures are attached to the root spatial node.

I haven't looked into this case specifically, but it might be possible to detect / store both the scroll root (which it currently does) and also the root spatial node of the picture, and use this for zoom detection?

Another option might be to make the concept of being "in zoom phase" a global flag, rather than per-scroll-node? I recall when we originally implemented this, we decided it had to be per-scroll-node, but I can't quite recall the reasoning for that now. It might be worth re-considering if that's still needed, since fixing this particular problem would be much simpler if zooming was a global properly on the clip-scroll tree or similar?

Comment 5

[Jamie Nicol [:jnicol]]

This was basically the same problem as bug 1661135 (we were using the wrong Spatial Node to detect zooming). Closing this as a dup.