Skip to content

Commit 983de2c

Browse files
iskakaushikiskakaushik
authored
Remove pipeline in favor of layer tree holder (flutter#17688)
go/flutter-pipeline-improvements for more details.
1 parent 6767517 commit 983de2c

16 files changed

+204
-498
lines changed

‎ci/licenses_golden/licenses_flutter

+3-3
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
579582
FILE: ../../../flutter/shell/common/persistent_cache.cc
580583
FILE: ../../../flutter/shell/common/persistent_cache.h
581584
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

+3-3
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",
9092
"persistent_cache.cc",
9193
"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",
194195
"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

+5-40
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
#include "flutter/shell/common/animator.h"
6+
#include <memory>
67

78
#include "flutter/fml/trace_event.h"
89
#include "third_party/dart/runtime/include/dart_tools_api.h"
@@ -28,27 +29,15 @@ Animator::Animator(Delegate& delegate,
2829
last_frame_begin_time_(),
2930
last_frame_target_time_(),
3031
dart_frame_deadline_(0),
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
32+
layer_tree_holder_(std::make_shared<LayerTreeHolder>()),
4333
pending_frame_semaphore_(1),
4434
frame_number_(1),
4535
paused_(false),
4636
regenerate_layer_tree_(false),
4737
frame_scheduled_(false),
4838
notify_idle_task_id_(0),
4939
dimension_change_pending_(false),
50-
weak_factory_(this) {
51-
}
40+
weak_factory_(this) {}
5241

5342
Animator::~Animator() = default;
5443

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

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-
136106
last_frame_begin_time_ = frame_start_time;
137107
last_frame_target_time_ = frame_target_time;
138108
dart_frame_deadline_ = FxlToDartOrEarlier(frame_target_time);
@@ -184,13 +154,8 @@ void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
184154
last_frame_target_time_);
185155
}
186156

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_);
157+
layer_tree_holder_->ReplaceIfNewer(std::move(layer_tree));
158+
delegate_.OnAnimatorDraw(layer_tree_holder_);
194159
}
195160

196161
bool Animator::CanReuseLastLayerTree() {

‎shell/common/animator.h

+4-6
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
#define FLUTTER_SHELL_COMMON_ANIMATOR_H_
77

88
#include <deque>
9+
#include <memory>
910

1011
#include "flutter/common/task_runners.h"
1112
#include "flutter/fml/memory/ref_ptr.h"
1213
#include "flutter/fml/memory/weak_ptr.h"
1314
#include "flutter/fml/synchronization/semaphore.h"
1415
#include "flutter/fml/time/time_point.h"
15-
#include "flutter/shell/common/pipeline.h"
16+
#include "flutter/shell/common/layer_tree_holder.h"
1617
#include "flutter/shell/common/rasterizer.h"
1718
#include "flutter/shell/common/vsync_waiter.h"
1819

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

3738
virtual void OnAnimatorDraw(
38-
fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) = 0;
39+
std::shared_ptr<LayerTreeHolder> layer_tree_holder) = 0;
3940

