1923392 - [META] [css-anchor-position-1] WebRender work required for async scrolling of anchor query elements

Status: NEW

(Core :: Graphics: WebRender, enhancement)

Description

[David Shin[:dshin]]

Attachment: Scrolling example.

... To follow the spec and enable smooth scrolling.
This is expected to behave somewhat similarly to sticky elements.

Comment 1

[David Shin[:dshin]]

Backgrounder for sticky elements (Thanks Botond!), which is probably relevant:

Layout does reposition sticky elements itself after scrolling, but we have a scrolling fast path that allows compositing frames that reflect a new scroll offset without waiting for Layout (or any part of the content process rendering pipeline).
So the compositor (WebRender / APZ) support for sticky positioning has to do with making an equivalent adjustment to what Layout would make, in the compositor, so that the positions of sticky elements rendered during that fast path are correct.

Code locations of interest:

1. During display list building, information about sticky elements that WebRender will need (not just the current position, but information needed to adjust the position in response to scrolling) is sent to WebRender, around here.
2. During the scrolling fast-path, APZ tells WebRender about changes to the scroll offset. (This is probably less interesting but for completeness it happens here.)
3. During compositing, WebRender combines the latest scroll offset from APZ, with the information from the last display list build, to compute and apply an up to date position for the sticky element, around here.

Comment 2

[Timothy Nikkel (:tnikkel)]

I've been looking into how to implement this.

The adjustment that needs to be made on the compositor side (in webrender) is exactly the same as if the queried element was in the same scroll frame as the anchor element. Unlike position sticky where the adjustment needs to stop when the sticky element reaches some offset. So the information that needs to be communicated is this association of painted content (of the queried element) to the scroll frame (of the anchor element). The way that webrender stores this data is in the spatial tree, which is a tree of spatial nodes. We just need to have this painted content be in the subtree of the spatial node that corresponds to the scroll frame of the anchor element. Probably the simplest way to do that is assign the relevant display items from the queried element the active scrolled root (ASR) of the anchor content. Since queried elements are abs pos and hence out of flow, we already have a mechanism where we adjust the current ASR when traversing into OOF content, and that could be extended for this case. There are two different "levels" that we could do this ASR change at: we could change the ASR on the gecko display item, or we could leave that alone and change the scroll id that we pass to webrender after that. The first approach is probably the simplest and it makes sense to tell the gecko display list what content moves async with respect to. The second approach might be needed if there are complications. The second approach is similar to one aspect of how sticky position works, where it defines a new spatial node as a child of the ASR it wants, add an override to the ASR to spatial node map when in the sticky pos subtree. It's not as straight forward as sticky pos because sticky pos is a stacking context, but the queried element is only abs pos, so its only a pseudo stacking context, and so we can't have one containing display item for the queried content.

