From e555fa0b81e6af760e5fc55b8bcee10e8b56d242 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 18 Apr 2025 08:21:58 +0000 Subject: [PATCH] Bug 1961115 - Avoid scheduling an extra frame when a transaction is rendered immediately. r=gfx-reviewers,lsalzman In addition, rename NotifyDidSceneBuild into ScheduleFrameAfterSceneBuild. Right now it is only used for that purpose but it would cause issues if we start relying on it to be called after all scene builds since with this patch we won't call it if we do not need a new frame. Differential Revision: https://phabricator.services.mozilla.com/D245898 --- gfx/layers/ipc/CompositorBridgeParent.cpp | 4 ++-- gfx/layers/ipc/CompositorBridgeParent.h | 3 ++- gfx/layers/wr/WebRenderBridgeParent.cpp | 2 +- gfx/layers/wr/WebRenderBridgeParent.h | 3 ++- gfx/webrender_bindings/RenderThread.cpp | 14 ++++++++------ gfx/webrender_bindings/src/bindings.rs | 11 +++++------ gfx/wr/webrender/src/renderer/init.rs | 2 +- gfx/wr/webrender/src/scene_builder_thread.rs | 10 +++++++++- 8 files changed, 30 insertions(+), 19 deletions(-) diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp index dd5e96a72206..baca0b8ba1a3 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CompositorBridgeParent.cpp @@ -1439,7 +1439,7 @@ CompositorBridgeParent::LayerTreeState::GetCompositorController() const { return mParent; } -void CompositorBridgeParent::NotifyDidSceneBuild( +void CompositorBridgeParent::ScheduleFrameAfterSceneBuild( RefPtr aInfo) { MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread()); if (mPaused) { @@ -1447,7 +1447,7 @@ void CompositorBridgeParent::NotifyDidSceneBuild( } if (mWrBridge) { - mWrBridge->NotifyDidSceneBuild(aInfo); + mWrBridge->ScheduleFrameAfterSceneBuild(aInfo); } } diff --git a/gfx/layers/ipc/CompositorBridgeParent.h b/gfx/layers/ipc/CompositorBridgeParent.h index 735aa743bdf1..dcc26c4bedb5 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.h +++ b/gfx/layers/ipc/CompositorBridgeParent.h @@ -334,7 +334,8 @@ class CompositorBridgeParent final : public CompositorBridgeParentBase, TimeStamp& aCompositeStart, TimeStamp& aRenderStart, TimeStamp& aCompositeEnd, wr::RendererStats* aStats = nullptr); - void NotifyDidSceneBuild(RefPtr aInfo); + void ScheduleFrameAfterSceneBuild( + RefPtr aInfo); RefPtr GetAsyncImagePipelineManager() const; PCompositorWidgetParent* AllocPCompositorWidgetParent( diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp index ef78f2b7b628..52016224c623 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.cpp +++ b/gfx/layers/wr/WebRenderBridgeParent.cpp @@ -2526,7 +2526,7 @@ void WebRenderBridgeParent::NotifySceneBuiltForEpoch( } } -void WebRenderBridgeParent::NotifyDidSceneBuild( +void WebRenderBridgeParent::ScheduleFrameAfterSceneBuild( RefPtr aInfo) { MOZ_ASSERT(IsRootWebRenderBridgeParent()); if (!mCompositorScheduler) { diff --git a/gfx/layers/wr/WebRenderBridgeParent.h b/gfx/layers/wr/WebRenderBridgeParent.h index 42a4451ae424..ecef1b812b01 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.h +++ b/gfx/layers/wr/WebRenderBridgeParent.h @@ -279,7 +279,8 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent, */ void ScheduleForcedGenerateFrame(wr::RenderReasons aReasons); - void NotifyDidSceneBuild(RefPtr aInfo); + void ScheduleFrameAfterSceneBuild( + RefPtr aInfo); wr::Epoch UpdateWebRender( CompositorVsyncScheduler* aScheduler, RefPtr&& aApi, diff --git a/gfx/webrender_bindings/RenderThread.cpp b/gfx/webrender_bindings/RenderThread.cpp index dadfc85bbf7c..2232c8013717 100644 --- a/gfx/webrender_bindings/RenderThread.cpp +++ b/gfx/webrender_bindings/RenderThread.cpp @@ -1771,22 +1771,24 @@ void wr_schedule_render(mozilla::wr::WrWindowId aWindowId, "NotifyScheduleRender", &NotifyScheduleRender, aWindowId, aReasons)); } -static void NotifyDidSceneBuild( +static void ScheduleFrameAfterSceneBuild( mozilla::wr::WrWindowId aWindowId, const RefPtr& aInfo) { RefPtr cbp = mozilla::layers:: CompositorBridgeParent::GetCompositorBridgeParentFromWindowId(aWindowId); if (cbp) { - cbp->NotifyDidSceneBuild(aInfo); + cbp->ScheduleFrameAfterSceneBuild(aInfo); } } -void wr_finished_scene_build(mozilla::wr::WrWindowId aWindowId, - mozilla::wr::WrPipelineInfo* aPipelineInfo) { +void wr_schedule_frame_after_scene_build( + mozilla::wr::WrWindowId aWindowId, + mozilla::wr::WrPipelineInfo* aPipelineInfo) { RefPtr info = new wr::WebRenderPipelineInfo(); info->Raw() = std::move(*aPipelineInfo); - layers::CompositorThread()->Dispatch(NewRunnableFunction( - "NotifyDidSceneBuild", &NotifyDidSceneBuild, aWindowId, info)); + layers::CompositorThread()->Dispatch( + NewRunnableFunction("ScheduleFrameAfterSceneBuild", + &ScheduleFrameAfterSceneBuild, aWindowId, info)); } } // extern C diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs index b3ff2c5a8fb7..e4dfb9163fbf 100644 --- a/gfx/webrender_bindings/src/bindings.rs +++ b/gfx/webrender_bindings/src/bindings.rs @@ -561,7 +561,7 @@ extern "C" { fn wr_notifier_external_event(window_id: WrWindowId, raw_event: usize); fn wr_schedule_render(window_id: WrWindowId, reasons: RenderReasons); // NOTE: This moves away from pipeline_info. - fn wr_finished_scene_build(window_id: WrWindowId, pipeline_info: &mut WrPipelineInfo); + fn wr_schedule_frame_after_scene_build(window_id: WrWindowId, pipeline_info: &mut WrPipelineInfo); fn wr_transaction_notification_notified(handler: usize, when: Checkpoint); } @@ -1028,16 +1028,15 @@ impl SceneBuilderHooks for APZCallbacks { } } - fn post_scene_swap(&self, _document_ids: &Vec, info: PipelineInfo) { + fn post_scene_swap(&self, _document_ids: &Vec, info: PipelineInfo, schedule_frame: bool) { let mut info = WrPipelineInfo::new(&info); unsafe { apz_post_scene_swap(self.window_id, &info); } - // After a scene swap we should schedule a render for the next vsync, - // otherwise there's no guarantee that the new scene will get rendered - // anytime soon - unsafe { wr_finished_scene_build(self.window_id, &mut info) } + if schedule_frame { + unsafe { wr_schedule_frame_after_scene_build(self.window_id, &mut info) } + } gecko_profiler_end_marker("SceneBuilding"); } diff --git a/gfx/wr/webrender/src/renderer/init.rs b/gfx/wr/webrender/src/renderer/init.rs index 7a503a2611fd..f8df42c1e926 100644 --- a/gfx/wr/webrender/src/renderer/init.rs +++ b/gfx/wr/webrender/src/renderer/init.rs @@ -76,7 +76,7 @@ pub trait SceneBuilderHooks { /// This is called after each scene swap occurs. The PipelineInfo contains /// the updated epochs and pipelines removed in the new scene compared to /// the old scene. - fn post_scene_swap(&self, document_id: &Vec, info: PipelineInfo); + fn post_scene_swap(&self, document_id: &Vec, info: PipelineInfo, schedule_frame: bool); /// This is called after a resource update operation on the scene builder /// thread, in the case where resource updates were applied without a scene /// build. diff --git a/gfx/wr/webrender/src/scene_builder_thread.rs b/gfx/wr/webrender/src/scene_builder_thread.rs index a7bd3cfc8a06..ed8ef7187fa8 100644 --- a/gfx/wr/webrender/src/scene_builder_thread.rs +++ b/gfx/wr/webrender/src/scene_builder_thread.rs @@ -726,6 +726,13 @@ impl SceneBuilderThread { Vec::new() }; + // Unless a transaction generates a frame immediately, the compositor should + // schedule one whenever appropriate (probably at the next vsync) to present + // the changes in the scene. + let compositor_should_schedule_a_frame = !txns.iter().any(|txn| { + txn.render_frame + }); + #[cfg(feature = "capture")] match self.capture_config { Some(ref config) => self.send(SceneBuilderResult::CapturedTransactions(txns, config.clone(), result_tx)), @@ -740,7 +747,8 @@ impl SceneBuilderThread { let swap_result = result_rx.unwrap().recv(); Telemetry::stop_and_accumulate_sceneswap_time(timer_id); self.hooks.as_ref().unwrap().post_scene_swap(&document_ids, - pipeline_info); + pipeline_info, + compositor_should_schedule_a_frame); // Once the hook is done, allow the RB thread to resume if let Ok(SceneSwapResult::Complete(resume_tx)) = swap_result { resume_tx.send(()).ok();