Remove pipeline in favor of layer tree holder

ci/licenses_golden/licenses_flutter

@@ -576,12 +576,12 @@ FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png
 FILE: ../../../flutter/shell/common/input_events_unittests.cc
 FILE: ../../../flutter/shell/common/isolate_configuration.cc
 FILE: ../../../flutter/shell/common/isolate_configuration.h
+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.cc
 FILE: ../../../flutter/shell/common/persistent_cache.h
 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

@@ -87,10 +87,10 @@ source_set("common") {
     "engine.h",
     "isolate_configuration.cc",
     "isolate_configuration.h",
+    "layer_tree_holder.cc",
+    "layer_tree_holder.h",
     "persistent_cache.cc",
     "persistent_cache.h",
-    "pipeline.cc",
-    "pipeline.h",
     "platform_view.cc",
     "platform_view.h",
     "pointer_data_dispatcher.cc",
@@ -191,8 +191,8 @@ if (enable_unittests) {
       "animator_unittests.cc",
       "canvas_spy_unittests.cc",
       "input_events_unittests.cc",
+      "layer_tree_holder_unittests.cc",
       "persistent_cache_unittests.cc",
-      "pipeline_unittests.cc",
       "renderer_context_manager_unittests.cc",
       "renderer_context_test.cc",
       "renderer_context_test.h",

shell/common/animator.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "flutter/shell/common/animator.h"
+#include <memory>
 
 #include "flutter/fml/trace_event.h"
 #include "third_party/dart/runtime/include/dart_tools_api.h"
@@ -28,27 +29,15 @@ Animator::Animator(Delegate& delegate,
       last_frame_begin_time_(),
       last_frame_target_time_(),
       dart_frame_deadline_(0),
-#if FLUTTER_SHELL_ENABLE_METAL
-      layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
-#else   // FLUTTER_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  // FLUTTER_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;
 
@@ -114,25 +103,6 @@ void Animator::BeginFrame(fml::TimePoint frame_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_ = frame_start_time;
   last_frame_target_time_ = frame_target_time;
   dart_frame_deadline_ = FxlToDartOrEarlier(frame_target_time);
@@ -184,13 +154,8 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
                                 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_);
+  layer_tree_holder_->ReplaceIfNewer(std::move(layer_tree));
+  delegate_.OnAnimatorDraw(layer_tree_holder_);
 }
 
 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) = 0;
+        std::shared_ptr<LayerTreeHolder> layer_tree_holder) = 0;
 
     virtual void OnAnimatorDrawLastLayerTree() = 0;
   };
@@ -80,8 +81,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_frame_begin_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

@@ -406,25 +406,18 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   ///             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);
 
   //----------------------------------------------------------------------------

shell/common/layer_tree_holder.cc

@@ -0,0 +1,28 @@
+// 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 {
+
+std::unique_ptr<LayerTree> LayerTreeHolder::Get() {
+  std::scoped_lock lock(layer_tree_mutex);
+  return std::move(layer_tree_);
+}
+
+void LayerTreeHolder::ReplaceIfNewer(
+    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,35 @@
+// 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 {
+
+class LayerTreeHolder {
+ public:
+  LayerTreeHolder() = default;
+
+  ~LayerTreeHolder() = default;
+
+  bool IsEmpty() const;
+
+  std::unique_ptr<LayerTree> Get();
+
+  void ReplaceIfNewer(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, 100.0f, 1.0f);
+  layer_tree_holder.ReplaceIfNewer(std::move(layer_tree));
+  ASSERT_FALSE(layer_tree_holder.IsEmpty());
+  const auto stored = layer_tree_holder.Get();
+  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, 100.0f, 1.0f);
+  layer_tree_1->RecordBuildTime(build_begin, target_time_1);
+  layer_tree_holder.ReplaceIfNewer(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, 100.0f, 1.0f);
+  layer_tree_2->RecordBuildTime(build_begin, target_time_2);
+  layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_2));
+
+  const auto stored = layer_tree_holder.Get();
+  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, 100.0f, 1.0f);
+  layer_tree_1->RecordBuildTime(build_begin, target_time_1);
+  layer_tree_holder.ReplaceIfNewer(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, 100.0f, 1.0f);
+  layer_tree_2->RecordBuildTime(build_begin, target_time_2);
+  layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_2));
+
+  const auto stored = layer_tree_holder.Get();
+  ASSERT_EQ(stored->frame_size(), frame_size_1);
+  ASSERT_TRUE(layer_tree_holder.IsEmpty());
+}
+
+}  // namespace testing
+}  // namespace flutter