1366295 - [meta] Add support for position:sticky in webrender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

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

Right now webrender doesn't seem to support position:sticky. We'll need to add support for it. Most of the work will be in upstream webrender, which I'll file an issue for. However there will be a bit of integration work as well in bindings.rs or the WR layer tree, and we can use this bug for that.

Comment 1

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

servo/webrender#1277 is the WR issue.

Comment 2

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

Adding a link to a page with an example of some position:sticky content.

Comment 3

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

Upstream WR now has position:sticky support implemented, so I'm going to pick this up and see if I can write the necessary glue on the gecko side.

Comment 4

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

Braindump since I'm going on PTO next week:

- Current WIP patch queue consists of the top two patches at [1]. First patch just exposes the sticky API and second patch tries to implement nsDisplayStickyPosition::CreateWebRenderCommands
- The problem I'm running into now is something to do with the clip chains not lining up. We have a gecko DL that looks like this:

...
StickyPosition ... clip() asr(A) clipChain(<B> A, <R> [root asr])
  ...
  BackgroundColor ... clip(C) asr(A) clipChain(<C> A, <R> [root asr])
...

Note that the clip chain for the StickyPosition and the BackgroundColor have the same root <R> but different leafs (<B> vs <C>). Currently, when we process the StickyPosition item, we push <B> onto the WR clip stack and recurse into the nested list. Then, when we process the BackgroundColor item, we need to ensure that <C> is defined with only <R> on the clip stack. So we push <R> and define/push <C>, push the background color rect, and then pop <C> and pop <R>. This is all fine.

However, because sticky positioning is implemented as a push/pop of a clip id in WR, what happens with my patches is this: As we process the StickyPosition, we push <B> onto the clip stack, and then define/push a sticky frame <S>, before recursing into the nested list. As before, we then handle the BackgroundColor and go to push <R> and define/push <C>, but pushing <R> now means that <S> is "hidden". That is, the definition of <C> is done with an effective clip stack that doesn't have <S>, and so all the things inside <C> (the background color rect) are effectively outside the sticky frame. So that's not great.

        R                             R
       /|\      instead of           / \
      B S C                         B   S
                                        |
                                        C

I think one way of looking at the problem is that WR treats sticky frames similar to the way it treats clips, but in gecko the clip chain doesn't have anything for the sticky frame. I'm not sure if this is the best way to look at the problem, because the solution here would be to insert something into the gecko clip chain that is a placeholder for sticky frames. That might get kinda complicated. I'll think about this more when I get back.

[1] https://github.com/staktrace/gecko-dev/commits/apzfree

Comment 5

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

I'm going to split this bug into smaller bugs, since it appears to be nontrivial to get this working. I have code now that gets a very basic position:sticky element working as expected but I'll need to spend more time making sure all the clips and such work properly.

Comment 6

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

This should be all done now; the last open dependency just merged to m-c.