Additional changes for this attempt.

Author: dkwingsmt

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

@@ -226,6 +226,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;
@@ -341,7 +342,7 @@ void RuntimeController::ScheduleFrame() {
 }
 
 void RuntimeController::EndWarmUpFrame() {
-  client_.EndWarmUpFrame();
+  client_.OnAllViewsRendered();
 }
 
 // |PlatformConfigurationClient|
@@ -356,6 +357,21 @@ void RuntimeController::Render(int64_t view_id,
   }
   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

@@ -660,6 +660,24 @@ 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 registered views (through `AddView`) have been rendered, then the
+  // end of frame will be called immediately, submitting the views to the
+  // pipeline a bit earlier than having to wait for the end of the vsync.
+  //
+  // 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();

runtime/runtime_delegate.h

@@ -25,7 +25,7 @@ class RuntimeDelegate {
 
   virtual void ScheduleFrame(bool regenerate_layer_trees = true) = 0;
 
-  virtual void EndWarmUpFrame() = 0;
+  virtual void OnAllViewsRendered() = 0;
 
   virtual void Render(int64_t view_id,
                       std::unique_ptr<flutter::LayerTree> layer_tree,

shell/common/animator.cc

@@ -119,7 +119,12 @@ void Animator::BeginFrame(
 }
 
 void Animator::EndFrame() {
-  FML_DCHECK(frame_timings_recorder_ != nullptr);
+  if (frame_timings_recorder_ == nullptr) {
+    // `EndFrame` has been called in this frame. This happens if the engine has
+    // called `OnAllViewsRendered` and then the end of the vsync task calls
+    // `EndFrame` again.
+    return;
+  }
   if (!layer_trees_tasks_.empty()) {
     // The build is completed in OnAnimatorBeginFrame.
     frame_timings_recorder_->RecordBuildEnd(fml::TimePoint::Now());
@@ -274,7 +279,7 @@ void Animator::AwaitVSync() {
   }
 }
 
-void Animator::EndWarmUpFrame() {
+void Animator::OnAllViewsRendered() {
   if (!layer_trees_tasks_.empty()) {
     EndFrame();
   }

shell/common/animator.h

@@ -54,20 +54,27 @@ class Animator final {
   void RequestFrame(bool regenerate_layer_trees = true);
 
   //--------------------------------------------------------------------------
-  /// @brief    Tells the Animator that a warm up frame has ended.
+  /// @brief    Tells the Animator that all views that should render for this
+  ///           frame have been rendered.
   ///
-  ///           In a warm up frame, `Animator::Render` is called out of vsync
-  ///           tasks, and Animator requires an explicit end-of-frame call to
-  ///           know when to send the layer trees to the pipeline.
+  ///           In regular frames, since all `Render` calls must take place
+  ///           during a vsync task, the Animator knows that all views have
+  ///           been rendered at the end of the vsync task, therefore calling
+  ///           this method is not needed.
   ///
-  ///           This is different from regular frames, where Animator::Render is
-  ///           always called within a vsync task, and Animator can send
-  ///           the views at the end of the vsync task.
+  ///           However, the engine might decide to start it a bit earlier, for
+  ///           example, if the engine decides that no more views can be
+  ///           rendered, so that the rasterization can start a bit earlier.
+  ///
+  ///           This method is also useful in warm-up frames. In a warm up
+  ///           frame, `Animator::Render` is called out of vsync tasks, and
+  ///           Animator requires an explicit end-of-frame call to know when to
+  ///           send the layer trees to the pipeline.
   ///
   ///           For more about warm up frames, see
   ///           `PlatformDispatcher.scheduleWarmUpFrame`.
   ///
-  void EndWarmUpFrame();
+  void OnAllViewsRendered();
 
   //--------------------------------------------------------------------------
   /// @brief    Tells the Animator that this frame needs to render another view.

shell/common/engine.cc

@@ -464,8 +464,8 @@ void Engine::ScheduleFrame(bool regenerate_layer_trees) {
   animator_->RequestFrame(regenerate_layer_trees);
 }
 
-void Engine::EndWarmUpFrame() {
-  animator_->EndWarmUpFrame();
+void Engine::OnAllViewsRendered() {
+  animator_->OnAllViewsRendered();
 }
 
 void Engine::Render(int64_t view_id,

shell/common/engine.h

@@ -838,7 +838,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   void ScheduleFrame() { ScheduleFrame(true); }
 
   // |RuntimeDelegate|
-  void EndWarmUpFrame() override;
+  void OnAllViewsRendered() override;
 
   // |RuntimeDelegate|
   FontCollection& GetFontCollection() override;

shell/common/engine_unittests.cc

@@ -120,7 +120,7 @@ class MockRuntimeDelegate : public RuntimeDelegate {
  public:
   MOCK_METHOD(std::string, DefaultRouteName, (), (override));
   MOCK_METHOD(void, ScheduleFrame, (bool), (override));
-  MOCK_METHOD(void, EndWarmUpFrame, (), (override));
+  MOCK_METHOD(void, OnAllViewsRendered, (), (override));
   MOCK_METHOD(void,
               Render,
               (int64_t, std::unique_ptr<flutter::LayerTree>, float),
@@ -679,6 +679,64 @@ TEST_F(EngineTest, AnimatorAcceptsMultipleRenders) {
   draw_latch.Wait();
 }
 
+TEST_F(EngineTest, AnimatorSubmitsSingleViewBeforeDrawFrameEnds) {
+  MockAnimatorDelegate animator_delegate;
+  std::unique_ptr<EngineContext> engine_context;
+
+  std::shared_ptr<PlatformMessageHandler> platform_message_handler =
+      std::make_shared<MockPlatformMessageHandler>();
+  EXPECT_CALL(delegate_, GetPlatformMessageHandler)
+      .WillOnce(ReturnRef(platform_message_handler));
+
+  static bool rasterization_started = false;
+  EXPECT_CALL(animator_delegate, OnAnimatorDraw)
+      .WillOnce(Invoke([](const std::shared_ptr<FramePipeline>& pipeline) {
+        rasterization_started = true;
+        auto status = pipeline->Consume([&](std::unique_ptr<FrameItem> item) {
+          EXPECT_EQ(item->layer_tree_tasks.size(), 1u);
+          EXPECT_EQ(item->layer_tree_tasks[0]->view_id, kFlutterImplicitViewId);
+        });
+        EXPECT_EQ(status, PipelineConsumeResult::Done);
+      }));
+  EXPECT_CALL(animator_delegate, OnAnimatorBeginFrame)
+      .WillRepeatedly(Invoke([&engine_context](fml::TimePoint frame_target_time,
+                                               uint64_t frame_number) {
+        engine_context->EngineTaskSync([&](Engine& engine) {
+          engine.BeginFrame(frame_target_time, frame_number);
+        });
+      }));
+
+  std::unique_ptr<Animator> animator;
+  PostSync(task_runners_.GetUITaskRunner(),
+           [&animator, &animator_delegate, &task_runners = task_runners_] {
+             animator = std::make_unique<Animator>(
+                 animator_delegate, task_runners,
+                 static_cast<std::unique_ptr<VsyncWaiter>>(
+                     std::make_unique<testing::ConstantFiringVsyncWaiter>(
+                         task_runners)));
+           });
+
+  native_latch.Reset();
+  // The native_latch is signaled at the end of handleDrawFrame.
+  AddNativeCallback("NotifyNative", [](auto args) {
+    EXPECT_EQ(rasterization_started, true);
+    native_latch.Signal();
+  });
+
+  engine_context = EngineContext::Create(delegate_, settings_, task_runners_,
+                                         std::move(animator));
+
+  engine_context->EngineTaskSync([](Engine& engine) {
+    engine.AddView(kFlutterImplicitViewId, ViewportMetrics{1.0, 10, 10, 1, 0});
+  });
+
+  auto configuration = RunConfiguration::InferFromSettings(settings_);
+  configuration.SetEntrypoint("renderSingleViewAndCallAfterOnDrawFrame");
+  engine_context->Run(std::move(configuration));
+
+  native_latch.Wait();
+}
+
 // The animator should submit to the pipeline the implicit view rendered in a
 // warm up frame if there's already a continuation (i.e. Animator::BeginFrame
 // has been called)

shell/common/fixtures/shell_test.dart

@@ -142,6 +142,10 @@ external void sayHiFromFixturesAreFunctionalMain();
 @pragma('vm:external-name', 'NotifyNative')
 external void notifyNative();
 
+@pragma('vm:entry-point')
+@pragma('vm:external-name', 'NotifyNative2')
+external void notifyNative2();
+
 @pragma('vm:entry-point')
 void thousandCallsToNative() {
   for (int i = 0; i < 1000; i++) {
@@ -553,6 +557,26 @@ void onDrawFrameRenderAllViews() {
   notifyNative();
 }
 
+@pragma('vm:entry-point')
+void renderSingleViewAndCallAfterOnDrawFrame() {
+  PlatformDispatcher.instance.onDrawFrame = () {
+    final SceneBuilder builder = SceneBuilder();
+    final PictureRecorder recorder = PictureRecorder();
+    final Canvas canvas = Canvas(recorder);
+    canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF));
+    final Picture picture = recorder.endRecording();
+    builder.addPicture(Offset.zero, picture);
+
+    final Scene scene = builder.build();
+    PlatformDispatcher.instance.implicitView!.render(scene);
+    notifyNative();
+
+    scene.dispose();
+    picture.dispose();
+  };
+  PlatformDispatcher.instance.scheduleFrame();
+}
+
 @pragma('vm:entry-point')
 void renderWarmUpImplicitView() {
   bool beginFrameCalled = false;