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

@@ -1439,7 +1439,7 @@ CompositorBridgeParent::LayerTreeState::GetCompositorController() const {
   return mParent;
 }
 
-void CompositorBridgeParent::NotifyDidSceneBuild(
+void CompositorBridgeParent::ScheduleFrameAfterSceneBuild(
     RefPtr<const wr::WebRenderPipelineInfo> aInfo) {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
   if (mPaused) {
@@ -1447,7 +1447,7 @@ void CompositorBridgeParent::NotifyDidSceneBuild(
   }
 
   if (mWrBridge) {
-    mWrBridge->NotifyDidSceneBuild(aInfo);
+    mWrBridge->ScheduleFrameAfterSceneBuild(aInfo);
   }
 }
 

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<const wr::WebRenderPipelineInfo> aInfo);
+  void ScheduleFrameAfterSceneBuild(
+      RefPtr<const wr::WebRenderPipelineInfo> aInfo);
   RefPtr<AsyncImagePipelineManager> GetAsyncImagePipelineManager() const;
 
   PCompositorWidgetParent* AllocPCompositorWidgetParent(

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -2526,7 +2526,7 @@ void WebRenderBridgeParent::NotifySceneBuiltForEpoch(
   }
 }
 
-void WebRenderBridgeParent::NotifyDidSceneBuild(
+void WebRenderBridgeParent::ScheduleFrameAfterSceneBuild(
     RefPtr<const wr::WebRenderPipelineInfo> aInfo) {
   MOZ_ASSERT(IsRootWebRenderBridgeParent());
   if (!mCompositorScheduler) {

gfx/layers/wr/WebRenderBridgeParent.h

@@ -279,7 +279,8 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent,
    */
   void ScheduleForcedGenerateFrame(wr::RenderReasons aReasons);
 
-  void NotifyDidSceneBuild(RefPtr<const wr::WebRenderPipelineInfo> aInfo);
+  void ScheduleFrameAfterSceneBuild(
+      RefPtr<const wr::WebRenderPipelineInfo> aInfo);
 
   wr::Epoch UpdateWebRender(
       CompositorVsyncScheduler* aScheduler, RefPtr<wr::WebRenderAPI>&& aApi,

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<const wr::WebRenderPipelineInfo>& aInfo) {
   RefPtr<mozilla::layers::CompositorBridgeParent> 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<wr::WebRenderPipelineInfo> 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

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<DocumentId>, info: PipelineInfo) {
+    fn post_scene_swap(&self, _document_ids: &Vec<DocumentId>, 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");
     }
 

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<DocumentId>, info: PipelineInfo);
+    fn post_scene_swap(&self, document_id: &Vec<DocumentId>, 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.

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();