From 23acb623c887a095e8c6b1e2db10ca1398614945 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Thu, 12 Jun 2025 21:25:04 +0200 Subject: [PATCH] script: Allow reflows that do not produce display lists (#37186) This change has two parts which depend on each other: 1. An early exit in the layout process, which allows for skipping display list construction entirely when nothing would change. 2. A simplification and unification of the way that "fake" animation frames are triggered. Now this happens on an entire ScriptThread at once and is based on whether or not any Pipeline triggered a display list update. Animations are never canceled in the compositor when the Pipeline isn't updating, instead the fake animation frame is triggered far enough in the future that an unexpected compositor tick will cancel it. This could happen, for instance, if some other Pipeline in some other ScriptThread produced a new display list for a tick. This makes everything simpler about these ticks. The goal is that in a future change the ScriptThread-based animation ticks will be made more generic so that they can throttle the number of "update the rendering" calls triggered by script. This should make Servo do a lot less work when moving the cursor over a page. Before it would constantly produce new display lists. Fixes: #17029. Testing: This should not cause any web observable changes. The fact that all WPT tests keep passing is the test for this change. Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/compositing/refresh_driver.rs | 6 +- components/layout/layout_impl.rs | 64 ++++++-- components/script/dom/document.rs | 141 +++--------------- components/script/dom/documentorshadowroot.rs | 9 +- components/script/dom/globalscope.rs | 9 +- components/script/dom/node.rs | 8 +- components/script/dom/window.rs | 44 ++---- components/script/script_thread.rs | 115 ++++++++++++-- components/script/timers.rs | 40 +++-- components/shared/script_layout/lib.rs | 2 + components/timers/lib.rs | 53 ++++--- 11 files changed, 257 insertions(+), 234 deletions(-) diff --git a/components/compositing/refresh_driver.rs b/components/compositing/refresh_driver.rs index 5531a7257c8..1fda0dc32fb 100644 --- a/components/compositing/refresh_driver.rs +++ b/components/compositing/refresh_driver.rs @@ -14,7 +14,7 @@ use constellation_traits::EmbedderToConstellationMessage; use crossbeam_channel::{Sender, select}; use embedder_traits::EventLoopWaker; use log::warn; -use timers::{BoxedTimerCallback, TimerEventId, TimerEventRequest, TimerScheduler, TimerSource}; +use timers::{BoxedTimerCallback, TimerEventRequest, TimerScheduler}; use crate::compositor::RepaintReason; use crate::webview_renderer::WebViewRenderer; @@ -62,7 +62,7 @@ impl RefreshDriver { fn timer_callback(&self) -> BoxedTimerCallback { let waiting_for_frame_timeout = self.waiting_for_frame_timeout.clone(); let event_loop_waker = self.event_loop_waker.clone_box(); - Box::new(move |_| { + Box::new(move || { waiting_for_frame_timeout.store(false, Ordering::Relaxed); event_loop_waker.wake(); }) @@ -226,8 +226,6 @@ impl TimerThread { .sender .send(TimerThreadMessage::Request(TimerEventRequest { callback, - source: TimerSource::FromWorker, - id: TimerEventId(0), duration, })); } diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 61d37a5a91f..5eb88a218e8 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -136,8 +136,18 @@ pub struct LayoutThread { /// A FontContext to be used during layout. font_context: Arc, + /// Whether or not user agent stylesheets have been added to the Stylist or not. + have_added_user_agent_stylesheets: bool, + /// Is this the first reflow in this LayoutThread? - first_reflow: Cell, + have_ever_generated_display_list: Cell, + + /// Whether a new display list is necessary due to changes to layout or stacking + /// contexts. This is set to true every time layout changes, even when a display list + /// isn't requested for this layout, such as for layout queries. The next time a + /// layout requests a display list, it is produced unconditionally, even when the + /// layout trees remain the same. + need_new_display_list: Cell, /// The box tree. box_tree: RefCell>>, @@ -520,7 +530,9 @@ impl LayoutThread { registered_painters: RegisteredPaintersImpl(Default::default()), image_cache: config.image_cache, font_context: config.font_context, - first_reflow: Cell::new(true), + have_added_user_agent_stylesheets: false, + have_ever_generated_display_list: Cell::new(false), + need_new_display_list: Cell::new(false), box_tree: Default::default(), fragment_tree: Default::default(), stacking_context_tree: Default::default(), @@ -659,9 +671,9 @@ impl LayoutThread { ); self.calculate_overflow(damage); self.build_stacking_context_tree(&reflow_request, damage); - self.build_display_list(&reflow_request, &mut layout_context); + let built_display_list = + self.build_display_list(&reflow_request, damage, &mut layout_context); - self.first_reflow.set(false); if let ReflowGoal::UpdateScrollNode(scroll_state) = reflow_request.reflow_goal { self.update_scroll_node_state(&scroll_state); } @@ -674,6 +686,7 @@ impl LayoutThread { std::mem::take(&mut *layout_context.node_image_animation_map.write()); Some(ReflowResult { + built_display_list, pending_images, pending_rasterization_images, iframe_sizes, @@ -709,7 +722,7 @@ impl LayoutThread { ua_stylesheets: &UserAgentStylesheets, snapshot_map: &SnapshotMap, ) { - if self.first_reflow.get() { + if !self.have_added_user_agent_stylesheets { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { self.stylist .append_stylesheet(stylesheet.clone(), guards.ua_or_user); @@ -726,6 +739,7 @@ impl LayoutThread { guards.ua_or_user, ); } + self.have_added_user_agent_stylesheets = true; } if reflow_request.stylesheets_changed { @@ -818,6 +832,9 @@ impl LayoutThread { // had is now out of date and should be rebuilt. *self.stacking_context_tree.borrow_mut() = None; + // Force display list generation as layout has changed. + self.need_new_display_list.set(true); + if self.debug.dump_style_tree { println!( "{:?}", @@ -879,28 +896,41 @@ impl LayoutThread { fragment_tree, viewport_size, self.id.into(), - self.first_reflow.get(), + !self.have_ever_generated_display_list.get(), &self.debug, )); + + // Force display list generation as layout has changed. + self.need_new_display_list.set(true); } + /// Build the display list for the current layout and send it to the renderer. If no display + /// list is built, returns false. fn build_display_list( &self, reflow_request: &ReflowRequest, + damage: RestyleDamage, layout_context: &mut LayoutContext<'_>, - ) { + ) -> bool { if !reflow_request.reflow_goal.needs_display() { - return; + return false; } let Some(fragment_tree) = &*self.fragment_tree.borrow() else { - return; + return false; }; - let mut stacking_context_tree = self.stacking_context_tree.borrow_mut(); let Some(stacking_context_tree) = stacking_context_tree.as_mut() else { - return; + return false; }; + // It's not enough to simply check `damage` here as not all reflow requests + // require display lists. If a non-display-list-generating reflow updated layout + // in a previous refow, we cannot skip display list generation here the next time + // a display list is requested. + if !self.need_new_display_list.get() && !damage.contains(RestyleDamage::REPAINT) { + return false; + } + let mut epoch = self.epoch.get(); epoch.next(); self.epoch.set(epoch); @@ -922,7 +952,11 @@ impl LayoutThread { .font_context .collect_unused_webrender_resources(false /* all */); self.compositor_api - .remove_unused_font_resources(keys, instance_keys) + .remove_unused_font_resources(keys, instance_keys); + + self.have_ever_generated_display_list.set(true); + self.need_new_display_list.set(false); + true } fn update_scroll_node_state(&self, state: &ScrollState) { @@ -947,10 +981,10 @@ impl LayoutThread { } else { TimerMetadataFrameType::RootWindow }, - incremental: if self.first_reflow.get() { - TimerMetadataReflowType::FirstReflow - } else { + incremental: if self.have_ever_generated_display_list.get() { TimerMetadataReflowType::Incremental + } else { + TimerMetadataReflowType::FirstReflow }, }) } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 99fca82bc88..4d235d7eb15 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -212,15 +212,6 @@ use crate::task::TaskBox; use crate::task_source::TaskSourceName; use crate::timers::OneshotTimerCallback; -/// The number of times we are allowed to see spurious `requestAnimationFrame()` calls before -/// falling back to fake ones. -/// -/// A spurious `requestAnimationFrame()` call is defined as one that does not change the DOM. -const SPURIOUS_ANIMATION_FRAME_THRESHOLD: u8 = 5; - -/// The amount of time between fake `requestAnimationFrame()`s. -const FAKE_REQUEST_ANIMATION_FRAME_DELAY: u64 = 16; - pub(crate) enum TouchEventResult { Processed(bool), Forwarded, @@ -2563,27 +2554,26 @@ impl Document { /// pub(crate) fn request_animation_frame(&self, callback: AnimationFrameCallback) -> u32 { let ident = self.animation_frame_ident.get() + 1; - self.animation_frame_ident.set(ident); - self.animation_frame_list - .borrow_mut() - .push_back((ident, Some(callback))); - // If we are running 'fake' animation frames, we unconditionally - // set up a one-shot timer for script to execute the rAF callbacks. - if self.is_faking_animation_frames() && !self.window().throttled() { - self.schedule_fake_animation_frame(); - } else if !self.running_animation_callbacks.get() { - // No need to send a `ChangeRunningAnimationsState` if we're running animation callbacks: - // we're guaranteed to already be in the "animation callbacks present" state. - // - // This reduces CPU usage by avoiding needless thread wakeups in the common case of - // repeated rAF. + let had_animation_frame_callbacks; + { + let mut animation_frame_list = self.animation_frame_list.borrow_mut(); + had_animation_frame_callbacks = !animation_frame_list.is_empty(); + animation_frame_list.push_back((ident, Some(callback))); + } - let event = ScriptToConstellationMessage::ChangeRunningAnimationsState( - AnimationState::AnimationCallbacksPresent, + // No need to send a `ChangeRunningAnimationsState` if we're running animation callbacks: + // we're guaranteed to already be in the "animation callbacks present" state. + // + // This reduces CPU usage by avoiding needless thread wakeups in the common case of + // repeated rAF. + if !self.running_animation_callbacks.get() && !had_animation_frame_callbacks { + self.window().send_to_constellation( + ScriptToConstellationMessage::ChangeRunningAnimationsState( + AnimationState::AnimationCallbacksPresent, + ), ); - self.window().send_to_constellation(event); } ident @@ -2597,23 +2587,11 @@ impl Document { } } - fn schedule_fake_animation_frame(&self) { - warn!("Scheduling fake animation frame. Animation frames tick too fast."); - let callback = FakeRequestAnimationFrameCallback { - document: Trusted::new(self), - }; - self.global().schedule_callback( - OneshotTimerCallback::FakeRequestAnimationFrame(callback), - Duration::from_millis(FAKE_REQUEST_ANIMATION_FRAME_DELAY), - ); - } - /// pub(crate) fn run_the_animation_frame_callbacks(&self, can_gc: CanGc) { let _realm = enter_realm(self); self.running_animation_callbacks.set(true); - let was_faking_animation_frames = self.is_faking_animation_frames(); let timing = self.global().performance().Now(); let num_callbacks = self.animation_frame_list.borrow().len(); @@ -2623,68 +2601,12 @@ impl Document { callback.call(self, *timing, can_gc); } } - self.running_animation_callbacks.set(false); - let callbacks_did_not_trigger_reflow = self.needs_reflow().is_none(); - let is_empty = self.animation_frame_list.borrow().is_empty(); - if !is_empty && callbacks_did_not_trigger_reflow && !was_faking_animation_frames { - // If the rAF callbacks did not mutate the DOM, then the impending - // reflow call as part of *update the rendering* will not do anything - // and therefore no new frame will be sent to the compositor. - // If this happens, the compositor will not tick the animation - // and the next rAF will never be called! When this happens - // for several frames, then the spurious rAF detection below - // will kick in and use a timer to tick the callbacks. However, - // for the interim frames where we are deciding whether this rAF - // is considered spurious, we need to ensure that the layout - // and compositor *do* tick the animation. - self.set_needs_paint(true); - } - - // Update the counter of spurious animation frames. - let spurious_frames = self.spurious_animation_frames.get(); - if callbacks_did_not_trigger_reflow { - if spurious_frames < SPURIOUS_ANIMATION_FRAME_THRESHOLD { - self.spurious_animation_frames.set(spurious_frames + 1); - } - } else { - self.spurious_animation_frames.set(0); - } - - // Only send the animation change state message after running any callbacks. - // This means that if the animation callback adds a new callback for - // the next frame (which is the common case), we won't send a NoAnimationCallbacksPresent - // message quickly followed by an AnimationCallbacksPresent message. - // - // If this frame was spurious and we've seen too many spurious frames in a row, tell the - // constellation to stop giving us video refresh callbacks, to save energy. (A spurious - // animation frame is one in which the callback did not mutate the DOM—that is, an - // animation frame that wasn't actually used for animation.) - let just_crossed_spurious_animation_threshold = - !was_faking_animation_frames && self.is_faking_animation_frames(); - if is_empty || just_crossed_spurious_animation_threshold { - if !is_empty { - // We just realized that we need to stop requesting compositor's animation ticks - // due to spurious animation frames, but we still have rAF callbacks queued. Since - // `is_faking_animation_frames` would not have been true at the point where these - // new callbacks were registered, the one-shot timer will not have been setup in - // `request_animation_frame()`. Since we stop the compositor ticks below, we need - // to expliclty trigger a OneshotTimerCallback for these queued callbacks. - self.schedule_fake_animation_frame(); - } - let event = ScriptToConstellationMessage::ChangeRunningAnimationsState( - AnimationState::NoAnimationCallbacksPresent, - ); - self.window().send_to_constellation(event); - } - - // If we were previously faking animation frames, we need to re-enable video refresh - // callbacks when we stop seeing spurious animation frames. - if was_faking_animation_frames && !self.is_faking_animation_frames() && !is_empty { + if self.animation_frame_list.borrow().is_empty() { self.window().send_to_constellation( ScriptToConstellationMessage::ChangeRunningAnimationsState( - AnimationState::AnimationCallbacksPresent, + AnimationState::NoAnimationCallbacksPresent, ), ); } @@ -4715,12 +4637,6 @@ impl Document { .set(self.ignore_opens_during_unload_counter.get() - 1); } - /// Whether we've seen so many spurious animation frames (i.e. animation frames that didn't - /// mutate the DOM) that we've decided to fall back to fake ones. - fn is_faking_animation_frames(&self) -> bool { - self.spurious_animation_frames.get() >= SPURIOUS_ANIMATION_FRAME_THRESHOLD - } - // https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen pub(crate) fn enter_fullscreen(&self, pending: &Element, can_gc: CanGc) -> Rc { // Step 1 @@ -6758,27 +6674,6 @@ pub(crate) enum FocusEventType { Blur, // Element lost focus. Doesn't bubble. } -/// A fake `requestAnimationFrame()` callback—"fake" because it is not triggered by the video -/// refresh but rather a simple timer. -/// -/// If the page is observed to be using `requestAnimationFrame()` for non-animation purposes (i.e. -/// without mutating the DOM), then we fall back to simple timeouts to save energy over video -/// refresh. -#[derive(JSTraceable, MallocSizeOf)] -pub(crate) struct FakeRequestAnimationFrameCallback { - /// The document. - #[ignore_malloc_size_of = "non-owning"] - document: Trusted, -} - -impl FakeRequestAnimationFrameCallback { - pub(crate) fn invoke(self, can_gc: CanGc) { - // TODO: Once there is a more generic mechanism to trigger `update_the_rendering` when - // not driven by the compositor, it should be used here. - with_script_thread(|script_thread| script_thread.update_the_rendering(true, can_gc)) - } -} - /// This is a temporary workaround to update animated images, /// we should get rid of this after we have refresh driver #3406 #[derive(JSTraceable, MallocSizeOf)] diff --git a/components/script/dom/documentorshadowroot.rs b/components/script/dom/documentorshadowroot.rs index e3b09924689..5672282f921 100644 --- a/components/script/dom/documentorshadowroot.rs +++ b/components/script/dom/documentorshadowroot.rs @@ -103,13 +103,8 @@ impl DocumentOrShadowRoot { query_type: NodesFromPointQueryType, can_gc: CanGc, ) -> Vec { - if !self - .window - .layout_reflow(QueryMsg::NodesFromPointQuery, can_gc) - { - return vec![]; - }; - + self.window + .layout_reflow(QueryMsg::NodesFromPointQuery, can_gc); self.window .layout() .query_nodes_from_point(*client_point, query_type) diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index ade7b86caa8..011f7eb15b6 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -65,7 +65,7 @@ use profile_traits::{ipc as profile_ipc, mem as profile_mem, time as profile_tim use script_bindings::interfaces::GlobalScopeHelpers; use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; use snapshot::Snapshot; -use timers::{TimerEventId, TimerEventRequest, TimerSource}; +use timers::{TimerEventRequest, TimerId}; use url::Origin; use uuid::Uuid; #[cfg(feature = "webgpu")] @@ -150,6 +150,7 @@ use crate::task_manager::TaskManager; use crate::task_source::SendableTaskSource; use crate::timers::{ IsInterval, OneshotTimerCallback, OneshotTimerHandle, OneshotTimers, TimerCallback, + TimerEventId, TimerSource, }; use crate::unminify::unminified_path; @@ -2483,10 +2484,10 @@ impl GlobalScope { /// Schedule a [`TimerEventRequest`] on this [`GlobalScope`]'s [`timers::TimerScheduler`]. /// Every Worker has its own scheduler, which handles events in the Worker event loop, /// but `Window`s use a shared scheduler associated with their [`ScriptThread`]. - pub(crate) fn schedule_timer(&self, request: TimerEventRequest) { + pub(crate) fn schedule_timer(&self, request: TimerEventRequest) -> Option { match self.downcast::() { - Some(worker_global) => worker_global.timer_scheduler().schedule_timer(request), - _ => with_script_thread(|script_thread| script_thread.schedule_timer(request)), + Some(worker_global) => Some(worker_global.timer_scheduler().schedule_timer(request)), + _ => with_script_thread(|script_thread| Some(script_thread.schedule_timer(request))), } } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 6a969ccf157..e61f3c45c2a 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1443,12 +1443,8 @@ impl Node { } pub(crate) fn style(&self, can_gc: CanGc) -> Option> { - if !self - .owner_window() - .layout_reflow(QueryMsg::StyleQuery, can_gc) - { - return None; - } + self.owner_window() + .layout_reflow(QueryMsg::StyleQuery, can_gc); self.style_data .borrow() .as_ref() diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 48460e2546c..b34da3f06d2 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -2143,7 +2143,7 @@ impl Window { /// no reflow is performed. If reflow is suppressed, no reflow will be performed for ForDisplay /// goals. /// - /// Returns true if layout actually happened, false otherwise. + /// Returns true if layout actually happened and it sent a new display list to the renderer. /// /// NOTE: This method should almost never be called directly! Layout and rendering updates should /// happen as part of the HTML event loop via *update the rendering*. @@ -2309,16 +2309,21 @@ impl Window { document.update_animations_post_reflow(); self.update_constellation_epoch(); - true + results.built_display_list } /// Reflows the page if it's possible to do so and the page is dirty. Returns true if layout - /// actually happened, false otherwise. + /// actually happened and produced a new display list, false otherwise. /// /// NOTE: This method should almost never be called directly! Layout and rendering updates /// should happen as part of the HTML event loop via *update the rendering*. Currerntly, the /// only exceptions are script queries and scroll requests. pub(crate) fn reflow(&self, reflow_goal: ReflowGoal, can_gc: CanGc) -> bool { + // Never reflow inactive Documents. + if !self.Document().is_fully_active() { + return false; + } + // Count the pending web fonts before layout, in case a font loads during the layout. let waiting_for_web_fonts_to_load = self.font_context.web_fonts_still_loading() != 0; @@ -2497,9 +2502,7 @@ impl Window { value: String, can_gc: CanGc, ) -> Option> { - if !self.layout_reflow(QueryMsg::ResolvedFontStyleQuery, can_gc) { - return None; - } + self.layout_reflow(QueryMsg::ResolvedFontStyleQuery, can_gc); let document = self.Document(); let animations = document.animations().sets.clone(); @@ -2519,25 +2522,19 @@ impl Window { } pub(crate) fn content_box_query(&self, node: &Node, can_gc: CanGc) -> Option> { - if !self.layout_reflow(QueryMsg::ContentBox, can_gc) { - return None; - } + self.layout_reflow(QueryMsg::ContentBox, can_gc); self.content_box_query_unchecked(node) } pub(crate) fn content_boxes_query(&self, node: &Node, can_gc: CanGc) -> Vec> { - if !self.layout_reflow(QueryMsg::ContentBoxes, can_gc) { - return vec![]; - } + self.layout_reflow(QueryMsg::ContentBoxes, can_gc); self.layout .borrow() .query_content_boxes(node.to_trusted_node_address()) } pub(crate) fn client_rect_query(&self, node: &Node, can_gc: CanGc) -> UntypedRect { - if !self.layout_reflow(QueryMsg::ClientRectQuery, can_gc) { - return Rect::zero(); - } + self.layout_reflow(QueryMsg::ClientRectQuery, can_gc); self.layout .borrow() .query_client_rect(node.to_trusted_node_address()) @@ -2550,9 +2547,7 @@ impl Window { node: Option<&Node>, can_gc: CanGc, ) -> UntypedRect { - if !self.layout_reflow(QueryMsg::ScrollingAreaQuery, can_gc) { - return Rect::zero(); - } + self.layout_reflow(QueryMsg::ScrollingAreaQuery, can_gc); self.layout .borrow() .query_scrolling_area(node.map(Node::to_trusted_node_address)) @@ -2603,9 +2598,7 @@ impl Window { property: PropertyId, can_gc: CanGc, ) -> DOMString { - if !self.layout_reflow(QueryMsg::ResolvedStyleQuery, can_gc) { - return DOMString::new(); - } + self.layout_reflow(QueryMsg::ResolvedStyleQuery, can_gc); let document = self.Document(); let animations = document.animations().sets.clone(); @@ -2640,10 +2633,7 @@ impl Window { node: &Node, can_gc: CanGc, ) -> (Option>, UntypedRect) { - if !self.layout_reflow(QueryMsg::OffsetParentQuery, can_gc) { - return (None, Rect::zero()); - } - + self.layout_reflow(QueryMsg::OffsetParentQuery, can_gc); let response = self .layout .borrow() @@ -2661,9 +2651,7 @@ impl Window { point_in_node: UntypedPoint2D, can_gc: CanGc, ) -> Option { - if !self.layout_reflow(QueryMsg::TextIndexQuery, can_gc) { - return None; - } + self.layout_reflow(QueryMsg::TextIndexQuery, can_gc); self.layout .borrow() .query_text_indext(node.to_opaque(), point_in_node) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 695f8e7edfe..d2a0dea2c6a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -95,7 +95,7 @@ use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; use style::dom::OpaqueNode; use style::thread_state::{self, ThreadState}; use stylo_atoms::Atom; -use timers::{TimerEventRequest, TimerScheduler}; +use timers::{TimerEventRequest, TimerId, TimerScheduler}; use url::Position; #[cfg(feature = "webgpu")] use webgpu_traits::{WebGPUDevice, WebGPUMsg}; @@ -339,6 +339,18 @@ pub struct ScriptThread { /// The screen coordinates where the primary mouse button was pressed. #[no_trace] relative_mouse_down_point: Cell>, + + /// The [`TimerId`] of the scheduled ScriptThread-only animation tick timer, if any. + /// This may be non-`None` when rAF callbacks do not trigger display list creation. In + /// that case the compositor will never trigger a new animation tick because it's + /// dependent on the rendering of a new WebRender frame. + #[no_trace] + scheduled_script_thread_animation_timer: RefCell>, + + /// A flag that lets the [`ScriptThread`]'s main loop know that the + /// [`Self::scheduled_script_thread_animation_timer`] timer fired and it should + /// trigger an animation tick "update the rendering" call. + should_trigger_script_thread_animation_tick: Arc, } struct BHMExitSignal { @@ -554,8 +566,8 @@ impl ScriptThread { } /// Schedule a [`TimerEventRequest`] on this [`ScriptThread`]'s [`TimerScheduler`]. - pub(crate) fn schedule_timer(&self, request: TimerEventRequest) { - self.timer_scheduler.borrow_mut().schedule_timer(request); + pub(crate) fn schedule_timer(&self, request: TimerEventRequest) -> TimerId { + self.timer_scheduler.borrow_mut().schedule_timer(request) } // https://html.spec.whatwg.org/multipage/#await-a-stable-state @@ -966,6 +978,8 @@ impl ScriptThread { inherited_secure_context: state.inherited_secure_context, layout_factory, relative_mouse_down_point: Cell::new(Point2D::zero()), + scheduled_script_thread_animation_timer: Default::default(), + should_trigger_script_thread_animation_tick: Arc::new(AtomicBool::new(false)), } } @@ -1174,9 +1188,34 @@ impl ScriptThread { /// /// Attempt to update the rendering and then do a microtask checkpoint if rendering was actually /// updated. - pub(crate) fn update_the_rendering(&self, requested_by_compositor: bool, can_gc: CanGc) { + pub(crate) fn update_the_rendering(&self, requested_by_renderer: bool, can_gc: CanGc) { *self.last_render_opportunity_time.borrow_mut() = Some(Instant::now()); + // If the ScriptThread animation timer fired, this is an animation tick. + let mut is_animation_tick = requested_by_renderer; + if self + .should_trigger_script_thread_animation_tick + .load(Ordering::Relaxed) + { + self.should_trigger_script_thread_animation_tick + .store(false, Ordering::Relaxed); + *self.scheduled_script_thread_animation_timer.borrow_mut() = None; + is_animation_tick = true; + } + + // If this is an animation tick, cancel any upcoming ScriptThread-based animation timer. + // This tick serves the purpose and we to limit animation ticks if some are coming from + // the renderer. + if requested_by_renderer { + if let Some(timer_id) = self + .scheduled_script_thread_animation_timer + .borrow_mut() + .take() + { + self.timer_scheduler.borrow_mut().cancel_timer(timer_id); + } + } + if !self.can_continue_running_inner() { return; } @@ -1196,7 +1235,7 @@ impl ScriptThread { // If we aren't explicitly running rAFs, this update wasn't requested by the compositor, // and we are running animations, then wait until the compositor tells us it is time to // update the rendering via a TickAllAnimations message. - if !requested_by_compositor && any_animations_running { + if !is_animation_tick && any_animations_running { return; } @@ -1220,6 +1259,7 @@ impl ScriptThread { // steps per doc in docs. Currently `