Correct the frame timing in 'Rasterizer::Draw' when pipeline is more available (flutter#32283)

Author: ColdPaleLight

shell/common/animator.cc

@@ -163,26 +163,25 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
   delegate_.OnAnimatorUpdateLatestFrameTargetTime(
       frame_timings_recorder_->GetVsyncTargetTime());
 
+  auto layer_tree_item = std::make_unique<LayerTreeItem>(
+      std::move(layer_tree), std::move(frame_timings_recorder_));
   // Commit the pending continuation.
   PipelineProduceResult result =
-      producer_continuation_.Complete(std::move(layer_tree));
+      producer_continuation_.Complete(std::move(layer_tree_item));
 
   if (!result.success) {
-    frame_timings_recorder_.reset();
     FML_DLOG(INFO) << "No pending continuation to commit";
     return;
   }
 
   if (!result.is_first_item) {
-    frame_timings_recorder_.reset();
     // It has been successfully pushed to the pipeline but not as the first
     // item. Eventually the 'Rasterizer' will consume it, so we don't need to
     // notify the delegate.
     return;
   }
 
-  delegate_.OnAnimatorDraw(layer_tree_pipeline_,
-                           std::move(frame_timings_recorder_));
+  delegate_.OnAnimatorDraw(layer_tree_pipeline_);
 }
 
 const std::weak_ptr<VsyncWaiter> Animator::GetVsyncWaiter() const {

shell/common/animator.h

@@ -40,8 +40,7 @@ class Animator final {
         fml::TimePoint frame_target_time) = 0;
 
     virtual void OnAnimatorDraw(
-        std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
-        std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) = 0;
+        std::shared_ptr<LayerTreePipeline> pipeline) = 0;
 
     virtual void OnAnimatorDrawLastLayerTree(
         std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) = 0;
@@ -83,8 +82,6 @@ class Animator final {
   void EnqueueTraceFlowId(uint64_t trace_flow_id);
 
  private:
-  using LayerTreePipeline = Pipeline<flutter::LayerTree>;
-
   void BeginFrame(std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder);
 
   bool CanReuseLastLayerTree();

shell/common/animator_unittests.cc

@@ -35,10 +35,8 @@ class FakeAnimatorDelegate : public Animator::Delegate {
   MOCK_METHOD1(OnAnimatorUpdateLatestFrameTargetTime,
                void(fml::TimePoint frame_target_time));
 
-  MOCK_METHOD2(
-      OnAnimatorDraw,
-      void(std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
-           std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder));
+  MOCK_METHOD1(OnAnimatorDraw,
+               void(std::shared_ptr<LayerTreePipeline> pipeline));
 
   void OnAnimatorDrawLastLayerTree(
       std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) override {}

shell/common/pipeline.h

@@ -9,6 +9,8 @@
 #include <memory>
 #include <mutex>
 
+#include "flutter/flow/frame_timings.h"
+#include "flutter/flow/layers/layer_tree.h"
 #include "flutter/fml/macros.h"
 #include "flutter/fml/memory/ref_counted.h"
 #include "flutter/fml/synchronization/semaphore.h"
@@ -219,6 +221,17 @@ class Pipeline {
   FML_DISALLOW_COPY_AND_ASSIGN(Pipeline);
 };
 
+struct LayerTreeItem {
+  LayerTreeItem(std::unique_ptr<LayerTree> layer_tree,
+                std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder)
+      : layer_tree(std::move(layer_tree)),
+        frame_timings_recorder(std::move(frame_timings_recorder)) {}
+  std::unique_ptr<LayerTree> layer_tree;
+  std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder;
+};
+
+using LayerTreePipeline = Pipeline<LayerTreeItem>;
+
 }  // namespace flutter
 
 #endif  // FLUTTER_SHELL_COMMON_PIPELINE_H_

shell/common/rasterizer.cc

@@ -160,12 +160,9 @@ void Rasterizer::DrawLastLayerTree(
   }
 }
 
