-
Notifications
You must be signed in to change notification settings - Fork 706
[css-view-transitions-1] Flush the callback queue before performing other view-transition operations #11947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ther view-transition operations This ensures that a DOM update callback that skips the active transition would take effect. Closes w3c#11943
This PR makes it a bit more obvious that the initial PR (#11693) may have had a problem in that we're running script in the middle of update the rendering steps. Namely after layout but before intersection observers and paint. That seems problematic. Maybe we should instead frame it as if we need to set up capture and the queue is not empty, abort these steps. And then we can flush the queue outside of update the rendering (and schedule another animation frame) |
Good point
Maybe, instead, we should keep rendering surpression until the queue has been emptied? |
Maybe, instead, we should keep rendering surpression until the queue has
been emptied?
That's a good question. If we're going to capture the old state after the
queue is flushed, we'd presumably need at least one frame produced for
those pixels. Not sure exactly what the author or user expectations would
be here
|
OK, I think the right solution here is to flush the queue at the beginning of the rendering step (before |
1. [=list/For each=] |transition| in |document|'s [=update callback queue=], [=call the update callback=] given |transition|. | ||
|
||
1. Set |document|'s [=update callback queue=] to an empty list. | ||
</div> | ||
|
||
Note: a scheduled update callback is guaranteed to be called before the next rendering steps, regardless of whether the | ||
transition has succeeded or not. This is guaranteed by [=flush the update callback queue|flushing=] the queue in one of two situations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to understand the implications of the rendering loop one:
- Does this mean if I run a VT and skip it (essentially queuing up the DOM update callback), then this addition would mean that we guarantee to run that callback before the next update the rendering? (this is a new constraint IIUC)
- If I do queue up a transition that ends up cancelling the previous one, is there an opportunity for the visuals of the first callback to appear on screen before the capture of the second callback? It sounds like yes since the update the rendering will produce a frame here.
Another comment here is that we can have a situation where you have something like
for (let i = 0; i < 10; i++) {
document.startViewTransition(async () => { await new Promise(() => {}) });
}
which can essentially hang the page for 40 seconds (4sec timeout * 10).
Of course in the alternative I'm hoping for, we simply not start a view transition until the queue is empty and rely on existing scheduling to run the callbacks. This would mean that startViewTransition()
after the above would seemingly do nothing (and the page would be interactive) and then suddenly trigger 40 seconds later.
Neither of these approaches seem great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to understand the implications of the rendering loop one:
An important note is that we're just calling those skipped update callbacks but not waiting for them to resolve.
- Does this mean if I run a VT and skip it (essentially queuing up the DOM update callback), then this addition would mean that we guarantee to run that callback before the next update the rendering? (this is a new constraint IIUC)
It's not a new constraint exactly. It means that instead of running the callback at the end of the rendering loop like in the current spec, we'll run it in the beginning.
It's a possible scenario already if the scheduled task happens to pop before the next rendering opportunity, which is the guaranteed behavior in Safari.
- If I do queue up a transition that ends up cancelling the previous one, is there an opportunity for the visuals of the first callback to appear on screen before the capture of the second callback? It sounds like yes since the update the rendering will produce a frame here.
The skipped one would modify the DOM at the same rendering loop as the old state capture for the new transition.
It's entirely equivalent to a scenario where the scheduled task happened to pop before the next rendering loop after starting the new transition.
Another comment here is that we can have a situation where you have something like
for (let i = 0; i < 10; i++) { document.startViewTransition(async () => { await new Promise(() => {}) }); }
which can essentially hang the page for 40 seconds (4sec timeout * 10).
The following would happen:
- The first task would potentially pop, running the first 9 async functions
- The next rendering cycle would start, immediately running the first 9 async functions if they weren't called yet
- The old state would be captured, and the 10th async function would run, supressing rendering for 4 seconds.
Of course in the alternative I'm hoping for, we simply not start a view transition until the queue is empty and rely on existing scheduling to run the callbacks. This would mean that
startViewTransition()
after the above would seemingly do nothing (and the page would be interactive) and then suddenly trigger 40 seconds later.Neither of these approaches seem great
1. [=list/For each=] |transition| in |document|'s [=update callback queue=], [=call the update callback=] given |transition|. | ||
|
||
1. Set |document|'s [=update callback queue=] to an empty list. | ||
</div> | ||
|
||
Note: a scheduled update callback is guaranteed to be called before the next rendering steps, regardless of whether the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... what's the behavior if you put stuff in the queue in a requestAnimationFrame
callback with your proposed behavior? That would have a frame delay right? Not necessarily a deal breaker, just trying to understand the implications of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... what's the behavior if you put stuff in the queue in a
requestAnimationFrame
callback with your proposed behavior? That would have a frame delay right? Not necessarily a deal breaker, just trying to understand the implications of this.
I don't think this scenario is valid.
If you have a scheduled update callback that wasn't cancelled yet, meaning that this is an active transition waiting for the new state to settle, rendering is guaranteed to be suppressed so we won't enter the rendering loop.
The only time this would take effect is if there are skipped transitions plus a new one that was scheduled before this rendering loop and hadn't had its old state captured yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if you started and skipped more than one view transition inside the rAF
callback itself, those callbacks would be counted towards the new state rather than the old state. I think that's ok.
This ensures that a DOM update callback that skips the active transition would take effect.
Closes #11943
[css-spec-shortname-1] Brief description which should also include the #issuenum-or-URL and/or link to relevant CSSWG minutes.
Copy the above line into the Title and replace with the relevant details. Fill in any additional details here. See https://github.com/w3c/csswg-drafts/blob/master/CONTRIBUTING.md for more info.