From 6dd4df985204a1b922d343026cdb596d9be1fddb Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 26 Sep 2025 18:12:12 +0200 Subject: [PATCH] Base waiting on frames --- components/compositing/compositor.rs | 136 ++++++++++++++++----------- components/servo/lib.rs | 3 +- components/shared/compositing/lib.rs | 2 +- 3 files changed, 83 insertions(+), 58 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 34dc0650422..06652e046a5 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -132,8 +132,8 @@ pub struct IOCompositor { /// The [`RenderingContext`] instance that webrender targets, which is the viewport. rendering_context: Rc, - /// The number of frames pending to receive from WebRender. - pending_frames: Cell, + next_frame_id: Cell, + current_frame: Cell, /// A handle to the memory profiler which will automatically unregister /// when it's dropped. @@ -206,6 +206,8 @@ pub(crate) struct PipelineDetails { /// Which parts of Servo have reported that this `Pipeline` has exited. Only when all /// have done so will it be discarded. pub exited: PipelineExitSource, + + pub display_list_epoch: Option, } impl PipelineDetails { @@ -231,6 +233,7 @@ impl PipelineDetails { first_paint_metric: PaintMetricState::Waiting, first_contentful_paint_metric: PaintMetricState::Waiting, exited: PipelineExitSource::empty(), + display_list_epoch: None, } } @@ -316,7 +319,8 @@ impl IOCompositor { needs_repaint: Cell::default(), webrender: Some(state.webrender), rendering_context: state.rendering_context, - pending_frames: Cell::new(0), + next_frame_id: Default::default(), + current_frame: Default::default(), _mem_profiler_registration: registration, }; @@ -500,7 +504,7 @@ impl IOCompositor { } }, - CompositorMsg::NewWebRenderFrameReady(_document_id, recomposite_needed) => { + CompositorMsg::NewWebRenderFrameReady(_document_id, _, recomposite_needed) => { self.handle_new_webrender_frame_ready(recomposite_needed); }, @@ -620,6 +624,8 @@ impl IOCompositor { Some(display_list_info.viewport_details.hidpi_scale_factor); let epoch = display_list_info.epoch; + details.display_list_epoch = Some(Epoch(epoch.0)); + let first_reflow = display_list_info.first_reflow; if details.first_paint_metric == PaintMetricState::Waiting { details.first_paint_metric = PaintMetricState::Seen(epoch, first_reflow); @@ -646,9 +652,10 @@ impl IOCompositor { }, CompositorMsg::GenerateFrame => { + self.prepare_screenshot_requests_for_render(); + let mut global = self.global.borrow_mut(); global.frame_delayer.set_pending_frame(true); - if global.frame_delayer.needs_new_frame() { let mut transaction = Transaction::new(); self.generate_frame(&mut transaction, RenderReasons::SCENE); @@ -811,10 +818,6 @@ impl IOCompositor { result_sender, ); }, - CompositorMsg::NewWebRenderFrameReady(..) => { - // Subtract from the number of pending frames, but do not do any compositing. - self.pending_frames.set(self.pending_frames.get() - 1); - }, _ => { debug!("Ignoring message ({:?} while shutting down", msg); }, @@ -844,8 +847,9 @@ impl IOCompositor { /// Queue a new frame in the transaction and increase the pending frames count. pub(crate) fn generate_frame(&self, transaction: &mut Transaction, reason: RenderReasons) { - self.pending_frames.set(self.pending_frames.get() + 1); - transaction.generate_frame(0, true /* present */, reason); + let next_frame_id = self.next_frame_id.get(); + transaction.generate_frame(next_frame_id, true /* present */, reason); + self.next_frame_id.set(next_frame_id); } /// Set the root pipeline for our WebRender scene to a display list that consists of an iframe @@ -1347,20 +1351,17 @@ impl IOCompositor { pub fn handle_messages(&mut self, mut messages: Vec) { // Check for new messages coming from the other threads in the system. let mut found_recomposite_msg = false; - messages.retain(|message| { - match message { - 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.set(self.pending_frames.get() - 1); - false - }, - CompositorMsg::NewWebRenderFrameReady(..) => { - found_recomposite_msg = true; - true - }, - _ => true, - } + messages.retain(|message| match message { + CompositorMsg::NewWebRenderFrameReady(_, frame_id, _) if found_recomposite_msg => { + self.current_frame.set(*frame_id); + false + }, + CompositorMsg::NewWebRenderFrameReady(_, frame_id, _) => { + self.current_frame.set(*frame_id); + found_recomposite_msg = true; + true + }, + _ => true, }); for message in messages { @@ -1570,7 +1571,6 @@ impl IOCompositor { } fn handle_new_webrender_frame_ready(&mut self, recomposite_needed: bool) { - self.pending_frames.set(self.pending_frames.get() - 1); if recomposite_needed { self.refresh_cursor(); } @@ -1600,24 +1600,63 @@ impl IOCompositor { webview_id: WebViewId, expected_epochs: FxHashMap, ) { - let mut global = self.global.borrow_mut(); - let expected_epochs = Rc::new(expected_epochs); + { + let mut global = self.global.borrow_mut(); + let expected_epochs = Rc::new(expected_epochs); + for screenshot_request in global.screenshot_requests.iter_mut() { + if screenshot_request.webview_id != webview_id || + screenshot_request.state != ScreenshotRequestState::WaitingOnConstellation + { + continue; + } + screenshot_request.state = + ScreenshotRequestState::WaitingOnPipelines(expected_epochs.clone()); + } + } + + if self.prepare_screenshot_requests_for_render() { + let mut transaction = Transaction::new(); + self.generate_frame(&mut transaction, RenderReasons::APZ); + self.global.borrow_mut().send_transaction(transaction); + } + } + + fn prepare_screenshot_requests_for_render(&self) -> bool { + let mut global = self.global.borrow_mut(); + let next_frame_id = self.next_frame_id.get(); let mut any_became_ready = false; + for screenshot_request in global.screenshot_requests.iter_mut() { - if screenshot_request.webview_id != webview_id || - screenshot_request.state != ScreenshotRequestState::WaitingOnConstellation - { + let ScreenshotRequestState::WaitingOnPipelines(pipelines) = &screenshot_request.state + else { + continue; + }; + + let Some(webview) = self.webview_renderers.get(screenshot_request.webview_id) else { + continue; + }; + + let ready = pipelines.iter().all(|(pipeline_id, expected_epoch)| { + let Some(epoch) = webview + .pipelines + .get(pipeline_id) + .and_then(|pipeline| pipeline.display_list_epoch) + else { + return false; + }; + epoch >= *expected_epoch + }); + + if !ready { continue; } - screenshot_request.state = - ScreenshotRequestState::WaitingOnWebRender(expected_epochs.clone()); + + screenshot_request.state = ScreenshotRequestState::WaitingOnFrame(next_frame_id); any_became_ready = true; } - if any_became_ready { - self.set_needs_repaint(RepaintReason::ReadyForScreenshot); - } + any_became_ready } fn create_screenshots_after_paint(&self) { @@ -1626,17 +1665,12 @@ impl IOCompositor { return; } - let document_id = global.webrender_document; - let Some(webrender) = self.webrender.as_ref() else { - return; - }; - // TODO: This can eventually just be `extract_if`. We need to have ownership // of the ScreenshotRequest in order to call the `FnOnce` callabck. let screenshots = global.screenshot_requests.drain(..); global.screenshot_requests = screenshots .filter_map(|screenshot_request| { - if !screenshot_request.screenshot_ready(webrender, &document_id) { + if !screenshot_request.screenshot_ready(self.current_frame.get()) { return Some(screenshot_request); } @@ -1742,22 +1776,12 @@ struct ScreenshotRequest { #[derive(PartialEq)] enum ScreenshotRequestState { WaitingOnConstellation, - WaitingOnWebRender(Rc>), + WaitingOnPipelines(Rc>), + WaitingOnFrame(u64), } impl ScreenshotRequest { - fn screenshot_ready(&self, webrender: &webrender::Renderer, &document_id: &DocumentId) -> bool { - let ScreenshotRequestState::WaitingOnWebRender(pipelines_and_epochs) = &self.state else { - return false; - }; - pipelines_and_epochs - .iter() - .all(|(pipeline_id, necessary_epoch)| { - webrender - .current_epoch(document_id, pipeline_id.into()) - .is_some_and(|rendered_epoch| { - rendered_epoch >= WebRenderEpoch(necessary_epoch.0) - }) - }) + fn screenshot_ready(&self, current_frame_id: u64) -> bool { + matches!(self.state,ScreenshotRequestState::WaitingOnFrame(frame_id) if frame_id <= current_frame_id) } } diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 16f2acb80ab..123edcf32e7 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -241,12 +241,13 @@ impl webrender_api::RenderNotifier for RenderNotifier { fn new_frame_ready( &self, document_id: DocumentId, - _: FramePublishId, + frame_id: FramePublishId, frame_ready_params: &FrameReadyParams, ) { self.compositor_proxy .send(CompositorMsg::NewWebRenderFrameReady( document_id, + frame_id.0, frame_ready_params.render, )); } diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index 0b7971dd3bb..cec256186da 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -93,7 +93,7 @@ pub enum CompositorMsg { /// WebRender has produced a new frame. This message informs the compositor that /// the frame is ready. It contains a bool to indicate if it needs to composite and the /// `DocumentId` of the new frame. - NewWebRenderFrameReady(DocumentId, bool), + NewWebRenderFrameReady(DocumentId, u64, bool), /// Script or the Constellation is notifying the renderer that a Pipeline has finished /// shutting down. The renderer will not discard the Pipeline until both report that /// they have fully shut it down, to avoid recreating it due to any subsequent