Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 2494d1c

Browse files
authored
Revert "Remove pipeline in favor of layer tree holder (#18285)" (#18427)
This reverts commit 2cdbc7f.
1 parent 49fe4b8 commit 2494d1c

File tree

16 files changed

+498
-228
lines changed

16 files changed

+498
-228
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -576,12 +576,12 @@ FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png
576576
FILE: ../../../flutter/shell/common/input_events_unittests.cc
577577
FILE: ../../../flutter/shell/common/isolate_configuration.cc
578578
FILE: ../../../flutter/shell/common/isolate_configuration.h
579-
FILE: ../../../flutter/shell/common/layer_tree_holder.cc
580-
FILE: ../../../flutter/shell/common/layer_tree_holder.h
581-
FILE: ../../../flutter/shell/common/layer_tree_holder_unittests.cc
582579
FILE: ../../../flutter/shell/common/persistent_cache.cc
583580
FILE: ../../../flutter/shell/common/persistent_cache.h
584581
FILE: ../../../flutter/shell/common/persistent_cache_unittests.cc
582+
FILE: ../../../flutter/shell/common/pipeline.cc
583+
FILE: ../../../flutter/shell/common/pipeline.h
584+
FILE: ../../../flutter/shell/common/pipeline_unittests.cc
585585
FILE: ../../../flutter/shell/common/platform_view.cc
586586
FILE: ../../../flutter/shell/common/platform_view.h
587587
FILE: ../../../flutter/shell/common/pointer_data_dispatcher.cc

shell/common/BUILD.gn

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,10 @@ source_set("common") {
8787
"engine.h",
8888
"isolate_configuration.cc",
8989
"isolate_configuration.h",
90-
"layer_tree_holder.cc",
91-
"layer_tree_holder.h",
9290
"persistent_cache.cc",
9391
"persistent_cache.h",
92+
"pipeline.cc",
93+
"pipeline.h",
9494
"platform_view.cc",
9595
"platform_view.h",
9696
"pointer_data_dispatcher.cc",
@@ -191,8 +191,8 @@ if (enable_unittests) {
191191
"animator_unittests.cc",
192192
"canvas_spy_unittests.cc",
193193
"input_events_unittests.cc",
194-
"layer_tree_holder_unittests.cc",
195194
"persistent_cache_unittests.cc",
195+
"pipeline_unittests.cc",
196196
"renderer_context_manager_unittests.cc",
197197
"renderer_context_test.cc",
198198
"renderer_context_test.h",

shell/common/animator.cc

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
// found in the LICENSE file.
44

55
#include "flutter/shell/common/animator.h"
6-
#include <memory>
76

87
#include "flutter/fml/trace_event.h"
98
#include "third_party/dart/runtime/include/dart_tools_api.h"
@@ -29,15 +28,27 @@ Animator::Animator(Delegate& delegate,
2928
last_frame_begin_time_(),
3029
last_frame_target_time_(),
3130
dart_frame_deadline_(0),
32-
layer_tree_holder_(std::make_shared<LayerTreeHolder>()),
31+
#if FLUTTER_SHELL_ENABLE_METAL
32+
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
33+
#else // FLUTTER_SHELL_ENABLE_METAL
34+
// TODO(dnfield): We should remove this logic and set the pipeline depth
35+
// back to 2 in this case. See
36+
// https://github.com/flutter/engine/pull/9132 for discussion.
37+
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
38+
task_runners.GetPlatformTaskRunner() ==
39+
task_runners.GetRasterTaskRunner()
40+
? 1
41+
: 2)),
42+
#endif // FLUTTER_SHELL_ENABLE_METAL
3343
pending_frame_semaphore_(1),
3444
frame_number_(1),
3545
paused_(false),
3646
regenerate_layer_tree_(false),
3747
frame_scheduled_(false),
3848
notify_idle_task_id_(0),
3949
dimension_change_pending_(false),
40-
weak_factory_(this) {}
50+
weak_factory_(this) {
51+
}
4152

4253
Animator::~Animator() = default;
4354

@@ -103,6 +114,25 @@ void Animator::BeginFrame(fml::TimePoint frame_start_time,
103114
regenerate_layer_tree_ = false;
104115
pending_frame_semaphore_.Signal();
105116

117+
if (!producer_continuation_) {
118+
// We may already have a valid pipeline continuation in case a previous
119+
// begin frame did not result in an Animation::Render. Simply reuse that
120+
// instead of asking the pipeline for a fresh continuation.
121+
producer_continuation_ = layer_tree_pipeline_->Produce();
122+
123+
if (!producer_continuation_) {
124+
// If we still don't have valid continuation, the pipeline is currently
125+
// full because the consumer is being too slow. Try again at the next
126+
// frame interval.
127+
RequestFrame();
128+
return;
129+
}
130+
}
131+
132+
// We have acquired a valid continuation from the pipeline and are ready
133+
// to service potential frame.
134+
FML_DCHECK(producer_continuation_);
135+
106136
last_frame_begin_time_ = frame_start_time;
107137
last_frame_target_time_ = frame_target_time;
108138
dart_frame_deadline_ = FxlToDartOrEarlier(frame_target_time);
@@ -154,8 +184,13 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
154184
last_frame_target_time_);
155185
}
156186

157-
layer_tree_holder_->PushIfNewer(std::move(layer_tree));
158-
delegate_.OnAnimatorDraw(layer_tree_holder_, last_frame_target_time_);
187+
// Commit the pending continuation.
188+
bool result = producer_continuation_.Complete(std::move(layer_tree));
189+
if (!result) {
190+
FML_DLOG(INFO) << "No pending continuation to commit";
191+
}
192+
193+
delegate_.OnAnimatorDraw(layer_tree_pipeline_, last_frame_target_time_);
159194
}
160195