A secondary concern is hit testing: which scroll frame should move if the user scrolls while the mouse cursor is over the queried element? The scroll frame of the anchor element (usually scrolling over content scrolls the scroll frame that it moves with)? Or the scroll frame of the queried element (if it wasn't anchored to something that scrolls with a different scroll frame) (this is what Chrome/Safari currently do)? We can do either one, but the way we implement the async scroll positioning (above) affects how much work this is. The simpler approach outlined above would automatically give us the first behaviour. If we needed to change what the queried content hit tests to we could do something around HitTestInfoManager::ProcessItem

https://searchfox.org/mozilla-central/rev/12536fb3a3712cfded41d427bd1470ecf76e5691/gfx/layers/wr/HitTestInfoManager.cpp#93

and adjust which scrollid we put in the hit test info we pass to webrender.

Comment 3

[Botond Ballo [:botond]]

The analysis in comment 2 makes sense to me.

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

The second approach is similar to one aspect of how sticky position works, where it defines a new spatial node as a child of the ASR it wants, add an override to the ASR to spatial node map when in the sticky pos subtree.

Note that in bug 1730749, which Dan and I are actively working on, we will get rid of this override, and instead model sticky ASRs directly in the Gecko display list. This basically makes the modelling of position:sticky closer to the "first approach" than the "second approach" (and could potentially be an argument for preferring the first approach for anchor positioning as well, for consistency).

Comment 4

[David Shin[:dshin]]

So for the first approach, and the second approach (That will be more like the first approach, from what I understand), is it ok that the query element's ASR doesn't necessarily move to one of its ancestors?

To my (Limited) understanding, sticky elements is attached to its "normal" scroller until it would normally get scrolled out, at which point it's assigned an ancestor scroller to prevent it from moving. However, in the case of positioned elements, its needs to be rooted to the scroller of the anchor element, which isn't necessarily the positioned element's ancestor. In the test case attached:

• #d1: absolute-containing scroller
  • #d2: scroller
    • #d3: scroller
      • #anchor: the anchor element
  • #d4: scroller
    • #query: the query element

#query needs to scroll with #d3, which is previous sibling's descendant.

(Aside: I misnamed the query in the test case as queried element, which is a non-standards term and could be misunderstood as the anchor element. My bad on that.)

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to David Shin[:dshin] from comment #4)

So for the first approach, and the second approach (That will be more like the first approach, from what I understand), is it ok that the query element's ASR doesn't necessarily move to one of its ancestors?

I'll answer this below.

To my (Limited) understanding, sticky elements is attached to its "normal" scroller until it would normally get scrolled out, at which point it's assigned an ancestor scroller to prevent it from moving.

This is how sticky works as a concept, but when we implement it the ASR can't change due to scrolling. An ASR can only change due to something like a DOM modification. The ASR is assigned by the gecko content process main thread, and ASRs staying the same during async scrolling on the webrender/compositor side is a basic requirement of async scrolling and is how async scrolling offsets are communicated. They can be changed by generating a new display list and sending it to webrender.

So then with the ASR staying the same how does async scrolling of sticky work? We send over a bunch of bounds/coordinates to webrender so that it can compute when to scroll the sticky content and when to leave it fixed. It takes these coordinates, the initial position of the sticky content, and the current async scroll offset as input and figures out what transform to apply to the sticky content in order to achieve this effect. So when the sticky content hits the bound where it changes from scrolling to fixed the transform that is computed keeps it in the same place. As the async scroll offset increases, the transform increases to compensate.

However, in the case of positioned elements, its needs to be rooted to the scroller of the anchor element, which isn't necessarily the positioned element's ancestor. In the test case attached:

• #d1: absolute-containing scroller
  • #d2: scroller
    • #d3: scroller
      • #anchor: the anchor element
  • #d4: scroller
    • #query: the query element

#query needs to scroll with #d3, which is previous sibling's descendant.

So I'll hopefully answer your first question from above here. Without this work the ASR of the query element would be d4 (and its parent ASR would be d1). The plan I outlined would be to change it's ASR to be the ASR of d3. Yes it does seem a little bit weird that it's ASR would be a sibling and not an ancestor. That is a good observation. This is one of the reasons why I'm not sure if the plan to change the ASR at the gecko display list level is going to be the winner or we'll have to make the ASR (aka spatial node) change one level lower when we move to the webrender display list. So I've been trying to think through any potential issues this might cause until I can actually start writing code and see what issues come up.

One issue that I encountered is that currently the ASR struct would always be created before we need to reference the ASR struct somewhere else (because we would always be guaranteed to call BuildDisplayList on the scroll frame before anyone needs to reference it). And when fission is not turned on which scrollframes become ASRs (ie allow async scrolling) can depend on what is inside the scroll frame. So to get around that my idea is to treat any scrollframes (and ancestors) that are referenced for anchor position like fission is enabled, ie always turning them into ASRs if they can ever be an ASR when we look at them while following anchor references.

There are also various ASR related asserts that we have that this new wrinkle for ASRs could trip. Whether these come up and if we can find a way to deal with them is part of why I have outlined two potential approaches.

When we generate a gecko display list it needs to be valid for any async scroll offset that might happen (ie if we can async scroll an opaque element out of view we need to paint whats underneath it), and having the right ASR on elements is part of that, so that is one point in favour of having the ASR that the query content moves with as it's ASR in the gecko display list.

(Aside: I misnamed the query in the test case as queried element, which is a non-standards term and could be misunderstood as the anchor element. My bad on that.)

Ah, thanks! That'll avoid some confusion.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #5)

Without this work the ASR of the query element would be d4 (and its parent ASR would be d1).

This is a mistake. The asr of the query element would be the same asr as d1 because the query element is abs pos.

Comment 7

[Timothy Nikkel (:tnikkel)]

Working through this in more detail, another requirement of our ASRs/clips is that the ASRs in a clipchain form an ancestor relationship. If the query element is in a different top layer from the anchor element, then the ASR of the anchor element would have no ancestor relationship with the ASRs that could be in the clip chain of query element (when they are in the same top layer, the abs pos/containing block requirements of the spec I think simplifies what kinds of cases are possible). If there was then a scrollframe insider the query element we would need to add a clip chain item with the ASR of the anchor item to the query elements existing clip chain and this would not work.

I thought of a way to work around this though. Bug 1730749 introduces the idea of creating a different type of ASR and inserting it into the tree in order to solve a problem. I think we can use that idea here. Let A1 through An be the asr chain of the anchor element (An being the deepest in the tree). Let Ak be the deepest asr that is an ancestor of the asr of the query element. Then we introduce a new type of asr kind that we call "clone". We create clone asrs A(k+1) through An call them A'(k+1) through A'n, and insert them into the asr tree as you would expect beside their clones but distinct from that ancestor chain. And A'n becomes the asr of the query element. Other than having to keep track of these clones adding complexity this seems like it should work

Comment 8

[David Shin[:dshin]]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

(In reply to Timothy Nikkel (:tnikkel) from comment #5)

Without this work the ASR of the query element would be d4 (and its parent ASR would be d1).

This is a mistake. The asr of the query element would be the same asr as d1 because the query element is abs pos.

Not 100% sure if I understand this - the query element is required to scroll with the anchor element, when #d4 scrolls, right? Can we support that with #d1 being the query element's ASR?

One issue that I encountered is that currently the ASR struct would always be created before we need to reference the ASR struct somewhere else (because we would always be guaranteed to call BuildDisplayList on the scroll frame before anyone needs to reference it). And when fission is not turned on which scrollframes become ASRs (ie allow async scrolling) can depend on what is inside the scroll frame. So to get around that my idea is to treat any scrollframes (and ancestors) that are referenced for anchor position like fission is enabled, ie always turning them into ASRs if they can ever be an ASR when we look at them while following anchor references.

:tnikkel and I discussed this, and there's a potential Android perf impact to watch for - Android doesn't have Fission on by default, so once we do this, more scroll frames may become ASRs. The impact should be limited to the query element's containing scrollers, though.

Comment 9

[Timothy Nikkel (:tnikkel)]

(In reply to David Shin[:dshin] from comment #8)

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

(In reply to Timothy Nikkel (:tnikkel) from comment #5)

Without this work the ASR of the query element would be d4 (and its parent ASR would be d1).

This is a mistake. The asr of the query element would be the same asr as d1 because the query element is abs pos.

Not 100% sure if I understand this - the query element is required to scroll with the anchor element, when #d4 scrolls, right? Can we support that with #d1 being the query element's ASR?

The sentence I was correcting was only speaking about a hypothetical scenario if no CSS anchor positioning existed in the testcase just to get a baseline understanding established. Sorry I didn't quote more context to make that clear.

One issue that I encountered is that currently the ASR struct would always be created before we need to reference the ASR struct somewhere else (because we would always be guaranteed to call BuildDisplayList on the scroll frame before anyone needs to reference it). And when fission is not turned on which scrollframes become ASRs (ie allow async scrolling) can depend on what is inside the scroll frame. So to get around that my idea is to treat any scrollframes (and ancestors) that are referenced for anchor position like fission is enabled, ie always turning them into ASRs if they can ever be an ASR when we look at them while following anchor references.

:tnikkel and I discussed this, and there's a potential Android perf impact to watch for - Android doesn't have Fission on by default, so once we do this, more scroll frames may become ASRs. The impact should be limited to the query element's containing scrollers, though.

Yeah, it's a cost that we worked to minimize before we turned on fission for desktop and we've not noticed any problems with it since. And it's a cost that we will have to pay anyways when fission is turned on on android.

Comment 10

[David Shin[:dshin]]

Attachment: Example where query element scrolls with default anchor scroller in one axis

Query element may position in relation to default anchor element, or any element in default anchor's scroller, only in one axis. In this case, the query element will scroll with the default anchor's scroller only in that axis.

Comment 11

[David Shin[:dshin]]

Attachment: Example of referring to multiple anchors, some in different scrollers

Comment 12

[David Shin[:dshin]]

Attachment: Taking Scroll Into Account when Query Element Starts Displaying

This (Contrived) example:

• Shouldn't scroll with any anchor
• Should be sized from the lower right of the anchor in the upper left scroller, to the upper left of the anchor in the lower right scroller.
• When you toggle the positioned element out of existence by clicking the button, move the scrollers around, then toggle back the element through the button again, the query element should be sized based on the current scroller position. I think - Pending clarification on spec

Comment 13

[Timothy Nikkel (:tnikkel)]

We realized that the anchor spec allows the anchored content to scroll with the anchor content only in one axis, leaving the other axis still scrolling with the anchored content's original scroll frame. Implementing this is more involved because nothing like an element scrolling with two different scrollframes in each axis has existed so far. I came up with a rough plan of how we could implement that, it's still a little high level and needs more details worked out. As described above we would create clone ASRs of the chain of ASRs that scroll the anchor content and make them children of the original ASR of the anchored content. We'll need to figure out some way of getting the same async scroll updates to the spatial nodes that these clone ASRs create. We'll need to create a new spatial node type for the anchored content, and we'll need to record in this new spatial node type which spatial node ancestors it responds to in which axis. And then when we calculate the async scroll adjustment transform we will need to be able to pick which axis of the transform we use from each spatial node ancestor.

Comment 14

[Glenn Watson [:gw]]

(In reply to Timothy Nikkel (:tnikkel) from comment #13)

We realized that the anchor spec allows the anchored content to scroll with the anchor content only in one axis, leaving the other axis still scrolling with the anchored content's original scroll frame. Implementing this is more involved because nothing like an element scrolling with two different scrollframes in each axis has existed so far. I came up with a rough plan of how we could implement that, it's still a little high level and needs more details worked out. As described above we would create clone ASRs of the chain of ASRs that scroll the anchor content and make them children of the original ASR of the anchored content. We'll need to figure out some way of getting the same async scroll updates to the spatial nodes that these clone ASRs create. We'll need to create a new spatial node type for the anchored content, and we'll need to record in this new spatial node type which spatial node ancestors it responds to in which axis. And then when we calculate the async scroll adjustment transform we will need to be able to pick which axis of the transform we use from each spatial node ancestor.

This sounds like a good approach to me.

Comment 15

[Timothy Nikkel (:tnikkel)]

This is a breakdown of implementing this.

• Implement a "is compensating for scroll" function so we know when to apply async scrolling for anchor positioning (bug 1968745)
• Implement async scroll of anchored elements whose containing block is an ancestor (or equal) to the containing block of the anchor element
  • Add a way to determine if all scroll frames containing the anchor are active or not at the time of the BuildDisplayList call for the anchored element
  • Assign the ASR (active scrolled root) of the anchor to the anchored element
• Implement the ability to async scroll in each axis with different scroll frames
  • Create clone ASRs that only respond to scrolling in one axis and adopt them into the asr tree
  • Create new spatial node types to correspond to these clone ASRs that somehow records which axis to respond to
  • Hook up async scroll updates to the spatial nodes that are created by these clone ASRs
  • Calculate the total async scroll offset from these new spatial nodes for the anchored content

This plan does not handle the case "positioned el is in a higher top layer than possible anchor" of from the spec. This is because:

• Chrome does not implement it
• this spec issue is proposing to change that part of the spec
• it is a lot more work

If the spec does not change and Chrome does implement it at a future date we can at that point in time put the effort into implementing it. Even if this future comes to pass we will not have wasted any work as the above work is necessary as a stepping stone to this potential future work.

Comment 16

[David Shin[:dshin]]

Added bugs as per :tnikkel's breakout:

(In reply to Timothy Nikkel (:tnikkel) from comment #15)

• Implement async scroll of anchored elements whose containing block is an ancestor (or equal) to the containing block of the anchor element
  • Add a way to determine if all scroll frames containing the anchor are active or not at the time of the BuildDisplayList call for the anchored element (Bug 1988030)
  • Assign the ASR (active scrolled root) of the anchor to the anchored element (Bug 1988032)
• Implement the ability to async scroll in each axis with different scroll frames
  • Create clone ASRs that only respond to scrolling in one axis and adopt them into the asr tree (Bug 1988034)
  • Create new spatial node types to correspond to these clone ASRs that somehow records which axis to respond to (Bug 1988035)
  • Hook up async scroll updates to the spatial nodes that are created by these clone ASRs (Bug 1988036)
  • Calculate the total async scroll offset from these new spatial nodes for the anchored content (Bug 1988037)

:tnikkel, let me know if we need more.

Comment 17

[Timothy Nikkel (:tnikkel)]

Thanks.