-RasterStatus Rasterizer::Draw(
-    std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
-    std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
-    LayerTreeDiscardCallback discard_callback) {
-  TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder, "flutter",
-                                "GPURasterizer::Draw");
+RasterStatus Rasterizer::Draw(std::shared_ptr<LayerTreePipeline> pipeline,
+                              LayerTreeDiscardCallback discard_callback) {
+  TRACE_EVENT0("flutter", "GPURasterizer::Draw");
   if (raster_thread_merger_ &&
       !raster_thread_merger_->IsOnRasterizingThread()) {
     // we yield and let this frame be serviced on the right thread.
@@ -175,13 +172,12 @@ RasterStatus Rasterizer::Draw(
                  .GetRasterTaskRunner()
                  ->RunsTasksOnCurrentThread());
 
-  std::unique_ptr<FrameTimingsRecorder> resubmit_recorder =
-      frame_timings_recorder->CloneUntil(
-          FrameTimingsRecorder::State::kBuildEnd);
-
   RasterStatus raster_status = RasterStatus::kFailed;
-  Pipeline<flutter::LayerTree>::Consumer consumer =
-      [&](std::unique_ptr<LayerTree> layer_tree) {
+  LayerTreePipeline::Consumer consumer =
+      [&](std::unique_ptr<LayerTreeItem> item) {
+        std::unique_ptr<LayerTree> layer_tree = std::move(item->layer_tree);
+        std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder =
+            std::move(item->frame_timings_recorder);
         if (discard_callback(*layer_tree.get())) {
           raster_status = RasterStatus::kDiscarded;
         } else {
@@ -199,9 +195,11 @@ RasterStatus Rasterizer::Draw(
 
   bool should_resubmit_frame = ShouldResubmitFrame(raster_status);
   if (should_resubmit_frame) {
+    auto resubmitted_layer_tree_item = std::make_unique<LayerTreeItem>(
+        std::move(resubmitted_layer_tree_), std::move(resubmitted_recorder_));
     auto front_continuation = pipeline->ProduceIfEmpty();
     PipelineProduceResult result =
-        front_continuation.Complete(std::move(resubmitted_layer_tree_));
+        front_continuation.Complete(std::move(resubmitted_layer_tree_item));
     if (result.success) {
       consume_result = PipelineConsumeResult::MoreAvailable;
     }
@@ -224,11 +222,9 @@ RasterStatus Rasterizer::Draw(
       delegate_.GetTaskRunners().GetRasterTaskRunner()->PostTask(
           fml::MakeCopyable(
               [weak_this = weak_factory_.GetWeakPtr(), pipeline,
-               resubmit_recorder = std::move(resubmit_recorder),
                discard_callback = std::move(discard_callback)]() mutable {
                 if (weak_this) {
-                  weak_this->Draw(std::move(resubmit_recorder), pipeline,
-                                  std::move(discard_callback));
+                  weak_this->Draw(pipeline, std::move(discard_callback));
                 }
               }));
       break;
@@ -395,6 +391,8 @@ fml::Milliseconds Rasterizer::GetFrameBudget() const {
 RasterStatus Rasterizer::DoDraw(
     std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
     std::unique_ptr<flutter::LayerTree> layer_tree) {
+  TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder, "flutter",
+                                "Rasterizer::DoDraw");
   FML_DCHECK(delegate_.GetTaskRunners()
                  .GetRasterTaskRunner()
                  ->RunsTasksOnCurrentThread());
@@ -412,6 +410,8 @@ RasterStatus Rasterizer::DoDraw(
     last_layer_tree_ = std::move(layer_tree);
   } else if (ShouldResubmitFrame(raster_status)) {
     resubmitted_layer_tree_ = std::move(layer_tree);
+    resubmitted_recorder_ = frame_timings_recorder->CloneUntil(
+        FrameTimingsRecorder::State::kBuildEnd);
     return raster_status;
   } else if (raster_status == RasterStatus::kDiscarded) {
     return raster_status;

shell/common/rasterizer.h

@@ -241,10 +241,8 @@ class Rasterizer final : public SnapshotDelegate,
   /// @param[in]  discard_callback if specified and returns true, the layer tree
   ///                             is discarded instead of being rendered
   ///
-  RasterStatus Draw(
-      std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
-      std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
-      LayerTreeDiscardCallback discard_callback = NoDiscard);
+  RasterStatus Draw(std::shared_ptr<LayerTreePipeline> pipeline,
+                    LayerTreeDiscardCallback discard_callback = NoDiscard);
 
   //----------------------------------------------------------------------------
   /// @brief      The type of the screenshot to obtain of the previously
@@ -509,6 +507,7 @@ class Rasterizer final : public SnapshotDelegate,
   // has not successfully rasterized. This can happen due to the change in the
   // thread configuration. This will be inserted to the front of the pipeline.
   std::unique_ptr<flutter::LayerTree> resubmitted_layer_tree_;
+  std::unique_ptr<FrameTimingsRecorder> resubmitted_recorder_;
   fml::closure next_frame_callback_;
   bool user_override_resource_cache_bytes_;
   std::optional<size_t> max_cache_bytes_;