161196
bool Animator::CanReuseLastLayerTree() {

shell/common/animator.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@
66
#define FLUTTER_SHELL_COMMON_ANIMATOR_H_
77

88
#include <deque>
9-
#include <memory>
109

1110
#include "flutter/common/task_runners.h"
1211
#include "flutter/fml/memory/ref_ptr.h"
1312
#include "flutter/fml/memory/weak_ptr.h"
1413
#include "flutter/fml/synchronization/semaphore.h"
1514
#include "flutter/fml/time/time_point.h"
16-
#include "flutter/shell/common/layer_tree_holder.h"
15+
#include "flutter/shell/common/pipeline.h"
1716
#include "flutter/shell/common/rasterizer.h"
1817
#include "flutter/shell/common/vsync_waiter.h"
1918

@@ -36,7 +35,7 @@ class Animator final {
3635
virtual void OnAnimatorNotifyIdle(int64_t deadline) = 0;
3736

3837
virtual void OnAnimatorDraw(
39-
std::shared_ptr<LayerTreeHolder> layer_tree_holder,
38+
fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
4039
fml::TimePoint frame_target_time) = 0;
4140

4241
virtual void OnAnimatorDrawLastLayerTree() = 0;
@@ -82,6 +81,8 @@ class Animator final {
8281
void EnqueueTraceFlowId(uint64_t trace_flow_id);
8382

8483
private:
84+
using LayerTreePipeline = Pipeline<flutter::LayerTree>;
85+
8586
void BeginFrame(fml::TimePoint frame_start_time,
8687
fml::TimePoint frame_target_time);
8788

@@ -99,8 +100,9 @@ class Animator final {
99100
fml::TimePoint last_frame_begin_time_;
100101
fml::TimePoint last_frame_target_time_;
101102
int64_t dart_frame_deadline_;
102-
std::shared_ptr<LayerTreeHolder> layer_tree_holder_;
103+
fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
103104
fml::Semaphore pending_frame_semaphore_;
105+
LayerTreePipeline::ProducerContinuation producer_continuation_;
104106
int64_t frame_number_;
105107
bool paused_;
106108
bool regenerate_layer_tree_;

shell/common/engine.h

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -406,18 +406,25 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
406406
/// will cause the jank in the Flutter application:
407407
/// * The time taken by this method to create a layer-tree exceeds
408408
/// on frame interval (for example, 16.66 ms on a 60Hz display).
409-
/// * A new layer-tree produced by this method replaces a stale
410-
/// layer tree in `LayerTreeHolder`. See:
411-
/// `LayerTreeHolder::ReplaceIfNewer`. This could happen if
412-
/// rasterizer takes more than one frame interval to rasterize a
413-
/// layer tree. This would cause some frames to be skipped and
414-
/// could result in perceptible jank.
409+
/// * The time take by this method to generate a new layer-tree
410+
/// causes the current layer-tree pipeline depth to change. To
411+
/// illustrate this point, note that maximum pipeline depth used
412+
/// by layer tree in the engine is 2. If both the UI and GPU
413+
/// task runner tasks finish within one frame interval, the
414+
/// pipeline depth is one. If the UI thread happens to be
415+
/// working on a frame when the raster thread is still not done
416+
/// with the previous frame, the pipeline depth is 2. When the
417+
/// pipeline depth changes from 1 to 2, animations and UI
418+
/// interactions that cause the generation of the new layer tree
419+
/// appropriate for (frame_time + one frame interval) will
420+
/// actually end up at (frame_time + two frame intervals). This
421+
/// is not what code running on the UI thread expected would
422+
/// happen. This causes perceptible jank.
415423
///
416424
/// @param[in] frame_time The point at which the current frame interval
417425
/// began. May be used by animation interpolators,
418426
/// physics simulations, etc..
419427
///
420-
/// @see `LayerTreeHolder::ReplaceIfNewer`
421428
void BeginFrame(fml::TimePoint frame_time);
422429

423430
//----------------------------------------------------------------------------

shell/common/layer_tree_holder.cc

Lines changed: 0 additions & 32 deletions
This file was deleted.

shell/common/layer_tree_holder.h

Lines changed: 0 additions & 55 deletions
This file was deleted.

shell/common/layer_tree_holder_unittests.cc

Lines changed: 0 additions & 76 deletions
This file was deleted.

shell/common/pipeline.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/shell/common/pipeline.h"
6+
7+
namespace flutter {
8+
9+
size_t GetNextPipelineTraceID() {
10+
static std::atomic_size_t PipelineLastTraceID = {0};
11+
return ++PipelineLastTraceID;
12+
}
13+
14+
} // namespace flutter

0 commit comments

Comments
 (0)