1384616 - Make WebRenderScrollData hold a more tree-like set of layers in layers-free transactions

Status: RESOLVED FIXED

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

Description

[Kartikaya Gupta (email:[e-mail])]

Building on bug 1384181, the next step is to add support for things like nsDisplayTransform and nsDisplayFixedPosition. What these have in common is that the display items are wrappers for other display items, forming a tree-like structure. i.e. nsDisplayFixedPosition wraps a bunch of display items that all comprise the fixed-position item.

Right now the layers-free WebRenderScrollData doesn't handle this, it just builds a flat list of WebRenderLayerScrollData items (with a synthetic root). However we need make this more tree-like so that APZ knows which items are subject to a transform or are fixed-position and so on.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d9f7f23ec98adc4cd5b84c1eeab56d8e1395c57

Comment 2

[Kartikaya Gupta (email:[e-mail])]

The patches I have successfully make the structure more tree-like and so nsDisplayFixedPosition and scrollbar items can be handled. However nsDisplayTransform is a little more tricky because of perspective items so I'll punt that to the next bug. Also scrollbar animation ids need to be hooked up still, I'll punt that to another bug as well.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1384616 - Fix indentation.

Review commit: https://reviewboard.mozilla.org/r/162246/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162246/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1384616 - Change how display items put their APZ-relevant info into the WebRenderScrollData.

Instead of the WebRenderLayerScrollData code knowing about all the
different display item types, it makes more sense to move this logic
into the display items.

In addition to avoiding dis-encapsulating the data from nsDisplayItem subclasses,
this makes it easier to handle two specific scenarios:
(1) the case where an nsDisplayItem A subclasses another nsDisplayItem B, but A
and B have different types returned by GetType(). Previously A and B would have
to be handled explicitly in the WebRenderLayerScrollData switch statements,
which doesn't scale well if new types are added. With the new approach the
virtual function is shared down from A to B and so takes care of it. This is
particularly relevant for types like nsDisplayOwnLayer which have a number of
subclasses.

(2) the case where a display item *might* have APZ-relevant information.
In this case the type of the item alone is not sufficient to determine
if we need to create a new WebRenderLayerScrollData for it. Instead, we
need to access specific state inside the display item. This is now
handled by the UpdateScrollData function returning true when passed
nullptr arguments, and replaces the switch statement in
WebRenderLayerManager that updated forceNewLayerData.

Review commit: https://reviewboard.mozilla.org/r/162248/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162248/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1384616 - Allow building a tree structure in WebRenderScrollData for layers-free transactions.

The three main changes in this patch are:
(1) Move the code to emplace_back a new WebRenderLayerScrollData to
after the display list recursion. This is necessary so that when we
empty the temporary mLayerScrollData stack into the final mScrollData
structure things end up in the right order.
(2) Maintain a stack of ASRs as we recurse so that when we are building
a given WebRenderLayerScrollData, we don't add scroll metadatas that
are already present on its ancestors.
(3) Compute the number of descendants created for each layer scroll
data item and record it, so that we can properly reconstruct the
shape of the tree.

Review commit: https://reviewboard.mozilla.org/r/162250/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162250/

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1384616 - Save APZ-relevant data from fixed-pos items in the WR scroll data for layers-free transactions.

Review commit: https://reviewboard.mozilla.org/r/162252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162252/

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1384616 - Save APZ-relevant scrollbar data into the WR scroll data for layers-free transactions.

Review commit: https://reviewboard.mozilla.org/r/162254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162254/

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8891065 [details]
Bug 1384616 - Fix indentation.

https://reviewboard.mozilla.org/r/162246/#review168320

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8891066 [details]
Bug 1384616 - Change how display items put their APZ-relevant info into the WebRenderScrollData.

https://reviewboard.mozilla.org/r/162248/#review168384

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8891067 [details]
Bug 1384616 - Allow building a tree structure in WebRenderScrollData for layers-free transactions.

https://reviewboard.mozilla.org/r/162250/#review168386

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8891068 [details]
Bug 1384616 - Save APZ-relevant data from fixed-pos items in the WR scroll data for layers-free transactions.

https://reviewboard.mozilla.org/r/162252/#review168388

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8891069 [details]
Bug 1384616 - Save APZ-relevant scrollbar data into the WR scroll data for layers-free transactions.

https://reviewboard.mozilla.org/r/162254/#review168390

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8891065 [details]
Bug 1384616 - Fix indentation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162246/diff/1-2/

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8891066 [details]
Bug 1384616 - Change how display items put their APZ-relevant info into the WebRenderScrollData.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162248/diff/1-2/

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8891067 [details]
Bug 1384616 - Allow building a tree structure in WebRenderScrollData for layers-free transactions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162250/diff/1-2/

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8891068 [details]
Bug 1384616 - Save APZ-relevant data from fixed-pos items in the WR scroll data for layers-free transactions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162252/diff/1-2/

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8891069 [details]
Bug 1384616 - Save APZ-relevant scrollbar data into the WR scroll data for layers-free transactions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162254/diff/1-2/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19ee1751ecc3
Fix indentation. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/509e1cc89fb5
Change how display items put their APZ-relevant info into the WebRenderScrollData. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/9e863293355e
Allow building a tree structure in WebRenderScrollData for layers-free transactions. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/279250114444
Save APZ-relevant data from fixed-pos items in the WR scroll data for layers-free transactions. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/236acd7a61fc
Save APZ-relevant scrollbar data into the WR scroll data for layers-free transactions. r=jrmuizel

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/19ee1751ecc3
https://hg.mozilla.org/mozilla-central/rev/509e1cc89fb5
https://hg.mozilla.org/mozilla-central/rev/9e863293355e
https://hg.mozilla.org/mozilla-central/rev/279250114444
https://hg.mozilla.org/mozilla-central/rev/236acd7a61fc

Comment 20

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> Also scrollbar animation ids need to be
> hooked up still, I'll punt that to another bug as well.

This is in bug 1386747.