Repository navigation
script: Deliver animation frames to dedicated workers - #46232
Merged
Taym95 merged 5 commits intoAug 6, 2026
Merged
Conversation
Taym95
marked this pull request as draft
July 3, 2026 10:23
|
🔨 Triggering try run (#28654422205) for Linux (WPT) |
Taym95
force-pushed
the
script-dedicated-worker-animation-frames
branch
from
July 3, 2026 10:43
4f9d58b to
f936d54
Compare
|
Test results for linux-wpt from try job (#28654422205): Flaky unexpected result (26)
Stable unexpected results that are known to be intermittent (13)
|
|
✨ Try run (#28654422205) succeeded. |
gterzian
reviewed
Jul 3, 2026
Member
There was a problem hiding this comment.
I'm not sure if we want the constellation to directly send a rednering oppirtunity to workers, because the concept of https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#concept-animationframeprovider-supported depends on https://html.spec.whatwg.org/multipage/workers.html#concept-WorkerGlobalScope-owner-set, which changes at https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroying-documents:concept-WorkerGlobalScope-owner-set.
So maybe this can be managed by the constellation, since the constellation also is involved in unregistereing the worker as a receiver of rendering opportunities, but now this happens as part of https://html.spec.whatwg.org/multipage/workers.html#close-a-worker, which is not the same thing as when the spec removes the document from the owner set (although in Servo it might actually kind of the same thing, I haven't traced through it yet).
so I think the important part is, when a document is removed from the owner set, which is when it is destroyed in the spec, we should from then on stop delivering rendering opportunities to the worker, because it is not supported anymore from that point on.
This can be done by the constellation, but instead of happening only when the worker is closed, it should happen also when the owner set on which the "supported" concept relies(so in the case of nested workers it needs to go up the chain) becomes empty, so basically when the constellation has sent an exit message to a pipeline, it should remove that pipeline from the owner set, and then if it is empty stop sending rendering opportunities to workers who rely on that owner set to be supported.
So either way, you could make the script-thread responsible for forward rendering opportunities to supported workers, but this would require quite some changes, or you need to ensure this happens via teh constellation.
gterzian
reviewed
Jul 3, 2026
gterzian
reviewed
Jul 3, 2026
gterzian
left a comment
Member
There was a problem hiding this comment.
So to recap, we need to not just mark a worker as supported or not upon initialization, we need to track in some way what the owner set it, which can be done either by the script-thread(but that requires making it responsible for forwarding rednering opportunies to workers), or by the constellation (so pipelines are tracked as owners of workers, and nested workers rely on the owner set of the top most worker)
Member
This may also be important because the compositor doesn't necessarily manage generating animation frames. When a page is requesting animation frames and nothing in the DOM is changing, generally the |
Member
|
Another important question here: what should happen to workers and how they are managing animation frames when animation frame management moves from the |
Member
|
I've had a look at what the script-thread does for rafs, and I now tend more towards the idea to handle the worker rafs from the constellation (or something at that level but living in the
Also worth noting that this PR can focus on the delivery of a rendering opportunity to the worker, by queuing the update the rendering task, and the running of rafs, and the actually plugging into the painting can be done later as part of offscreen canvas (without it there is actually nothing to paint). |
Member
Author
window callbacks still use TickAllAnimations, while worker callbacks are merged into the same per-pipeline callback state that we send to Paint. |
Taym95
force-pushed
the
script-dedicated-worker-animation-frames
branch
from
July 22, 2026 15:24
f936d54 to
5c1cc10
Compare
Member
Author
Yes the current code already does that:
|
|
🔨 Triggering try run (#29933340049) for Linux (WPT) |
|
|
Taym95
force-pushed
the
script-dedicated-worker-animation-frames
branch
2 times, most recently
from
July 27, 2026 10:59
c74b7c2 to
fb7d201
Compare
|
🔨 Triggering try run (#30260288820) for Linux (WPT) |
|
Test results for linux-wpt from try job (#30260288820): Flaky unexpected result (37)
Stable unexpected results that are known to be intermittent (17)
|
|
✨ Try run (#30260288820) succeeded. |
gterzian
reviewed
Jul 27, 2026
gterzian
left a comment
Member
There was a problem hiding this comment.
couple of comments/questions
| let animation_state = if callbacks_active { | ||
| AnimationState::AnimationCallbacksPresent | ||
| } else { | ||
| AnimationState::NoAnimationCallbacksPresent |
| #[no_trace] | ||
| #[ignore_malloc_size_of = "channels are hard"] | ||
| animation_frame_tick_receiver: Option<RoutedReceiver<WorkerAnimationFrameTick>>, | ||
| /// A receiver of control messages. |
Member
There was a problem hiding this comment.
nit, it would be more readable with a spec between fields.
| #[conditional_malloc_size_of] | ||
| closing: Arc<AtomicBool>, | ||
| #[conditional_malloc_size_of] | ||
| animation_frame_provider_supported: Arc<AtomicBool>, |
Member
There was a problem hiding this comment.
Not sure we need this, because if the owning pipeline is closed the constellation will know about it, so it could stop sending rendering ops to the worker.
Member
Author
There was a problem hiding this comment.
Huum, I think we need to keep it because the owner Doc can be destroyed before the pipeline exits, this flag disables rAF and clears the worker’s local callbacks other constellation only manages provider/tick tracking.
| } | ||
|
|
||
| fn register_animation_frame_provider(&self) { | ||
| if !self.animation_frame_provider_supported() { |
Member
There was a problem hiding this comment.
so this covers the case where the owner is closed even before the worker is started?
Not sure if that can ever be false, because starting a worker is called synchronously at
servo/components/script/dom/workers/worker.rs
Line 255 in e0363e4
Member
Author
There was a problem hiding this comment.
what if the owner document is be destroyed before the worker thread reaches register_animation_frame_provider() because run_worker_scope() starts the worker event loop asynchronously ?
| AnimationState::NoAnimationCallbacksPresent => { | ||
| self.handle_change_document_animation_frame_provider_state(pipeline_id, false); | ||
| }, | ||
| AnimationState::AnimationsPresent | AnimationState::NoAnimationsPresent => { |
Member
There was a problem hiding this comment.
does this logic not logically cover the two above match arms as well, meaning we can remove the above two?
Member
Author
There was a problem hiding this comment.
this need separate handler because AnimationsPresent and NoAnimationsPresent update the pipeline’s ordinary animation state, and callback states they update the document/worker callback bookkeeping.
| cx, | ||
| ); | ||
|
|
||
| global.register_animation_frame_provider(); |
Member
There was a problem hiding this comment.
This should only be called once the first rafs have been requested for the worker scope.
Member
Author
There was a problem hiding this comment.
indeed to avoid registering inactive worker
| debug!( | ||
| "Registering worker animation frame provider: worker={worker_id:?}, pipeline={pipeline_id:?}---->", | ||
| ); | ||
| self.worker_animation_frame_providers.insert( |
Member
There was a problem hiding this comment.
I think this might be ok for an initial pass, but this makes the workers animations dependent on that of the owning pipelines, so if the document itself isn't animating, we should probably send a PaintMessage::ChangeRunningAnimationsState here to make sure it does, although that could deliver rafs to a document that hasn't requested any. So ideally we should change the logic so that for a given pipeline (since that is what the painter tracks), we can opt to deliver rendering ops, but only for the actual global that requested it(so worker or documents, iframes?, independent of each other).
Right now I think delivering rendering ops to the worker is entirely dependent on the document animating (but I might be missing something).
Member
Author
There was a problem hiding this comment.
Worker rAF is independent of document animation now, Paint still schedules at pipeline/WebView granularity, so finer per global scheduling can be handled separately.
|
|
||
| /// <https://html.spec.whatwg.org/multipage/#event-loop-processing-model> | ||
| #[servo_tracing::instrument(skip_all)] | ||
| fn handle_tick_animation(&mut self, webview_ids: Vec<WebViewId>) { |
Member
There was a problem hiding this comment.
So this is entirely driven by documents having set the animating flag on the painter.
Member
Author
There was a problem hiding this comment.
callbacks_active is true when either:
pipeline.document_callbacks_active ||
!pipeline.worker_callbacks_active.is_empty()
| pub document_callbacks_active: bool, | ||
|
|
||
| /// Workers in this pipeline with active animation frame callbacks. | ||
| pub worker_callbacks_active: FxHashSet<WorkerId>, |
Member
There was a problem hiding this comment.
Not sure we need bot this and worker_animation_frame_providers, can we not consolidate in one place?
Member
Author
There was a problem hiding this comment.
Combining them would mix provider lifetime/transport state with active callback state and complicate perpipeline scheduling.
Taym95
force-pushed
the
script-dedicated-worker-animation-frames
branch
from
July 27, 2026 23:33
fb7d201 to
d040999
Compare
Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
Taym95
force-pushed
the
script-dedicated-worker-animation-frames
branch
from
August 1, 2026 21:35
d040999 to
7587e7c
Compare
|
🔨 Triggering try run (#30719468994) for Linux (WPT) |
|
Test results for linux-wpt from try job (#30719468994): Flaky unexpected result (37)
Stable unexpected results that are known to be intermittent (14)
|
|
✨ Try run (#30719468994) succeeded. |
gterzian
approved these changes
Aug 3, 2026
gterzian
left a comment
Member
There was a problem hiding this comment.
what if during worker or pipeline teardown a missing provider can occur? we would just skip it no?
Yes but we can log an error.
Otherwise looks good to me as a first step towards enabling off thread canvas.
Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
Taym95
enabled auto-merge
August 6, 2026 09:02
RichardTjokroutomo
pushed a commit
to RichardTjokroutomo/servo
that referenced
this pull request
Oct 6, 2026
this exposess AnimationFrameProvider on DedicatedWorkerGlobalScope, stores and cancels worker rAF callbacks on the worker global, and delivers callbacks through the worker event loop Testing: tests/wpt/tests/workers/WorkerGlobalScope_requestAnimationFrame.worker.js tests/wpt/tests/workers/worker-request-animation-frame.html tests/wpt/tests/html/dom/idlharness.any.js --------- Signed-off-by: Taym Haddadi <haddadi.taym@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this exposess AnimationFrameProvider on DedicatedWorkerGlobalScope, stores and cancels worker rAF callbacks on the worker global, and delivers callbacks through the worker event loop
Testing:
tests/wpt/tests/workers/WorkerGlobalScope_requestAnimationFrame.worker.js
tests/wpt/tests/workers/worker-request-animation-frame.html
tests/wpt/tests/html/dom/idlharness.any.js