From 6005049d88766b415f38f62930d3edf79b8befe8 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Thu, 7 Mar 2024 12:59:02 +0100 Subject: [PATCH] compositor: Improve the way we wait for frames (#31523) * compositor: Improve the way we wait for frames In the newest version of WebRender it will be harder to make the distinction between frame queued for scrolling and other kinds of pending frames. This change makes it so that we queue frames for both kinds of changes the same way and keeps a counting of pending frames. This is conceptually a lot simpler. In addition, do queue a composite even when recomposite isn't necessary for a WebRender frame when there are active requestAnimationFrame callbacks. Doing a composite is what triggers the callbacks to actually run in the script thread! I believe this was a bug, but the WebRender upgrade made it much more obvious. These changes are in preparation for the WebRender upgrade. * Remove spurious println --- components/compositing/compositor.rs | 107 ++++++++++++++++----------- components/servo/lib.rs | 21 ++---- components/shared/compositing/lib.rs | 44 +---------- 3 files changed, 74 insertions(+), 98 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 612774a7675..300c0b5d3a0 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -13,9 +13,8 @@ use std::time::{SystemTime, UNIX_EPOCH}; use canvas::canvas_paint_thread::ImageUpdate; use compositing_traits::{ - CanvasToCompositorMsg, CompositingReason, CompositionPipeline, CompositorMsg, - CompositorReceiver, ConstellationMsg, FontToCompositorMsg, ForwardedToCompositorMsg, - SendableFrameTree, + CanvasToCompositorMsg, CompositionPipeline, CompositorMsg, CompositorReceiver, + ConstellationMsg, FontToCompositorMsg, ForwardedToCompositorMsg, SendableFrameTree, }; use crossbeam_channel::Sender; use embedder_traits::Cursor; @@ -182,9 +181,6 @@ pub struct IOCompositor { /// Pending scroll/zoom events. pending_scroll_zoom_events: Vec, - /// Whether we're waiting on a recomposite after dispatching a scroll. - waiting_for_results_of_scroll: bool, - /// Used by the logic that determines when it is safe to output an /// image for the reftest framework. ready_to_save_state: ReadyState, @@ -244,10 +240,8 @@ pub struct IOCompositor { /// True to translate mouse input into touch events. convert_mouse_to_touch: bool, - /// True if a WR frame render has been requested. Screenshots - /// taken before the render is complete will not reflect the - /// most up to date rendering. - waiting_on_pending_frame: bool, + /// The number of frames pending to receive from WebRender. + pending_frames: usize, /// Waiting for external code to call present. waiting_on_present: bool, @@ -272,6 +266,23 @@ enum ScrollZoomEvent { Scroll(ScrollEvent), } +/// Why we performed a composite. This is used for debugging. +/// +/// TODO: It would be good to have a bit more precision here about why a composite +/// was originally triggered, but that would require tracking the reason when a +/// frame is queued in WebRender and then remembering when the frame is ready. +#[derive(Clone, Copy, Debug, PartialEq)] +enum CompositingReason { + /// We're performing the single composite in headless mode. + Headless, + /// We're performing a composite to run an animation. + Animation, + /// A new WebRender frame has arrived. + NewWebRenderFrame, + /// The window has been resized and will need to be synchronously repainted. + Resize, +} + #[derive(Debug, PartialEq)] enum CompositionRequest { NoCompositingNecessary, @@ -387,7 +398,6 @@ impl IOCompositor { composition_request: CompositionRequest::NoCompositingNecessary, touch_handler: TouchHandler::new(), pending_scroll_zoom_events: Vec::new(), - waiting_for_results_of_scroll: false, composite_target, shutdown_state: ShutdownState::NotShuttingDown, page_zoom: Scale::new(1.0), @@ -415,7 +425,7 @@ impl IOCompositor { is_running_problem_test, exit_after_load, convert_mouse_to_touch, - waiting_on_pending_frame: false, + pending_frames: 0, waiting_on_present: false, } } @@ -529,6 +539,10 @@ impl IOCompositor { } fn handle_browser_message(&mut self, msg: CompositorMsg) -> bool { + if matches!(msg, CompositorMsg::NewWebRenderFrameReady(..)) { + self.pending_frames -= 1; + } + match (msg, self.shutdown_state) { (_, ShutdownState::FinishedShuttingDown) => { error!("compositor shouldn't be handling messages after shutting down"); @@ -552,11 +566,6 @@ impl IOCompositor { self.send_scroll_positions_to_layout_for_pipeline(&frame_tree.pipeline.id); }, - (CompositorMsg::Recomposite(reason), ShutdownState::NotShuttingDown) => { - self.waiting_on_pending_frame = false; - self.composition_request = CompositionRequest::CompositeNow(reason) - }, - (CompositorMsg::TouchEventProcessed(result), ShutdownState::NotShuttingDown) => { self.touch_handler.on_event_processed(result); }, @@ -577,8 +586,7 @@ impl IOCompositor { self.ready_to_save_state, ReadyState::WaitingForConstellationReply ); - if is_ready && !self.waiting_on_pending_frame && !self.waiting_for_results_of_scroll - { + if is_ready && self.pending_frames == 0 { self.ready_to_save_state = ReadyState::ReadyToSaveImage; if self.is_running_problem_test { println!("ready to save image!"); @@ -607,17 +615,19 @@ impl IOCompositor { }, ( - CompositorMsg::NewScrollFrameReady(recomposite_needed), + CompositorMsg::NewWebRenderFrameReady(recomposite_needed), ShutdownState::NotShuttingDown, ) => { - self.waiting_for_results_of_scroll = false; - if let Some(result) = self.hit_test_at_device_point(self.cursor_pos) { - self.update_cursor(result); - } if recomposite_needed { - self.composition_request = CompositionRequest::CompositeNow( - CompositingReason::NewWebRenderScrollFrame, - ); + if let Some(result) = self.hit_test_at_device_point(self.cursor_pos) { + self.update_cursor(result); + } + self.composition_request = + CompositionRequest::CompositeNow(CompositingReason::NewWebRenderFrame); + } + + if recomposite_needed || self.animation_callbacks_active() { + self.composite_if_necessary(CompositingReason::NewWebRenderFrame) } }, @@ -700,7 +710,6 @@ impl IOCompositor { ForwardedToCompositorMsg::Layout( script_traits::ScriptToCompositorMsg::SendInitialTransaction(pipeline), ) => { - self.waiting_on_pending_frame = true; let mut txn = Transaction::new(); txn.set_display_list( WebRenderEpoch(0), @@ -710,6 +719,7 @@ impl IOCompositor { false, ); + self.generate_frame(&mut txn); self.webrender_api .send_transaction(self.webrender_document, txn); }, @@ -717,11 +727,9 @@ impl IOCompositor { ForwardedToCompositorMsg::Layout( script_traits::ScriptToCompositorMsg::SendScrollNode(point, scroll_id), ) => { - self.waiting_for_results_of_scroll = true; - let mut txn = Transaction::new(); txn.scroll_node_with_id(point, scroll_id, ScrollClamping::NoClamping); - txn.generate_frame(0); + self.generate_frame(&mut txn); self.webrender_api .send_transaction(self.webrender_document, txn); }, @@ -738,8 +746,6 @@ impl IOCompositor { _ => return warn!("Could not recieve WebRender display list."), }; - self.waiting_on_pending_frame = true; - let pipeline_id = display_list_info.pipeline_id; let details = self.pipeline_details(PipelineId::from_webrender(pipeline_id)); details.most_recent_display_list_epoch = Some(display_list_info.epoch); @@ -757,7 +763,7 @@ impl IOCompositor { ), true, ); - txn.generate_frame(0); + self.generate_frame(&mut txn); self.webrender_api .send_transaction(self.webrender_document, txn); }, @@ -879,6 +885,12 @@ impl IOCompositor { } } + /// Queue a new frame in the transaction and increase the pending frames count. + fn generate_frame(&mut self, transaction: &mut Transaction) { + self.pending_frames += 1; + transaction.generate_frame(0); + } + /// Sets or unsets the animations-running flag for the given pipeline, and schedules a /// recomposite if necessary. fn change_running_animations_state( @@ -1011,7 +1023,7 @@ impl IOCompositor { let mut txn = Transaction::new(); self.set_root_content_pipeline_handling_pinch_zoom(&mut txn); - txn.generate_frame(0); + self.generate_frame(&mut txn); self.webrender_api .send_transaction(self.webrender_document, txn); @@ -1449,10 +1461,9 @@ impl IOCompositor { let scroll_origin = LayoutPoint::new(-offset.x, -offset.y); transaction.scroll_node_with_id(scroll_origin, external_id, ScrollClamping::NoClamping); self.send_scroll_positions_to_layout_for_pipeline(&pipeline_id); - self.waiting_for_results_of_scroll = true } - transaction.generate_frame(0); + self.generate_frame(&mut transaction); self.webrender_api .send_transaction(self.webrender_document, transaction); } @@ -1630,6 +1641,13 @@ impl IOCompositor { false } + /// Returns true if any animation callbacks (ie `requestAnimationFrame`) are waiting for a response. + fn animation_callbacks_active(&self) -> bool { + self.pipeline_details + .values() + .any(|details| details.animation_callbacks_running) + } + /// Query the constellation to see if the current compositor /// output matches the current frame tree output, and if the /// associated script threads are idle. @@ -1930,7 +1948,6 @@ impl IOCompositor { self.composition_request = CompositionRequest::NoCompositingNecessary; self.process_animations(); - self.waiting_for_results_of_scroll = false; Ok(rv) } @@ -2013,8 +2030,12 @@ impl IOCompositor { let mut found_recomposite_msg = false; while let Some(msg) = self.port.try_recv_compositor_msg() { match msg { - CompositorMsg::Recomposite(_) if found_recomposite_msg => {}, - CompositorMsg::Recomposite(_) => { + CompositorMsg::NewWebRenderFrameReady(_) if found_recomposite_msg => { + // Only take one of duplicate NewWebRendeFrameReady messages, but do subtract + // one frame from the pending frames. + self.pending_frames -= 1; + }, + CompositorMsg::NewWebRenderFrameReady(_) => { found_recomposite_msg = true; compositor_messages.push(msg) }, @@ -2054,7 +2075,7 @@ impl IOCompositor { // The WebXR thread may make a different context current let _ = self.rendering_context.make_gl_context_current(); - if !self.pending_scroll_zoom_events.is_empty() && !self.waiting_for_results_of_scroll { + if !self.pending_scroll_zoom_events.is_empty() { self.process_pending_scroll_events() } self.shutdown_state != ShutdownState::FinishedShuttingDown @@ -2068,7 +2089,7 @@ impl IOCompositor { while self.shutdown_state != ShutdownState::ShuttingDown { let msg = self.port.recv_compositor_msg(); let need_recomposite = match msg { - CompositorMsg::Recomposite(_) => true, + CompositorMsg::NewWebRenderFrameReady(_) => true, _ => false, }; let keep_going = self.handle_browser_message(msg); @@ -2113,7 +2134,7 @@ impl IOCompositor { self.webrender.set_debug_flags(flags); let mut txn = Transaction::new(); - txn.generate_frame(0); + self.generate_frame(&mut txn); self.webrender_api .send_transaction(self.webrender_document, txn); } diff --git a/components/servo/lib.rs b/components/servo/lib.rs index c21c6af4ede..4b78eac3106 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -33,8 +33,8 @@ use canvas_traits::webgl::WebGLThreads; use compositing::windowing::{EmbedderEvent, EmbedderMethods, WindowMethods}; use compositing::{CompositeTarget, IOCompositor, InitialCompositorState, ShutdownState}; use compositing_traits::{ - CanvasToCompositorMsg, CompositingReason, CompositorMsg, CompositorProxy, CompositorReceiver, - ConstellationMsg, FontToCompositorMsg, ForwardedToCompositorMsg, + CanvasToCompositorMsg, CompositorMsg, CompositorProxy, CompositorReceiver, ConstellationMsg, + FontToCompositorMsg, ForwardedToCompositorMsg, }; #[cfg(all( not(target_os = "windows"), @@ -198,26 +198,17 @@ impl webrender_api::RenderNotifier for RenderNotifier { Box::new(RenderNotifier::new(self.compositor_proxy.clone())) } - fn wake_up(&self, composite_needed: bool) { - if composite_needed { - self.compositor_proxy - .recomposite(CompositingReason::NewWebRenderFrame); - } - } + fn wake_up(&self, _composite_needed: bool) {} fn new_frame_ready( &self, _document_id: DocumentId, - scrolled: bool, + _scrolled: bool, composite_needed: bool, _render_time_ns: Option, ) { - if scrolled { - self.compositor_proxy - .send(CompositorMsg::NewScrollFrameReady(composite_needed)); - } else { - self.wake_up(true); - } + self.compositor_proxy + .send(CompositorMsg::NewWebRenderFrameReady(composite_needed)); } } diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index b3810344439..a64f1a83eb3 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -27,33 +27,6 @@ use style_traits::CSSPixel; use webrender_api::units::{DeviceIntPoint, DeviceIntSize}; use webrender_api::{self, FontInstanceKey, FontKey, ImageKey}; -/// Why we performed a composite. This is used for debugging. -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum CompositingReason { - /// We hit the delayed composition timeout. (See `delayed_composition.rs`.) - DelayedCompositeTimeout, - /// The window has been scrolled and we're starting the first recomposite. - Scroll, - /// A scroll has continued and we need to recomposite again. - ContinueScroll, - /// We're performing the single composite in headless mode. - Headless, - /// We're performing a composite to run an animation. - Animation, - /// A new frame tree has been loaded. - NewFrameTree, - /// New painted buffers have been received. - NewPaintedBuffers, - /// The window has been zoomed. - Zoom, - /// A new WebRender frame has arrived. - NewWebRenderFrame, - /// WebRender has processed a scroll event and has generated a new frame. - NewWebRenderScrollFrame, - /// The window has been resized and will need to be synchronously repainted. - Resize, -} - /// Sends messages to the compositor. pub struct CompositorProxy { pub sender: Sender, @@ -92,12 +65,6 @@ impl CompositorReceiver { } } -impl CompositorProxy { - pub fn recomposite(&self, reason: CompositingReason) { - self.send(CompositorMsg::Recomposite(reason)); - } -} - /// Messages from (or via) the constellation thread to the compositor. pub enum CompositorMsg { /// Informs the compositor that the constellation has completed shutdown. @@ -108,8 +75,6 @@ pub enum CompositorMsg { ChangeRunningAnimationsState(PipelineId, AnimationState), /// Replaces the current frame tree, typically called during main frame navigation. SetFrameTree(SendableFrameTree), - /// Composite. - Recomposite(CompositingReason), /// Script has handled a touch event, and either prevented or allowed default actions. TouchEventProcessed(EventResult), /// Composite to a PNG file and return the Image over a passed channel. @@ -118,9 +83,9 @@ pub enum CompositorMsg { IsReadyToSaveImageReply(bool), /// Pipeline visibility changed PipelineVisibilityChanged(PipelineId, bool), - /// WebRender has successfully processed a scroll. The boolean specifies whether a composite is - /// needed. - NewScrollFrameReady(bool), + /// WebRender has produced a new frame. This message informs the compositor that + /// the frame is ready, so that it may trigger a recomposite. + NewWebRenderFrameReady(bool /* composite_needed */), /// A pipeline was shut down. // This message acts as a synchronization point between the constellation, // when it shuts down a pipeline, to the compositor; when the compositor @@ -193,13 +158,12 @@ impl Debug for CompositorMsg { write!(f, "ChangeRunningAnimationsState({:?})", state) }, CompositorMsg::SetFrameTree(..) => write!(f, "SetFrameTree"), - CompositorMsg::Recomposite(..) => write!(f, "Recomposite"), CompositorMsg::TouchEventProcessed(..) => write!(f, "TouchEventProcessed"), CompositorMsg::CreatePng(..) => write!(f, "CreatePng"), CompositorMsg::IsReadyToSaveImageReply(..) => write!(f, "IsReadyToSaveImageReply"), CompositorMsg::PipelineVisibilityChanged(..) => write!(f, "PipelineVisibilityChanged"), CompositorMsg::PipelineExited(..) => write!(f, "PipelineExited"), - CompositorMsg::NewScrollFrameReady(..) => write!(f, "NewScrollFrameReady"), + CompositorMsg::NewWebRenderFrameReady(..) => write!(f, "NewWebRenderFrameReady"), CompositorMsg::Dispatch(..) => write!(f, "Dispatch"), CompositorMsg::PendingPaintMetric(..) => write!(f, "PendingPaintMetric"), CompositorMsg::LoadComplete(..) => write!(f, "LoadComplete"),