Revert "Remove pipeline in favor of layer tree holder (#17688)" (#18242)

flutter · May 8, 2020 · 4418ce8 · 4418ce8
1 parent ff6942f
commit 4418ce8
Show file tree

Hide file tree

Showing 16 changed files with 498 additions and 204 deletions.
diff --git a/ci/licenses_golden/licenses_flutter b/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

diff --git a/shell/common/BUILD.gn b/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",

diff --git a/shell/common/animator.cc b/shell/common/animator.cc
@@ -3,7 +3,6 @@
 // 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"
@@ -29,15 +28,27 @@ Animator::Animator(Delegate& delegate,
       last_frame_begin_time_(),
       last_frame_target_time_(),
       dart_frame_deadline_(0),
-      layer_tree_holder_(std::make_shared<LayerTreeHolder>()),
+#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
       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;
 
@@ -103,6 +114,25 @@ 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);
@@ -154,8 +184,13 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
                                 last_frame_target_time_);
   }
 
-  layer_tree_holder_->ReplaceIfNewer(std::move(layer_tree));
-  delegate_.OnAnimatorDraw(layer_tree_holder_);
+  // 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_);
 }
 
 bool Animator::CanReuseLastLayerTree() {

diff --git a/shell/common/animator.h b/shell/common/animator.h
@@ -6,14 +6,13 @@
 #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/layer_tree_holder.h"
+#include "flutter/shell/common/pipeline.h"
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/vsync_waiter.h"
 
@@ -36,7 +35,7 @@ class Animator final {
     virtual void OnAnimatorNotifyIdle(int64_t deadline) = 0;
 
     virtual void OnAnimatorDraw(
-        std::shared_ptr<LayerTreeHolder> layer_tree_holder) = 0;
+        fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) = 0;
 
     virtual void OnAnimatorDrawLastLayerTree() = 0;
   };
@@ -81,6 +80,8 @@ 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);
 
@@ -98,8 +99,9 @@ class Animator final {
   fml::TimePoint last_frame_begin_time_;
   fml::TimePoint last_frame_target_time_;
   int64_t dart_frame_deadline_;
-  std::shared_ptr<LayerTreeHolder> layer_tree_holder_;
+  fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
   fml::Semaphore pending_frame_semaphore_;
+  LayerTreePipeline::ProducerContinuation producer_continuation_;
   int64_t frame_number_;
   bool paused_;
   bool regenerate_layer_tree_;

diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -406,18 +406,25 @@ 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).
-  ///             * 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.
+  ///             * 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.
   ///
   /// @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);
 
   //----------------------------------------------------------------------------

diff --git a/shell/common/layer_tree_holder.cc b/shell/common/layer_tree_holder.cc
diff --git a/shell/common/layer_tree_holder.h b/shell/common/layer_tree_holder.h
diff --git a/shell/common/layer_tree_holder_unittests.cc b/shell/common/layer_tree_holder_unittests.cc
diff --git a/shell/common/pipeline.cc b/shell/common/pipeline.cc
@@ -0,0 +1,14 @@
+// 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/pipeline.h"
+
+namespace flutter {
+
+size_t GetNextPipelineTraceID() {
+  static std::atomic_size_t PipelineLastTraceID = {0};
+  return ++PipelineLastTraceID;
+}
+
+}  // namespace flutter