Reland "Remove pipeline in favor of layer tree holder (#18901)"

This reverts commit eba7627.

Author: iskakaushik

ci/licenses_golden/licenses_flutter

@@ -638,10 +638,10 @@ FILE: ../../../flutter/shell/common/engine_unittests.cc
 FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
 FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png
 FILE: ../../../flutter/shell/common/input_events_unittests.cc
+FILE: ../../../flutter/shell/common/layer_tree_holder.cc
+FILE: ../../../flutter/shell/common/layer_tree_holder.h
+FILE: ../../../flutter/shell/common/layer_tree_holder_unittests.cc
 FILE: ../../../flutter/shell/common/persistent_cache_unittests.cc
-FILE: ../../../flutter/shell/common/pipeline.cc
-FILE: ../../../flutter/shell/common/pipeline.h
-FILE: ../../../flutter/shell/common/pipeline_unittests.cc
 FILE: ../../../flutter/shell/common/platform_view.cc
 FILE: ../../../flutter/shell/common/platform_view.h
 FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc

shell/common/BUILD.gn

@@ -69,8 +69,8 @@ source_set("common") {
     "display_manager.h",
     "engine.cc",
     "engine.h",
-    "pipeline.cc",
-    "pipeline.h",
+    "layer_tree_holder.cc",
+    "layer_tree_holder.h",
     "platform_view.cc",
     "platform_view.h",
     "pointer_data_dispatcher.cc",
@@ -242,8 +242,8 @@ if (enable_unittests) {
       "canvas_spy_unittests.cc",
       "engine_unittests.cc",
       "input_events_unittests.cc",
+      "layer_tree_holder_unittests.cc",
       "persistent_cache_unittests.cc",
-      "pipeline_unittests.cc",
       "rasterizer_unittests.cc",
       "shell_unittests.cc",
       "skp_shader_warmup_unittests.cc",

shell/common/animator.cc

@@ -29,27 +29,15 @@ Animator::Animator(Delegate& delegate,
       last_vsync_start_time_(),
       last_frame_target_time_(),
       dart_frame_deadline_(0),
-#if SHELL_ENABLE_METAL
-      layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
-#else   // SHELL_ENABLE_METAL
-      // TODO(dnfield): We should remove this logic and set the pipeline depth
-      // back to 2 in this case. See
-      // https://github.com/flutter/engine/pull/9132 for discussion.
-      layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
-          task_runners.GetPlatformTaskRunner() ==
-                  task_runners.GetRasterTaskRunner()
-              ? 1
-              : 2)),
-#endif  // SHELL_ENABLE_METAL
+      layer_tree_holder_(std::make_shared<LayerTreeHolder>()),
       pending_frame_semaphore_(1),
       frame_number_(1),
       paused_(false),
       regenerate_layer_tree_(false),
       frame_scheduled_(false),
       notify_idle_task_id_(0),
       dimension_change_pending_(false),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
 Animator::~Animator() = default;
 
@@ -111,25 +99,6 @@ void Animator::BeginFrame(fml::TimePoint vsync_start_time,
   regenerate_layer_tree_ = false;
   pending_frame_semaphore_.Signal();
 
-  if (!producer_continuation_) {
-    // We may already have a valid pipeline continuation in case a previous
-    // begin frame did not result in an Animation::Render. Simply reuse that
-    // instead of asking the pipeline for a fresh continuation.
-    producer_continuation_ = layer_tree_pipeline_->Produce();
-
-    if (!producer_continuation_) {
-      // If we still don't have valid continuation, the pipeline is currently
-      // full because the consumer is being too slow. Try again at the next
-      // frame interval.
-      RequestFrame();
-      return;
-    }
-  }
-
-  // We have acquired a valid continuation from the pipeline and are ready
-  // to service potential frame.
-  FML_DCHECK(producer_continuation_);
-
   last_frame_begin_time_ = fml::TimePoint::Now();
   last_vsync_start_time_ = vsync_start_time;
   fml::tracing::TraceEventAsyncComplete("flutter", "VsyncSchedulingOverhead",
@@ -183,13 +152,8 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
   layer_tree->RecordBuildTime(last_vsync_start_time_, last_frame_begin_time_,
                               last_frame_target_time_);
 
-  // Commit the pending continuation.
-  bool result = producer_continuation_.Complete(std::move(layer_tree));
-  if (!result) {
-    FML_DLOG(INFO) << "No pending continuation to commit";
-  }
-
-  delegate_.OnAnimatorDraw(layer_tree_pipeline_, last_frame_target_time_);
+  layer_tree_holder_->PushIfNewer(std::move(layer_tree));
+  delegate_.OnAnimatorDraw(layer_tree_holder_, last_frame_target_time_);
 }
 
 bool Animator::CanReuseLastLayerTree() {

shell/common/animator.h

@@ -6,13 +6,14 @@
 #define FLUTTER_SHELL_COMMON_ANIMATOR_H_
 
 #include <deque>
+#include <memory>
 
 #include "flutter/common/task_runners.h"
 #include "flutter/fml/memory/ref_ptr.h"
 #include "flutter/fml/memory/weak_ptr.h"
 #include "flutter/fml/synchronization/semaphore.h"
 #include "flutter/fml/time/time_point.h"
-#include "flutter/shell/common/pipeline.h"
+#include "flutter/shell/common/layer_tree_holder.h"
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/vsync_waiter.h"
 
@@ -35,7 +36,7 @@ class Animator final {
     virtual void OnAnimatorNotifyIdle(int64_t deadline) = 0;
 
     virtual void OnAnimatorDraw(
-        fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+        std::shared_ptr<LayerTreeHolder> layer_tree_holder,
         fml::TimePoint frame_target_time) = 0;
 
     virtual void OnAnimatorDrawLastLayerTree() = 0;
@@ -79,8 +80,6 @@ class Animator final {
   void EnqueueTraceFlowId(uint64_t trace_flow_id);
 
  private:
-  using LayerTreePipeline = Pipeline<flutter::LayerTree>;
-
   void BeginFrame(fml::TimePoint frame_start_time,
                   fml::TimePoint frame_target_time);
 
@@ -99,9 +98,8 @@ class Animator final {
   fml::TimePoint last_vsync_start_time_;
   fml::TimePoint last_frame_target_time_;
   int64_t dart_frame_deadline_;
-  fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
+  std::shared_ptr<LayerTreeHolder> layer_tree_holder_;
   fml::Semaphore pending_frame_semaphore_;
-  LayerTreePipeline::ProducerContinuation producer_continuation_;
   int64_t frame_number_;
   bool paused_;
   bool regenerate_layer_tree_;

shell/common/engine.h

@@ -476,25 +476,18 @@ class Engine final : public RuntimeDelegate,
   ///             will cause the jank in the Flutter application:
   ///             * The time taken by this method to create a layer-tree exceeds
   ///               on frame interval (for example, 16.66 ms on a 60Hz display).
-  ///             * The time take by this method to generate a new layer-tree
-  ///               causes the current layer-tree pipeline depth to change. To
-  ///               illustrate this point, note that maximum pipeline depth used
-  ///               by layer tree in the engine is 2. If both the UI and GPU
-  ///               task runner tasks finish within one frame interval, the
-  ///               pipeline depth is one. If the UI thread happens to be
-  ///               working on a frame when the raster thread is still not done
-  ///               with the previous frame, the pipeline depth is 2. When the
-  ///               pipeline depth changes from 1 to 2, animations and UI
-  ///               interactions that cause the generation of the new layer tree
-  ///               appropriate for (frame_time + one frame interval) will
-  ///               actually end up at (frame_time + two frame intervals). This
-  ///               is not what code running on the UI thread expected would
-  ///               happen. This causes perceptible jank.
+  ///             * A new layer-tree produced by this method replaces a stale
+  ///               layer tree in `LayerTreeHolder`. See:
+  ///               `LayerTreeHolder::ReplaceIfNewer`. This could happen if
+  ///               rasterizer takes more than one frame interval to rasterize a
+  ///               layer tree. This would cause some frames to be skipped and
+  ///               could result in perceptible jank.
   ///
   /// @param[in]  frame_time  The point at which the current frame interval
   ///                         began. May be used by animation interpolators,
   ///                         physics simulations, etc..
   ///
+  /// @see         `LayerTreeHolder::ReplaceIfNewer`
   void BeginFrame(fml::TimePoint frame_time);
 
   // |HintFreedDelegate|

shell/common/layer_tree_holder.cc

@@ -0,0 +1,32 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/common/layer_tree_holder.h"
+
+namespace flutter {
+
+LayerTreeHolder::LayerTreeHolder() = default;
+
+LayerTreeHolder::~LayerTreeHolder() = default;
+
+std::unique_ptr<LayerTree> LayerTreeHolder::Pop() {
+  std::scoped_lock lock(layer_tree_mutex);
+  return std::move(layer_tree_);
+}
+
+void LayerTreeHolder::PushIfNewer(
+    std::unique_ptr<LayerTree> proposed_layer_tree) {
+  std::scoped_lock lock(layer_tree_mutex);
+  if (!layer_tree_ ||
+      layer_tree_->target_time() < proposed_layer_tree->target_time()) {
+    layer_tree_ = std::move(proposed_layer_tree);
+  }
+}
+
+bool LayerTreeHolder::IsEmpty() const {
+  std::scoped_lock lock(layer_tree_mutex);
+  return !layer_tree_;
+}
+
+};  // namespace flutter

shell/common/layer_tree_holder.h

@@ -0,0 +1,55 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_
+#define FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_
+
+#include <memory>
+
+#include "flow/layers/layer_tree.h"
+
+namespace flutter {
+
+/**
+ * @brief Holds the next `flutter::LayerTree` that needs to be rasterized. The
+ * accesses to `LayerTreeHolder` are thread safe. This is important as this
+ * component is accessed from both the UI and the Raster threads.
+ *
+ * A typical flow of events through this component would be:
+ *  1. `flutter::Animator` pushed a layer tree to be rendered during each
+ * `Animator::Render` call.
+ *  2. `flutter::Rasterizer::Draw` consumes the pushed layer tree via `Pop`.
+ *
+ * It is important to note that if a layer tree held by this class is yet to be
+ * consumed, it can be overriden by a newer layer tree produced by the
+ * `Animator`. The newness of the layer tree is determined by the target time.
+ */
+class LayerTreeHolder {
+ public:
+  LayerTreeHolder();
+
+  ~LayerTreeHolder();
+
+  /**
+   * @brief Checks if a layer tree is currently held.
+   *
+   * @return true is no layer tree is held.
+   * @return false if there is a layer tree waiting to be consumed.
+   */
+  bool IsEmpty() const;
+
+  [[nodiscard]] std::unique_ptr<LayerTree> Pop();
+
+  void PushIfNewer(std::unique_ptr<LayerTree> proposed_layer_tree);
+
+ private:
+  mutable std::mutex layer_tree_mutex;
+  std::unique_ptr<LayerTree> layer_tree_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(LayerTreeHolder);
+};
+
+};  // namespace flutter
+
+#endif  // FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_

shell/common/layer_tree_holder_unittests.cc

@@ -0,0 +1,76 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#define FML_USED_ON_EMBEDDER
+
+#include <functional>
+#include <future>
+#include <memory>
+
+#include "flutter/shell/common/layer_tree_holder.h"
+#include "gtest/gtest.h"
+
+namespace flutter {
+namespace testing {
+
+TEST(LayerTreeHolder, EmptyOnInit) {
+  const LayerTreeHolder layer_tree_holder;
+  ASSERT_TRUE(layer_tree_holder.IsEmpty());
+}
+
+TEST(LayerTreeHolder, PutOneAndGet) {
+  LayerTreeHolder layer_tree_holder;
+  const auto frame_size = SkISize::Make(64, 64);
+  auto layer_tree = std::make_unique<LayerTree>(frame_size, 1.0f);
+  layer_tree_holder.PushIfNewer(std::move(layer_tree));
+  ASSERT_FALSE(layer_tree_holder.IsEmpty());
+  const auto stored = layer_tree_holder.Pop();
+  ASSERT_EQ(stored->frame_size(), frame_size);
+  ASSERT_TRUE(layer_tree_holder.IsEmpty());
+}
+
+TEST(LayerTreeHolder, PutMultiGetsLatest) {
+  const auto build_begin = fml::TimePoint::Now();
+  const auto target_time_1 = build_begin + fml::TimeDelta::FromSeconds(2);
+  const auto target_time_2 = build_begin + fml::TimeDelta::FromSeconds(5);
+
+  LayerTreeHolder layer_tree_holder;
+  const auto frame_size_1 = SkISize::Make(64, 64);
+  auto layer_tree_1 = std::make_unique<LayerTree>(frame_size_1, 1.0f);
+  layer_tree_1->RecordBuildTime(build_begin, build_begin, target_time_1);
+  layer_tree_holder.PushIfNewer(std::move(layer_tree_1));
+
+  const auto frame_size_2 = SkISize::Make(128, 128);
+  auto layer_tree_2 = std::make_unique<LayerTree>(frame_size_2, 1.0f);
+  layer_tree_2->RecordBuildTime(build_begin, build_begin, target_time_2);
+  layer_tree_holder.PushIfNewer(std::move(layer_tree_2));
+
+  const auto stored = layer_tree_holder.Pop();
+  ASSERT_EQ(stored->frame_size(), frame_size_2);
+  ASSERT_TRUE(layer_tree_holder.IsEmpty());
+}
+
+TEST(LayerTreeHolder, RetainsOlderIfNewerFrameHasEarlierTargetTime) {
+  const auto build_begin = fml::TimePoint::Now();
+  const auto target_time_1 = build_begin + fml::TimeDelta::FromSeconds(5);
+  const auto target_time_2 = build_begin + fml::TimeDelta::FromSeconds(2);
+
+  LayerTreeHolder layer_tree_holder;
+  const auto frame_size_1 = SkISize::Make(64, 64);
+  auto layer_tree_1 = std::make_unique<LayerTree>(frame_size_1, 1.0f);
+  layer_tree_1->RecordBuildTime(build_begin, build_begin, target_time_1);
+  layer_tree_holder.PushIfNewer(std::move(layer_tree_1));
+
+  const auto frame_size_2 = SkISize::Make(128, 128);
+  auto layer_tree_2 = std::make_unique<LayerTree>(frame_size_2, 1.0f);
+  layer_tree_2->RecordBuildTime(build_begin, build_begin, target_time_2);
+  layer_tree_holder.PushIfNewer(std::move(layer_tree_2));
+
+  const auto stored = layer_tree_holder.Pop();
+  ASSERT_EQ(stored->frame_size(), frame_size_1);
+  ASSERT_TRUE(layer_tree_holder.IsEmpty());
+}
+
+}  // namespace testing
+}  // namespace flutter