4041
virtual void OnAnimatorDrawLastLayerTree() = 0;
4142
};
@@ -80,8 +81,6 @@ class Animator final {
8081
void EnqueueTraceFlowId(uint64_t trace_flow_id);
8182

8283
private:
83-
using LayerTreePipeline = Pipeline<flutter::LayerTree>;
84-
8584
void BeginFrame(fml::TimePoint frame_start_time,
8685
fml::TimePoint frame_target_time);
8786

@@ -99,9 +98,8 @@ class Animator final {
9998
fml::TimePoint last_frame_begin_time_;
10099
fml::TimePoint last_frame_target_time_;
101100
int64_t dart_frame_deadline_;
102-
fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
101+
std::shared_ptr<LayerTreeHolder> layer_tree_holder_;
103102
fml::Semaphore pending_frame_semaphore_;
104-
LayerTreePipeline::ProducerContinuation producer_continuation_;
105103
int64_t frame_number_;
106104
bool paused_;
107105
bool regenerate_layer_tree_;

‎shell/common/engine.h

+7-14
Original file line numberDiff line numberDiff line change
@@ -406,25 +406,18 @@ 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-
/// * 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.
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.
423415
///
424416
/// @param[in] frame_time The point at which the current frame interval
425417
/// began. May be used by animation interpolators,
426418
/// physics simulations, etc..
427419
///
420+
/// @see `LayerTreeHolder::ReplaceIfNewer`
428421
void BeginFrame(fml::TimePoint frame_time);
429422

430423
//----------------------------------------------------------------------------

‎shell/common/layer_tree_holder.cc

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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/layer_tree_holder.h"
6+
7+
namespace flutter {
8+
9+
std::unique_ptr<LayerTree> LayerTreeHolder::Get() {
10+
std::scoped_lock lock(layer_tree_mutex);
11+
return std::move(layer_tree_);
12+
}
13+
14+
void LayerTreeHolder::ReplaceIfNewer(
15+
std::unique_ptr<LayerTree> proposed_layer_tree) {
16+
std::scoped_lock lock(layer_tree_mutex);
17+
if (!layer_tree_ ||
18+
layer_tree_->target_time() < proposed_layer_tree->target_time()) {
19+
layer_tree_ = std::move(proposed_layer_tree);
20+
}
21+
}
22+
23+
bool LayerTreeHolder::IsEmpty() const {
24+
std::scoped_lock lock(layer_tree_mutex);
25+
return !layer_tree_;
26+
}
27+
28+
}; // namespace flutter

‎shell/common/layer_tree_holder.h

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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+
#ifndef FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_
6+
#define FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_
7+
8+
#include <memory>
9+
10+
#include "flow/layers/layer_tree.h"
11+
12+
namespace flutter {
13+
14+
class LayerTreeHolder {
15+
public:
16+
LayerTreeHolder() = default;
17+
18+
~LayerTreeHolder() = default;
19+
20+
bool IsEmpty() const;
21+
22+
std::unique_ptr<LayerTree> Get();
23+
24+
void ReplaceIfNewer(std::unique_ptr<LayerTree> proposed_layer_tree);
25+
26+
private:
27+
mutable std::mutex layer_tree_mutex;
28+
std::unique_ptr<LayerTree> layer_tree_;
29+
30+
FML_DISALLOW_COPY_AND_ASSIGN(LayerTreeHolder);
31+
};
32+
33+
}; // namespace flutter
34+
35+
#endif // FLUTTER_SHELL_COMMON_LAYER_TREE_HOLDER_H_

‎shell/common/layer_tree_holder_unittests.cc

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
#define FML_USED_ON_EMBEDDER
6+
7+
#include <functional>
8+
#include <future>
9+
#include <memory>
10+
11+
#include "flutter/shell/common/layer_tree_holder.h"
12+
#include "gtest/gtest.h"
13+
14+
namespace flutter {
15+
namespace testing {
16+
17+
TEST(LayerTreeHolder, EmptyOnInit) {
18+
const LayerTreeHolder layer_tree_holder;
19+
ASSERT_TRUE(layer_tree_holder.IsEmpty());
20+
}
21+
22+
TEST(LayerTreeHolder, PutOneAndGet) {
23+
LayerTreeHolder layer_tree_holder;
24+
const auto frame_size = SkISize::Make(64, 64);
25+
auto layer_tree = std::make_unique<LayerTree>(frame_size, 100.0f, 1.0f);
26+
layer_tree_holder.ReplaceIfNewer(std::move(layer_tree));
27+
ASSERT_FALSE(layer_tree_holder.IsEmpty());
28+
const auto stored = layer_tree_holder.Get();
29+
ASSERT_EQ(stored->frame_size(), frame_size);
30+
ASSERT_TRUE(layer_tree_holder.IsEmpty());
31+
}
32+
33+
TEST(LayerTreeHolder, PutMultiGetsLatest) {
34+
const auto build_begin = fml::TimePoint::Now();
35+
const auto target_time_1 = build_begin + fml::TimeDelta::FromSeconds(2);
36+
const auto target_time_2 = build_begin + fml::TimeDelta::FromSeconds(5);
37+
38+
LayerTreeHolder layer_tree_holder;
39+
const auto frame_size_1 = SkISize::Make(64, 64);
40+
auto layer_tree_1 = std::make_unique<LayerTree>(frame_size_1, 100.0f, 1.0f);
41+
layer_tree_1->RecordBuildTime(build_begin, target_time_1);
42+
layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_1));
43+
44+
const auto frame_size_2 = SkISize::Make(128, 128);
45+
auto layer_tree_2 = std::make_unique<LayerTree>(frame_size_2, 100.0f, 1.0f);
46+
layer_tree_2->RecordBuildTime(build_begin, target_time_2);
47+
layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_2));
48+
49+
const auto stored = layer_tree_holder.Get();
50+
ASSERT_EQ(stored->frame_size(), frame_size_2);
51+
ASSERT_TRUE(layer_tree_holder.IsEmpty());
52+
}
53+
54+
TEST(LayerTreeHolder, RetainsOlderIfNewerFrameHasEarlierTargetTime) {
55+
const auto build_begin = fml::TimePoint::Now();
56+
const auto target_time_1 = build_begin + fml::TimeDelta::FromSeconds(5);
57+
const auto target_time_2 = build_begin + fml::TimeDelta::FromSeconds(2);
58+
59+
LayerTreeHolder layer_tree_holder;
60+
const auto frame_size_1 = SkISize::Make(64, 64);
61+
auto layer_tree_1 = std::make_unique<LayerTree>(frame_size_1, 100.0f, 1.0f);
62+
layer_tree_1->RecordBuildTime(build_begin, target_time_1);
63+
layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_1));
64+
65+
const auto frame_size_2 = SkISize::Make(128, 128);
66+
auto layer_tree_2 = std::make_unique<LayerTree>(frame_size_2, 100.0f, 1.0f);
67+
layer_tree_2->RecordBuildTime(build_begin, target_time_2);
68+
layer_tree_holder.ReplaceIfNewer(std::move(layer_tree_2));
69+
70+
const auto stored = layer_tree_holder.Get();
71+
ASSERT_EQ(stored->frame_size(), frame_size_1);
72+
ASSERT_TRUE(layer_tree_holder.IsEmpty());
73+
}
74+
75+
} // namespace testing
76+
} // namespace flutter

‎shell/common/pipeline.cc

-14
This file was deleted.

0 commit comments

Comments
 (0)