Skip to content

Commit 994b854

Browse files
authored
Revert "Reland "Remove pipeline in favor of layer tree holder" (flutter#24947)" (flutter#25027)
1 parent 525271d commit 994b854

17 files changed

Lines changed: 539 additions & 297 deletions

ci/licenses_golden/licenses_flutter

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -643,10 +643,10 @@ FILE: ../../../flutter/shell/common/engine_unittests.cc
643643
FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
644644
FILE: ../../../flutter/shell/common/fixtures/shelltest_screenshot.png
645645
FILE: ../../../flutter/shell/common/input_events_unittests.cc
646-
FILE: ../../../flutter/shell/common/layer_tree_holder.cc
647-
FILE: ../../../flutter/shell/common/layer_tree_holder.h
648-
FILE: ../../../flutter/shell/common/layer_tree_holder_unittests.cc
649646
FILE: ../../../flutter/shell/common/persistent_cache_unittests.cc
647+
FILE: ../../../flutter/shell/common/pipeline.cc
648+
FILE: ../../../flutter/shell/common/pipeline.h
649+
FILE: ../../../flutter/shell/common/pipeline_unittests.cc
650650
FILE: ../../../flutter/shell/common/platform_view.cc
651651
FILE: ../../../flutter/shell/common/platform_view.h
652652
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
@@ -69,8 +69,8 @@ source_set("common") {
6969
"display_manager.h",
7070
"engine.cc",
7171
"engine.h",
72-
"layer_tree_holder.cc",
73-
"layer_tree_holder.h",
72+
"pipeline.cc",
73+
"pipeline.h",
7474
"platform_view.cc",
7575
"platform_view.h",
7676
"pointer_data_dispatcher.cc",
@@ -242,8 +242,8 @@ if (enable_unittests) {
242242
"canvas_spy_unittests.cc",
243243
"engine_unittests.cc",
244244
"input_events_unittests.cc",
245-
"layer_tree_holder_unittests.cc",
246245
"persistent_cache_unittests.cc",
246+
"pipeline_unittests.cc",
247247
"rasterizer_unittests.cc",
248248
"shell_unittests.cc",
249249
"skp_shader_warmup_unittests.cc",

shell/common/animator.cc

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,27 @@ Animator::Animator(Delegate& delegate,
2929
last_vsync_start_time_(),
3030
last_frame_target_time_(),
3131
dart_frame_deadline_(0),
32-
layer_tree_holder_(std::make_shared<LayerTreeHolder>()),
32+
#if SHELL_ENABLE_METAL
33+
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
34+
#else // SHELL_ENABLE_METAL
35+
// TODO(dnfield): We should remove this logic and set the pipeline depth
36+
// back to 2 in this case. See
37+
// https://github.com/flutter/engine/pull/9132 for discussion.
38+
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
39+
task_runners.GetPlatformTaskRunner() ==
40+
task_runners.GetRasterTaskRunner()
41+
? 1
42+
: 2)),
43+
#endif // SHELL_ENABLE_METAL
3344
pending_frame_semaphore_(1),
3445
frame_number_(1),
3546
paused_(false),
3647
regenerate_layer_tree_(false),
3748
frame_scheduled_(false),
3849
notify_idle_task_id_(0),
3950
dimension_change_pending_(false),
40-
weak_factory_(this) {}
51+
weak_factory_(this) {
52+
}
4153

4254
Animator::~Animator() = default;
4355

@@ -100,6 +112,25 @@ void Animator::BeginFrame(fml::TimePoint vsync_start_time,
100112
regenerate_layer_tree_ = false;
101113
pending_frame_semaphore_.Signal();
102114

115+
if (!producer_continuation_) {
116+
// We may already have a valid pipeline continuation in case a previous
117+
// begin frame did not result in an Animation::Render. Simply reuse that
118+
// instead of asking the pipeline for a fresh continuation.
119+
producer_continuation_ = layer_tree_pipeline_->Produce();
120+
121+
if (!producer_continuation_) {
122+
// If we still don't have valid continuation, the pipeline is currently
123+
// full because the consumer is being too slow. Try again at the next
124+
// frame interval.
125+
RequestFrame();
126+
return;
127+
}
128+
}
129+
130+
// We have acquired a valid continuation from the pipeline and are ready
131+
// to service potential frame.
132+
FML_DCHECK(producer_continuation_);
133+
103134
last_frame_begin_time_ = fml::TimePoint::Now();
104135
last_vsync_start_time_ = vsync_start_time;
105136
fml::tracing::TraceEventAsyncComplete("flutter", "VsyncSchedulingOverhead",
@@ -153,8 +184,13 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
153184
layer_tree->RecordBuildTime(last_vsync_start_time_, last_frame_begin_time_,
154185
last_frame_target_time_);
155186

156-
layer_tree_holder_->PushIfNewer(std::move(layer_tree));
157-
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_);
158194
}
159195

160196
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

@@ -103,8 +104,9 @@ class Animator final {
103104
fml::TimePoint last_vsync_start_time_;
104105
fml::TimePoint last_frame_target_time_;
105106
int64_t dart_frame_deadline_;
106-
std::shared_ptr<LayerTreeHolder> layer_tree_holder_;
107+
fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
107108
fml::Semaphore pending_frame_semaphore_;
109+
LayerTreePipeline::ProducerContinuation producer_continuation_;
108110
int64_t frame_number_;
109111
bool paused_;
110112
bool regenerate_layer_tree_;

shell/common/engine.h

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -477,18 +477,25 @@ class Engine final : public RuntimeDelegate,
477477
/// will cause the jank in the Flutter application:
478478
/// * The time taken by this method to create a layer-tree exceeds
479479
/// on frame interval (for example, 16.66 ms on a 60Hz display).
480-
/// * A new layer-tree produced by this method replaces a stale
481-
/// layer tree in `LayerTreeHolder`. See:
482-
/// `LayerTreeHolder::ReplaceIfNewer`. This could happen if
483-
/// rasterizer takes more than one frame interval to rasterize a
484-
/// layer tree. This would cause some frames to be skipped and
485-
/// could result in perceptible jank.
480+
/// * The time take by this method to generate a new layer-tree
481+
/// causes the current layer-tree pipeline depth to change. To
482+
/// illustrate this point, note that maximum pipeline depth used
483+
/// by layer tree in the engine is 2. If both the UI and GPU
484+
/// task runner tasks finish within one frame interval, the
485+
/// pipeline depth is one. If the UI thread happens to be
486+
/// working on a frame when the raster thread is still not done
487+
/// with the previous frame, the pipeline depth is 2. When the
488+
/// pipeline depth changes from 1 to 2, animations and UI
489+
/// interactions that cause the generation of the new layer tree
490+
/// appropriate for (frame_time + one frame interval) will
491+
/// actually end up at (frame_time + two frame intervals). This
492+
/// is not what code running on the UI thread expected would
493+
/// happen. This causes perceptible jank.
486494
///
487495
/// @param[in] frame_time The point at which the current frame interval
488496
/// began. May be used by animation interpolators,
489497
/// physics simulations, etc..
490498
///
491-
/// @see `LayerTreeHolder::ReplaceIfNewer`
492499
void BeginFrame(fml::TimePoint frame_time);
493500

494501
// |HintFreedDelegate|

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)