Reland 4: Multiview pipeline (flutter#50931)

This relands flutter#49950.

Nothing is changed. The error turns out a flake (it passes once),
although this PR might have made the flake more flaky.

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

Author: dkwingsmt

flow/frame_timings.cc

@@ -13,6 +13,31 @@
 
 namespace flutter {
 
+namespace {
+
+const char* StateToString(FrameTimingsRecorder::State state) {
+#ifndef NDEBUG
+  switch (state) {
+    case FrameTimingsRecorder::State::kUninitialized:
+      return "kUninitialized";
+    case FrameTimingsRecorder::State::kVsync:
+      return "kVsync";
+    case FrameTimingsRecorder::State::kBuildStart:
+      return "kBuildStart";
+    case FrameTimingsRecorder::State::kBuildEnd:
+      return "kBuildEnd";
+    case FrameTimingsRecorder::State::kRasterStart:
+      return "kRasterStart";
+    case FrameTimingsRecorder::State::kRasterEnd:
+      return "kRasterEnd";
+  };
+  FML_UNREACHABLE();
+#endif
+  return "";
+}
+
+}  // namespace
+
 std::atomic<uint64_t> FrameTimingsRecorder::frame_number_gen_ = {1};
 
 FrameTimingsRecorder::FrameTimingsRecorder()
@@ -255,7 +280,8 @@ const char* FrameTimingsRecorder::GetFrameNumberTraceArg() const {
 }
 
 void FrameTimingsRecorder::AssertInState(State state) const {
-  FML_DCHECK(state_ == state);
+  FML_DCHECK(state_ == state) << "Expected state " << StateToString(state)
+                              << ", actual state " << StateToString(state_);
 }
 
 }  // namespace flutter

flow/frame_timings.h

@@ -31,6 +31,7 @@ class FrameTimingsRecorder {
  public:
   /// Various states that the recorder can be in. When created the recorder is
   /// in an unitialized state and transtions in sequential order of the states.
+  // After adding an item to this enum, modify StateToString accordingly.
   enum class State : uint32_t {
     kUninitialized,
     kVsync,
@@ -121,6 +122,8 @@ class FrameTimingsRecorder {
   ///
   /// Instead of adding a `GetState` method and asserting on the result, this
   /// method prevents other logic from relying on the state.
+  ///
+  /// In release builds, this call is a no-op.
   void AssertInState(State state) const;
 
  private:

lib/ui/painting/image_dispose_unittests.cc

@@ -5,6 +5,7 @@
 #define FML_USED_ON_EMBEDDER
 
 #include "flutter/common/task_runners.h"
+#include "flutter/fml/synchronization/count_down_latch.h"
 #include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/lib/ui/painting/canvas.h"
 #include "flutter/lib/ui/painting/image.h"
@@ -57,6 +58,10 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
   };
 
   Settings settings = CreateSettingsForFixture();
+  fml::CountDownLatch frame_latch{2};
+  settings.frame_rasterized_callback = [&frame_latch](const FrameTiming& t) {
+    frame_latch.CountDown();
+  };
   auto task_runner = CreateNewThread();
   TaskRunners task_runners("test",                  // label
                            GetCurrentTaskRunner(),  // platform
@@ -83,12 +88,15 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
   shell->RunEngine(std::move(configuration), [&](auto result) {
     ASSERT_EQ(result, Engine::RunStatus::Success);
   });
-
   message_latch_.Wait();
 
   ASSERT_TRUE(current_display_list_);
   ASSERT_TRUE(current_image_);
 
+  // Wait for 2 frames to be rasterized. The 2nd frame releases resources of the
+  // 1st frame.
+  frame_latch.Wait();
+
   // Force a drain the SkiaUnrefQueue. The engine does this normally as frames
   // pump, but we force it here to make the test more deterministic.
   message_latch_.Reset();

lib/ui/window/platform_configuration.cc

@@ -453,12 +453,9 @@ void PlatformConfigurationNativeApi::Render(int64_t view_id,
                                             Scene* scene,
                                             double width,
                                             double height) {
-  // TODO(dkwingsmt): Currently only supports a single window.
-  // See https://github.com/flutter/flutter/issues/135530, item 2.
-  FML_DCHECK(view_id == kFlutterImplicitViewId);
   UIDartState::ThrowIfUIOperationsProhibited();
   UIDartState::Current()->platform_configuration()->client()->Render(
-      scene, width, height);
+      view_id, scene, width, height);
 }
 
 void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) {

lib/ui/window/platform_configuration.h

@@ -76,7 +76,10 @@ class PlatformConfigurationClient {
   /// @brief      Updates the client's rendering on the GPU with the newly
   ///             provided Scene.
   ///
-  virtual void Render(Scene* scene, double width, double height) = 0;
+  virtual void Render(int64_t view_id,
+                      Scene* scene,
+                      double width,
+                      double height) = 0;
 
   //--------------------------------------------------------------------------
   /// @brief      Receives an updated semantics tree from the Framework.

runtime/runtime_controller.cc

@@ -340,21 +340,21 @@ void RuntimeController::ScheduleFrame() {
   client_.ScheduleFrame();
 }
 
-// |PlatformConfigurationClient|
 void RuntimeController::EndWarmUpFrame() {
   client_.EndWarmUpFrame();
 }
 
 // |PlatformConfigurationClient|
-void RuntimeController::Render(Scene* scene, double width, double height) {
-  // TODO(dkwingsmt): Currently only supports a single window.
-  int64_t view_id = kFlutterImplicitViewId;
+void RuntimeController::Render(int64_t view_id,
+                               Scene* scene,
+                               double width,
+                               double height) {
   const ViewportMetrics* view_metrics =
       UIDartState::Current()->platform_configuration()->GetMetrics(view_id);
   if (view_metrics == nullptr) {
     return;
   }
-  client_.Render(scene->takeLayerTree(width, height),
+  client_.Render(view_id, scene->takeLayerTree(width, height),
                  view_metrics->device_pixel_ratio);
 }
 

runtime/runtime_controller.h

@@ -661,7 +661,10 @@ class RuntimeController : public PlatformConfigurationClient {
   void EndWarmUpFrame() override;
 
   // |PlatformConfigurationClient|
-  void Render(Scene* scene, double width, double height) override;
+  void Render(int64_t view_id,
+              Scene* scene,
+              double width,
+              double height) override;
 
   // |PlatformConfigurationClient|
   void UpdateSemantics(SemanticsUpdate* update) override;

runtime/runtime_delegate.h

@@ -27,7 +27,8 @@ class RuntimeDelegate {
 
   virtual void EndWarmUpFrame() = 0;
 
-  virtual void Render(std::unique_ptr<flutter::LayerTree> layer_tree,
+  virtual void Render(int64_t view_id,
+                      std::unique_ptr<flutter::LayerTree> layer_tree,
                       float device_pixel_ratio) = 0;
 
   virtual void UpdateSemantics(SemanticsNodeUpdates update,

shell/common/animator.cc

@@ -62,6 +62,10 @@ void Animator::BeginFrame(
     std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) {
   TRACE_EVENT_ASYNC_END0("flutter", "Frame Request Pending",
                          frame_request_number_);
+  // Clear layer trees rendered out of a frame. Only Animator::Render called
+  // within a frame is used.
+  layer_trees_tasks_.clear();
+
   frame_request_number_++;
 
   frame_timings_recorder_ = std::move(frame_timings_recorder);
@@ -112,6 +116,33 @@ void Animator::BeginFrame(
   dart_frame_deadline_ = frame_target_time.ToEpochDelta();
   uint64_t frame_number = frame_timings_recorder_->GetFrameNumber();
   delegate_.OnAnimatorBeginFrame(frame_target_time, frame_number);
+}
+
+void Animator::EndFrame() {
+  FML_DCHECK(frame_timings_recorder_ != nullptr);
+  if (!layer_trees_tasks_.empty()) {
+    // The build is completed in OnAnimatorBeginFrame.
+    frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now());
+
+    delegate_.OnAnimatorUpdateLatestFrameTargetTime(
+        frame_timings_recorder_->GetVsyncTargetTime());
+
+    // Commit the pending continuation.
+    PipelineProduceResult result =
+        producer_continuation_.Complete(std::make_unique<FrameItem>(
+            std::move(layer_trees_tasks_), std::move(frame_timings_recorder_)));
+
+    if (!result.success) {
+      FML_DLOG(INFO) << "Failed to commit to the pipeline";
+    } else if (!result.is_first_item) {
+      // Do nothing. 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.
+    } else {
+      delegate_.OnAnimatorDraw(layer_tree_pipeline_);
+    }
+  }
+  frame_timings_recorder_ = nullptr;
 
   if (!frame_scheduled_ && has_rendered_) {
     // Wait a tad more than 3 60hz frames before reporting a big idle period.
@@ -139,14 +170,18 @@ void Animator::BeginFrame(
         },
         kNotifyIdleTaskWaitTime);
   }
+  FML_DCHECK(layer_trees_tasks_.empty());
+  FML_DCHECK(frame_timings_recorder_ == nullptr);
 }
 
-void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree,
+void Animator::Render(int64_t view_id,
+                      std::unique_ptr<flutter::LayerTree> layer_tree,
                       float device_pixel_ratio) {
   has_rendered_ = true;
 
   if (!frame_timings_recorder_) {
-    // Framework can directly call render with a built scene.
+    // Framework can directly call render with a built scene. A major reason is
+    // to render warm up frames.
     frame_timings_recorder_ = std::make_unique<FrameTimingsRecorder>();
     const fml::TimePoint placeholder_time = fml::TimePoint::Now();
     frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time);
@@ -156,35 +191,9 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree,
   TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter",
                                 "Animator::Render", /*flow_id_count=*/0,
                                 /*flow_ids=*/nullptr);
-  frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now());
-
-  delegate_.OnAnimatorUpdateLatestFrameTargetTime(
-      frame_timings_recorder_->GetVsyncTargetTime());
 
-  // TODO(dkwingsmt): Currently only supports a single window.
-  // See https://github.com/flutter/flutter/issues/135530, item 2.
-  int64_t view_id = kFlutterImplicitViewId;
-  std::vector<std::unique_ptr<LayerTreeTask>> layer_trees_tasks;
-  layer_trees_tasks.push_back(std::make_unique<LayerTreeTask>(
+  layer_trees_tasks_.push_back(std::make_unique<LayerTreeTask>(
       view_id, std::move(layer_tree), device_pixel_ratio));
-  // Commit the pending continuation.
-  PipelineProduceResult result =
-      producer_continuation_.Complete(std::make_unique<FrameItem>(
-          std::move(layer_trees_tasks), std::move(frame_timings_recorder_)));
-
-  if (!result.success) {
-    FML_DLOG(INFO) << "No pending continuation to commit";
-    return;
-  }
-
-  if (!result.is_first_item) {
-    // 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_);
 }
 
 const std::weak_ptr<VsyncWaiter> Animator::GetVsyncWaiter() const {
@@ -256,6 +265,7 @@ void Animator::AwaitVSync() {
             self->DrawLastLayerTrees(std::move(frame_timings_recorder));
           } else {
             self->BeginFrame(std::move(frame_timings_recorder));
+            self->EndFrame();
           }
         }
       });
@@ -265,9 +275,9 @@ void Animator::AwaitVSync() {
 }
 
 void Animator::EndWarmUpFrame() {
-  // Do nothing. The warm up frame does not need any additional work to end the
-  // frame for now. This will change once the pipeline supports multi-view.
-  // https://github.com/flutter/flutter/issues/142851
+  if (!layer_trees_tasks_.empty()) {
+    EndFrame();
+  }
 }
 
 void Animator::ScheduleSecondaryVsyncCallback(uintptr_t id,

shell/common/animator.h

@@ -76,7 +76,8 @@ class Animator final {
   ///           technically, between Animator::BeginFrame and Animator::EndFrame
   ///           (both private methods). Otherwise, this call will be ignored.
   ///
-  void Render(std::unique_ptr<flutter::LayerTree> layer_tree,
+  void Render(int64_t view_id,
+              std::unique_ptr<flutter::LayerTree> layer_tree,
               float device_pixel_ratio);
 
   const std::weak_ptr<VsyncWaiter> GetVsyncWaiter() const;
@@ -105,7 +106,13 @@ class Animator final {
   void EnqueueTraceFlowId(uint64_t trace_flow_id);
 
  private:
+  // Animator's work during a vsync is split into two methods, BeginFrame and
+  // EndFrame. The two methods should be called synchronously back-to-back to
+  // avoid being interrupted by a regular vsync. The reason to split them is to
+  // allow ShellTest::PumpOneFrame to insert a Render in between.
+
   void BeginFrame(std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder);
+  void EndFrame();
 
   bool CanReuseLastLayerTrees();
 
@@ -122,6 +129,7 @@ class Animator final {
   std::shared_ptr<VsyncWaiter> waiter_;
 
   std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder_;
+  std::vector<std::unique_ptr<LayerTreeTask>> layer_trees_tasks_;
   uint64_t frame_request_number_ = 1;
   fml::TimeDelta dart_frame_deadline_;
   std::shared_ptr<FramePipeline> layer_tree_pipeline_;