Resolve conflicts between position:sticky and the ASR model
Categories
(Core :: Web Painting, task, P2)
Tracking
()
People
(Reporter: mstange, Assigned: botond)
References
(Blocks 3 open bugs)
Details
Attachments
(19 files, 1 obsolete file)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
Bug 1730749 - Create ASRs for sticky frames in BuildDisplayListForStackingContext. r=mstange,tnikkel
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
We keep having trouble with position:sticky and ASR-related assertions. That's because, at the moment, position:sticky doesn't really play well with the ASR model and sort of only works by accident - during the ASR work I eventually ran out of steam and then hacked position:sticky into it just enough to pass the tests.
I think I figured out how to make the two play well together.
The biggest trouble at the moment is that there's no natural choice for the ASR we put on the position:sticky container display item, especially if the position:sticky frame contains fixed descendants - does the sticky item scroll with the "scrolled ASR" or with the "fixed ASR"? It kind of transitions between both. Having a "sticky ASR" would let us set that ASR on the sticky container item, and on the items inside it (but not any fixed descendants). Then the next question.is where that sticky ASR should be located in the ASR tree. Sticky elements can change between being scrolled and fixed, so you'd think the sticky ASR node could be between the fixed ASR and the scrolled ASR. But that would mean having to insert an ASR node above the scrolled ASR once we detect sticky items inside the scroll frame. So instead, I think we can make the sticky ASR a child of the scrolled ASR:
- [A} Outer ASR
- [B] Scrolled ASR
- [C] Sticky ASR
- [B] Scrolled ASR
Then the sticky ASR is modeled like a nested scroll frame. While the sticky item is in the scrolled state, C acts like a scroll frame that's not being scrolled, so the sticky item just moves with B. But whenever the sticky item is in the "fixed" state, conceptually, C "reverses" any scrolling of B so that the sticky items stay fixed within A.
The actual sticky positioning calculation can stay the way it is now - there is no need to model a scroll position of C.
Having C be a child of B is valid because C has finite bounds with respect to B: A sticky element's position with respect to the scrolled ASR is always constrained by its parent element, so in this example, the sticky element always has finite bounds with respect to B (even during its fixed state). This allows us, for example, to compute valid bounds for the opacity item in this example:
<div class="scrollFrame">
<div class="opacity">
<div class="sticky"></div>
</div>
</div>
The opacity item gets ASR B, the sticky items get ASR C. The opacity item's bounds are with respect to ASR B; they're the bounds of the sticky contents +- the "sticky adjustment range" given by the size of the .opacity element. (Calculating these bounds might be a bit challenging but should be doable.)
And in this example:
<div class="scrollFrame">
<div class="opacity">
<div class="sticky">
<div class="fixed"></div>
<div class="contentsOfSticky"></div>
</div>
</div>
</div>
The ASR of the opacity item and of the fixed container item are ASR A, and the ASR for the contentsOfSticky items are the sticky ASR C. We don't have to calculate a container ASR for the sticky container item because there is no sticky container item any more.
So then the steps for this bug are:
- Make sticky elements create ASRs for themselves.
- Maybe rename ASRs to spatial nodes at this point.
Edit 2024-03-31: I've deleted the part about removing the sticky container item. I think having a sticky container item is fine and I see no advantages to be gained from removing it.
Updated•3 years ago
|
| Assignee | ||
Comment 1•1 year ago
|
||
Wanted to share an update for anyone who might be keeping an eye on this bug:
Starting around June, Dan and I have started looking at this refactoring, with close guidance from Markus. We've been working on it as a secondary project in parallel with more pressing primary projects, but we've been making steady progress, and plan to continue doing so in the coming months. We'll share further updates (including a draft patch once we have one worth sharing) as things progress.
One alteration we're looking to make to the design outlined in comment 0 so far, is that we're looking to keep nsDisplayStickyPosition for the time being (leaving its removal for a future, no-functional-changes follow-up refactoring).
Comment 2•1 year ago
|
||
Thanks @botond! We're following this very closely, currently we have exclusions on FF for some effects that require this composition to work properly.
Comment 3•1 year ago
|
||
Just want to point out that we managed to fix this issue in one case by slapping a will-change: transform on a wrapper element that's under the clipping element and parent of the sticky element: https://jsbin.com/wonopujena/edit?css,output
| Assignee | ||
Comment 4•10 months ago
|
||
| Assignee | ||
Comment 5•10 months ago
|
||
Dan and I have been continuing to work on this refactoring as a secondary project, making a bit of progress each week. I've uploaded a WIP patch of our changes so far. At this stage, the patch causes a lot of test failures and is not really ready for use or testing yet; I'm mostly sharing it for potential feedback from other developers.
| Assignee | ||
Updated•9 months ago
|
| Assignee | ||
Comment 6•5 months ago
•
|
||
Sharing another update:
Over the past few months, Dan and I have been continuing to plug away at this as a secondary project, and fixed many issues in our WIP patch, greening up a lot of tests, though there is more left to be done.
We're now planning to accelerate our efforts and focus more on this project as we try to get it across the finish line.
As we'll be working on this asynchronously, we'll be posting more frequent updates to this Bugzilla ticket.
We've recently re-worked part of the patch to make it easier to support retained display lists (specifically, we no longer have sticky ASRs store a pointer to the corresponding sticky display item). I fixed a few issues in the revised implementation approach, and kicked off a new CI run here; that will show us what test failures remain to be fixed.
| Assignee | ||
Comment 7•5 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #6)
We've recently re-worked part of the patch to make it easier to support retained display lists (specifically, we no longer have sticky ASRs store a pointer to the corresponding sticky display item). I fixed a few issues in the revised implementation approach, and kicked off a new CI run here; that will show us what test failures remain to be fixed.
The CI run is looking fairly good. Remaining test failures are:
- test_group_overscroll_handoff.html
- some reftest-snapshot tests
- a couple of view-transitions web platform reftests
| Assignee | ||
Comment 8•5 months ago
•
|
||
We started investigating the reftest-snapshot test failures. These all appear to have the same failure mode: there is a part of a sticky element that should be clipped out but isn't.
Here are some display list dumps for layout/reftests/position-sticky/top-6.html, one of the failing tests, with and without our patches.
Without:
SolidColor p=0x7f5e6059e4d8 f=0x7f5e605f2020() key=48 bounds(0,0,48000,60000) componentAlpha(0,0,0,0) clip() asr() clipChain() uniform (opaque 0,0,48000,60000) reuse-state(None) (rgba 255,255,255,255)
SolidColor p=0x7f5e6059e020 f=0x7f5e605f20d0() key=48 bounds(0,0,48000,60000) componentAlpha(0,0,0,0) clip() asr() clipChain() uniform (opaque 0,0,48000,60000) reuse-state(None) (rgba 255,255,255,255)
BackgroundColor p=0x7f5e6059e0d8 f=0x7f5e605f2f70( id:contain) key=6 bounds(480,-18420,47040,20400) componentAlpha(0,0,0,0) clip(480,480,47040,6000) asr() clipChain(0x7f5e6059e188 <480,480,47040,6000> [root asr]) uniform (opaque 480,-18420,47040,20400) hitTestInfo(0x0) hitTestArea(0,0,0,0) reuse-state(None) (rgba 0.501961,0.501961,0.501961,1) backgroundRect(x=480, y=-18420, w=47040, h=20400)
Border p=0x7f5e6059e1e0 f=0x7f5e605f2f70( id:contain) key=9 bounds(480,-18420,47040,20400) componentAlpha(0,0,0,0) clip(480,480,47040,6000) asr() clipChain(0x7f5e6059e188 <480,480,47040,6000> [root asr]) reuse-state(None)
StickyPosition p=0x7f5e6059e348 f=0x7f5e605f3030( id:sticky) key=51 bounds(480,480,47040,300) componentAlpha(0,0,0,0) clip() asr() clipChain(0x7f5e6059e188 <480,480,47040,6000> [root asr]) (opaque 480,180,47040,600) reuse-state(None) (flags 0x0) (scrolltarget 0)
BackgroundColor p=0x7f5e6059e288 f=0x7f5e605f3030( id:sticky) key=6 bounds(480,180,47040,600) componentAlpha(0,0,0,0) clip(480,480,47040,6000) asr() clipChain(0x7f5e6059e188 <480,480,47040,6000> [root asr]) uniform (opaque 480,180,47040,600) hitTestInfo(0x0) hitTestArea(0,0,0,0) reuse-state(None) (rgba 0,0,0,1) backgroundRect(x=480, y=180, w=47040, h=600)
With:
SolidColor p=0x7ff533cf3550 f=0x7ff533c7e020(Viewport(-1)) key=48 bounds(0,0,48000,60000) componentAlpha(0,0,0,0) clip() asr(null) clipChain() uniform (opaque 0,0,48000,60000) reuse-state(None) (rgba 255,255,255,255)
SolidColor p=0x7ff533cf3470 f=0x7ff533c7e0d8(Canvas(html)(-1)) key=48 bounds(0,0,48000,60000) componentAlpha(0,0,0,0) clip() asr(null) clipChain() uniform (opaque 0,0,48000,60000) reuse-state(None) (rgba 255,255,255,255)
BackgroundColor p=0x7ff533cf3020 f=0x7ff533c7efe0(Block(div id=contain)(3) id:contain) key=6 bounds(480,-18420,47040,20400) componentAlpha(0,0,0,0) clip(480,480,47040,6000) asr(null) clipChain(0x7ff533cf30e8 <480,480,47040,6000> [root asr]) uniform (opaque 480,-18420,47040,20400) hitTestInfo(0x0) hitTestArea(0,0,0,0) reuse-state(None) (rgba 0.501961,0.501961,0.501961,1) backgroundRect(x=480, y=-18420, w=47040, h=20400)
Border p=0x7ff533cf3140 f=0x7ff533c7efe0(Block(div id=contain)(3) id:contain) key=9 bounds(480,-18420,47040,20400) componentAlpha(0,0,0,0) clip(480,480,47040,6000) asr(null) clipChain(0x7ff533cf30e8 <480,480,47040,6000> [root asr]) reuse-state(None)
StickyPosition p=0x7ff533cf32d8 f=0x7ff533c7f0a8(Block(div id=sticky)(1) id:sticky) key=51 bounds(480,480,47040,6000) componentAlpha(0,0,0,0) clip() asr(null) clipChain(0x7ff533cf30e8 <480,480,47040,6000> [root asr]) (opaque 480,180,47040,600) reuse-state(None) (flags 0x0) (scrolltarget 0)
BackgroundColor p=0x7ff533cf3200 f=0x7ff533c7f0a8(Block(div id=sticky)(1) id:sticky) key=6 bounds(480,180,47040,600) componentAlpha(0,0,0,0) clip() asr(sticky <0x7ff533c7f0a8>) clipChain(0x7ff533cf30e8 <480,480,47040,6000> [root asr]) uniform (opaque 480,180,47040,600) hitTestInfo(0x0) hitTestArea(0,0,0,0) reuse-state(None) (rgba 0,0,0,1) backgroundRect(x=480, y=180, w=47040, h=600)
The first one was taken in a release build, so there are no frame names, but aside from that, a noticeable difference is that the BackgroundColor child of the StickyPosition item has clip(480,480,47040,6000) without our patches but clip() with our patches. I suspect this is what causes the rendering difference.
| Assignee | ||
Comment 9•5 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #8)
the
BackgroundColorchild of theStickyPositionitem hasclip(480,480,47040,6000)without our patches butclip()with our patches
That in turn is downstream of the BackgroundColor item having a different ASR.
Without our patches we have:
asr() clipChain(0x7f5e6059e188 <480,480,47040,6000> [root asr])
With our patches:
asr(sticky <0x7ff533c7f0a8>) clipChain(0x7ff533cf30e8 <480,480,47040,6000> [root asr])
nsDisplayItem::GetClip() returns "the clip in the item's clip chain that's associated with thte item's ASR". Without our patches, the item has a null ASR, so the <480,480,47040,6000> in the clip chain matches; with our patches, the item has a sticky ASR so it no longer matches.
| Assignee | ||
Comment 10•5 months ago
•
|
||
The clip in question is the scroll port clip of an overflow: hidden subframe containing the sticky element. This subframe is not active (it does not get an ASR), so its scroll port clip gets associated with the parent ASR, which in this case is the root.
So, I think the issue (or, at least, an issue) here is that we give the sticky item an ASR when its enclosing scroll frame does not have one. I think we should maybe only give ASRs to sticky items which are "active", in the sense that the scroll frame with respect to which they are sticky is active (gets an ASR)?
| Assignee | ||
Comment 11•5 months ago
|
||
Discussed this with Markus.
(In reply to Botond Ballo [:botond] from comment #10)
So, I think the issue (or, at least, an issue) here is that we give the sticky item an ASR when its enclosing scroll frame does not have one. I think we should maybe only give ASRs to sticky items which are "active", in the sense that the scroll frame with respect to which they are sticky is active (gets an ASR)?
Markus confirmed that we should only give ASRs to "active" sticky items.
However, it's not entirely clear that doing so would be a correctness problem (rather than just doing needless work from a performance point of view).
The fact that this is breaking rendering suggests that there may be a latent bug in the draw-snapshot codepath. Specifically, nsDisplayList::Paint should probably be applying all the clips in an item's clip chain, not just the item's GetClip().
So, next steps here:
- Only create ASRs for "active" sticky items (ones for which shouldFlatten=false).
- Update
nsDisplayList::Paintto apply all clips in an item's clip chain.
If (1) is sufficient to fix all reftest-snapshot failures, (2) can be left as a follow-up improvement.
| Assignee | ||
Comment 12•5 months ago
•
|
||
(In reply to Botond Ballo [:botond] from comment #11)
So, next steps here:
- Only create ASRs for "active" sticky items (ones for which shouldFlatten=false).
We've implemented this, and it does cause all reftest-snapshot tests in the position-sticky directory to pass locally. New CI run to see what remains to be fixed here.
| Assignee | ||
Comment 13•5 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #12)
New CI run to see what remains to be fixed here.
Here is a newer push with the patch series rebased onto a more recent baseline. Relative to the state of things in comment 7, we're not only passing reftest-snapshot tests, but the view-transitions web platform reftest failures are gone as well (thanks, I'm guessing to work on view transitions that has happened in parallel)!
So, it seems only the failure of test_group_overscroll_handoff.html remains to be fixed.
| Assignee | ||
Comment 14•5 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #13)
So, it seems only the failure of
test_group_overscroll_handoff.htmlremains to be fixed.
I've spent some time trying to debug this failure, and it's a very strange one.
The test hangs early during the execution of helper_position_sticky_scroll_handoff.html, during the waitUntilApzStable call.
The hang happens consistently, but the place where it occurs changes intermittently between two places. The first is during promiseAllPaintsDone, where the code in paint_listener.js waits for a MozAfterPaint event that never comes. I guess we are somehow breaking the paint listener mechanism / its platform implementation on the affected page, which is somewhat surprising, though it's the less surprising of the two hangs.
The more surprising hang is in promiseFocus, more specifically in this executeSoon call. It seems the callback is just never called. I tried tracing the implementation but got very quickly into the weeds of EventQueue / TaskController stuff I don't understand well.
My next step is probably to debug the first hang (MozAfterPaint never arrives) further, as it seems more tractable, though if anyone has suggestions for how to debug "executeSoon never calls its callback", I would be curious to hear it.
| Assignee | ||
Comment 15•4 months ago
•
|
||
It turns out the hangs discussed in the previous comment have a less mysterious cause than it seemed: the first time we try to paint helper_position_sticky_scroll_handoff.html, we get into an infinite loop during display list building. (I haven't delved into the details yet, but we somehow end up with an ASR which is set as its own parent, and then this loop iterates indefinitely.)
Sometimes the first paint of the page is already scheduled by the time promiseFocus gets to the executeSoon call, other times it only gets scheduled afterwards, hence the two different manifestations of the hang.
| Assignee | ||
Comment 16•4 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #15)
the first time we try to paint
helper_position_sticky_scroll_handoff.html, we get into an infinite loop during display list building. [...] we somehow end up with an ASR which is set as its own parent, and then this loop iterates indefinitely.)
The issue is that this page contains an element which is position: sticky and overflow: auto; this results in a frame which is both a sticky frame, and a scroll frame. Since we now create ASRs for both frame types, this frame has two associated ASRs. However, we cache ASRs on frames in a frame property, and so the two ASRs get mixed up. At some point, we end up querying what we expect to be the scroll ASR, but get back the sticky ASR because that has been written into the frame property more recently.
| Assignee | ||
Comment 17•4 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #16)
The issue is that this page contains an element which is
position: stickyandoverflow: auto; this results in a frame which is both a sticky frame, and a scroll frame. Since we now create ASRs for both frame types, this frame has two associated ASRs. However, we cache ASRs on frames in a frame property, and so the two ASRs get mixed up. At some point, we end up querying what we expect to be the scroll ASR, but get back the sticky ASR because that has been written into the frame property more recently.
Fixed this by using a second frame property to store sticky ASRs.
New CI run here. We are now passing test_group_overscroll_handoff.html on most platforms, but it's still failing on Linux with fission disabled. However, this is a different failure: it's the helper_position_fixed_scroll_handoff-5.html subtest that appears to be hanging.
Comment 18•4 months ago
|
||
(In reply to Botond Ballo [:botond] from comment #17)
New CI run here. We are now passing
test_group_overscroll_handoff.htmlon most platforms, but it's still failing on Linux with fission disabled. However, this is a different failure: it's thehelper_position_fixed_scroll_handoff-5.htmlsubtest that appears to be hanging.
That's likely bug 1966348 and so will be fixed by updating to tip.
| Assignee | ||
Comment 19•4 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #18)
(In reply to Botond Ballo [:botond] from comment #17)
New CI run here. We are now passing
test_group_overscroll_handoff.htmlon most platforms, but it's still failing on Linux with fission disabled. However, this is a different failure: it's thehelper_position_fixed_scroll_handoff-5.htmlsubtest that appears to be hanging.That's likely bug 1966348 and so will be fixed by updating to tip.
Indeed, that failure was unrelated to these patches. With the fix for that included, we now have a green CI run!
| Assignee | ||
Comment 20•4 months ago
|
||
No one creates ASRs of this kind yet (that will happen in a later patch).
Consumer code is adjusted to check for the ASR kind as appropriate.
To make this easier, two helper functions GetNearestScrollASR()
and GetNearestScrollASRViewId() are added.
Updated•4 months ago
|
| Assignee | ||
Comment 21•4 months ago
|
||
| Assignee | ||
Comment 22•4 months ago
|
||
| Assignee | ||
Comment 23•4 months ago
|
||
The implementation only needs the display item's frame and its
"per frame key". In some cases these inputs can be known before
the display item instance itself is available. A later patch in
this series makes use of this.
| Assignee | ||
Comment 24•4 months ago
|
||
This paves the way for moving the code that creates the animation id
to earlier during WebRender display list building, where we won't
have the sticky item available to store the animation id on it.
| Assignee | ||
Comment 25•4 months ago
|
||
This allows the code that checks the flag to avoid needing the
sticky display item.
| Assignee | ||
Comment 26•4 months ago
|
||
| Assignee | ||
Comment 27•4 months ago
|
||
A frame can be both a scroll frame and sticky, in which case it has
two associated ASRs. Using distinct frame properties allows retaining
information about both.
| Assignee | ||
Comment 28•4 months ago
|
||
This is needed because the sticky item's own ASR may not be the sticky ASR,
but the item needs to know the sticky ASR to look up the corresponding
spatial id in CreateWebRenderCommands.
| Assignee | ||
Comment 29•4 months ago
|
||
The current implementation relies of mSpaceAndClipChain, but that assumes
WebRender display list building has actually entered the sticky display
item.
A later patch will move this call to earlier during WebRender display
list building (ClipManager), where we'll need to provide the parent
spatial id explicitly.
| Assignee | ||
Comment 30•4 months ago
|
||
This is conceptually similar to the existing mapping of ViewIDs to spatial ids
for scroll ASRs, and the two could be unified in the future.
The patch also generalizes ClipManager::GetScrollLayer into GetSpatialId.
In nsDisplayStickyPosition::CreateWebRenderCommands, we use the new mapping,
in preparation for moving the DefineStickyFrame call out of there.
| Assignee | ||
Comment 31•4 months ago
|
||
This patch generalizes ClipManager::DefineScrollLayers into
DefineSpatialNodes, and introduces ClipManager::DefineStickyNode to
handle creating a spatial node for a sticky ASR.
For ease of review, this patch temporarily assumes that we can get hold of
the sticky display item in ClipManager, and contains a stub function for
doing so. That allows the initial implementation of DefineStickyNode to
mostly be cut-and-paste from nsDisplayStickyPosition::CreateWebRenderCommands.
The next patch will revise the implementation of DefineStickyNode to
avoid the dependency on the sticky display item.
| Assignee | ||
Comment 32•4 months ago
|
||
| Assignee | ||
Comment 33•4 months ago
|
||
This is no longer used now that the creation of the sticky animation id
happens in ClipManager.
| Assignee | ||
Comment 34•4 months ago
|
||
The use of this override for sticky content was essentially a workaround
for not having sticky ASRs. As we now have them, we get the desired
spatial id the usual way, mapping from ASR to spatial id.
| Assignee | ||
Comment 35•4 months ago
|
||
To avoid sticky content checkerboarding when we've scrolled farther
than the displayport, we do not apply the displayport clip to it.
This was previously implemented in a hacky way in ClipManager, but
the introduction of sticky ASRs complicates that implementation.
Instead, this patch implements the omission of displayport clips
for sticky content in the Gecko display list itself.
| Assignee | ||
Comment 36•4 months ago
|
||
Updated•4 months ago
|
| Assignee | ||
Comment 37•4 months ago
|
||
Note, the patches implementing this refactoring were coauthored by Dan Robertson; I'm not sure what's the best way to reflect that in the metadata (the original commit authorship information on the patches did not survive patch splitting / combination and moz-phab).
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•3 months ago
|
Updated•2 months ago
|
| Assignee | ||
Comment 38•1 month ago
|
||
We need to retain sticky ASRs for:
- looking up the ASR from the frame
- sharing the ASR between continuations of a sticky element
We need to retain scroll ASRs for consistency (if we retain
sticky ASRs but not scroll ASRs, the retained sticky ASRs will
point to stale scroll ASR parents).
| Assignee | ||
Comment 39•1 month ago
|
||
This is needed to avoid upsetting the assumptions made by the code to
track container ASRs. Without this, layout/base/crashtests/1464641.html
triggers the assertion in the PickAncestor() call in
SetCurrentActiveScrolledRoot().
| Assignee | ||
Comment 40•1 month ago
•
|
||
I posted an updated version of the patch series. Here is a summary of the updates:
- [New patch D264156]: ASRs are now always stored in a frame property, not just when retained-DL is enabled. This is needed in part due to the next bullet point, but also for the existing "Introduce ActiveScrolledRoot::GetStickyASRForFrame()" patch to work as intended.
- [New patch D264157]: If a sticky element creates multiple continuation frames, they now share a sticky ASR. More details in the commit message.
- [Changes to patch D254147]: Sticky ASRs are now created earlier in
BuildDisplayListForStackingContext, such that the they're already installed on the display list builder by the time thecontainerASRTrackeris created. - [Change to patch D254157]: The conditions under which the 'finite clip' assertion is relaxed were narrowed.
| Assignee | ||
Comment 41•1 month ago
|
||
(The latest update to the patch series is just a rebase.)
Description
•