Reland 5: Multiview pipeline (#51186)

This relands #50931.

The crash that caused the 4th revert has been fixed by flutter/flutter#144212.

[There has been discussion](#51019) on why the benchmark in previous attempts show significant drop in build time. This PR addresses it using option a as described in [this comment](#51186 (comment)).

This PR also addresses flutter/flutter#144584 with option 1. A test is added.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: dkwingsmt

ci/licenses_golden/excluded_files

@@ -252,6 +252,7 @@
 ../../../flutter/shell/common/base64_unittests.cc
 ../../../flutter/shell/common/context_options_unittests.cc
 ../../../flutter/shell/common/dl_op_spy_unittests.cc
+../../../flutter/shell/common/engine_animator_unittests.cc
 ../../../flutter/shell/common/engine_unittests.cc
 ../../../flutter/shell/common/fixtures
 ../../../flutter/shell/common/input_events_unittests.cc

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

@@ -77,7 +77,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/dart_isolate_unittests.cc

@@ -707,7 +707,10 @@ class FakePlatformConfigurationClient : public PlatformConfigurationClient {
   std::string DefaultRouteName() override { return ""; }
   void ScheduleFrame() override {}
   void EndWarmUpFrame() override {}
-  void Render(Scene* scene, double width, double height) override {}
+  void Render(int64_t view_id,
+              Scene* scene,
+              double width,
+              double height) override {}
   void UpdateSemantics(SemanticsUpdate* update) override {}
   void HandlePlatformMessage(
       std::unique_ptr<PlatformMessage> message) override {}

runtime/runtime_controller.cc

@@ -47,8 +47,8 @@ RuntimeController::RuntimeController(
 
 std::unique_ptr<RuntimeController> RuntimeController::Spawn(
     RuntimeDelegate& p_client,
-    std::string advisory_script_uri,
-    std::string advisory_script_entrypoint,
+    const std::string& advisory_script_uri,
+    const std::string& advisory_script_entrypoint,
     const std::function<void(int64_t)>& p_idle_notification_callback,
     const fml::closure& p_isolate_create_callback,
     const fml::closure& p_isolate_shutdown_callback,
@@ -57,13 +57,18 @@ std::unique_ptr<RuntimeController> RuntimeController::Spawn(
     fml::WeakPtr<ImageDecoder> image_decoder,
     fml::WeakPtr<ImageGeneratorRegistry> image_generator_registry,
     fml::TaskRunnerAffineWeakPtr<SnapshotDelegate> snapshot_delegate) const {
-  UIDartState::Context spawned_context{
-      context_.task_runners,          std::move(snapshot_delegate),
-      std::move(io_manager),          context_.unref_queue,
-      std::move(image_decoder),       std::move(image_generator_registry),
-      std::move(advisory_script_uri), std::move(advisory_script_entrypoint),
-      context_.volatile_path_tracker, context_.concurrent_task_runner,
-      context_.enable_impeller,       context_.runtime_stage_backend};
+  UIDartState::Context spawned_context{context_.task_runners,
+                                       std::move(snapshot_delegate),
+                                       std::move(io_manager),
+                                       context_.unref_queue,
+                                       std::move(image_decoder),
+                                       std::move(image_generator_registry),
+                                       advisory_script_uri,
+                                       advisory_script_entrypoint,
+                                       context_.volatile_path_tracker,
+                                       context_.concurrent_task_runner,
+                                       context_.enable_impeller,
+                                       context_.runtime_stage_backend};
   auto result =
       std::make_unique<RuntimeController>(p_client,                      //
                                           vm_,                           //
@@ -226,6 +231,7 @@ bool RuntimeController::SetAccessibilityFeatures(int32_t flags) {
 
 bool RuntimeController::BeginFrame(fml::TimePoint frame_time,
                                    uint64_t frame_number) {
+  MarkAsFrameBorder();
   if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
     platform_configuration->BeginFrame(frame_time, frame_number);
     return true;
@@ -340,22 +346,37 @@ void RuntimeController::ScheduleFrame() {
   client_.ScheduleFrame();
 }
 
-// |PlatformConfigurationClient|
 void RuntimeController::EndWarmUpFrame() {
-  client_.EndWarmUpFrame();
+  client_.OnAllViewsRendered();
 }
 
 // |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);
+  rendered_views_during_frame_.insert(view_id);
+  CheckIfAllViewsRendered();
+}
+
+void RuntimeController::MarkAsFrameBorder() {
+  rendered_views_during_frame_.clear();
+}
+
+void RuntimeController::CheckIfAllViewsRendered() {
+  if (rendered_views_during_frame_.size() != 0 &&
+      rendered_views_during_frame_.size() ==
+          platform_data_.viewport_metrics_for_views.size()) {
+    client_.OnAllViewsRendered();
+    MarkAsFrameBorder();
+  }
 }
 
 // |PlatformConfigurationClient|

runtime/runtime_controller.h

@@ -104,8 +104,8 @@ class RuntimeController : public PlatformConfigurationClient {
   ///
   std::unique_ptr<RuntimeController> Spawn(
       RuntimeDelegate& p_client,
-      std::string advisory_script_uri,
-      std::string advisory_script_entrypoint,
+      const std::string& advisory_script_uri,
+      const std::string& advisory_script_entrypoint,
       const std::function<void(int64_t)>& idle_notification_callback,
       const fml::closure& isolate_create_callback,
       const fml::closure& isolate_shutdown_callback,
@@ -660,6 +660,26 @@ class RuntimeController : public PlatformConfigurationClient {
       std::shared_ptr<PlatformIsolateManager>(new PlatformIsolateManager());
   bool has_flushed_runtime_state_ = false;
 
+  // Tracks the views that have been called `Render` during a frame.
+  //
+  // If all views that have been registered by `AddView` have been called
+  // `Render`, then the runtime controller notifies the client of the end of
+  // frame immediately, allowing the client to submit the views to the pipeline
+  // a bit earlier than having to wait for the end of `BeginFrame`. See also
+  // `Animator::OnAllViewsRendered`.
+  //
+  // This mechanism fixes https://github.com/flutter/flutter/issues/144584 with
+  // option 2 and
+  // https://github.com/flutter/engine/pull/51186#issuecomment-1977820525 with
+  // option a in most cases, except if there are multiple views and only part of
+  // them are rendered.
+  // TODO(dkwingsmt): Fix these problems for all cases.
+  std::unordered_set<uint64_t> rendered_views_during_frame_;
+
+  void MarkAsFrameBorder();
+
+  void CheckIfAllViewsRendered();
+
   PlatformConfiguration* GetPlatformConfigurationIfAvailable();
 
   bool FlushRuntimeStateToIsolate();
@@ -674,7 +694,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

@@ -25,9 +25,10 @@ class RuntimeDelegate {
 
   virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0;
 
-  virtual void EndWarmUpFrame() = 0;
+  virtual void OnAllViewsRendered() = 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/BUILD.gn

@@ -310,6 +310,7 @@ if (enable_unittests) {
       "base64_unittests.cc",
       "context_options_unittests.cc",
       "dl_op_spy_unittests.cc",
+      "engine_animator_unittests.cc",
       "engine_unittests.cc",
       "input_events_unittests.cc",
       "persistent_cache_unittests.cc",