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

Commit 48a5226

Browse files
dkwingsmtdkwingsmt
authored
Move Rasterizer::Draw's discard_callback to Delegate (#44813)
This PR `Rasterizer::Draw`'s `discard_callback` parameter, which is assigned by `Shell`, to `Delegate`, which is also implemented by `Shell`. This refactory makes the API cleaner. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I signed the [CLA]. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent f48bdba commit 48a5226

5 files changed

Lines changed: 62 additions & 62 deletions

File tree

‎shell/common/rasterizer.cc‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ void Rasterizer::DrawLastLayerTree(
195195
}
196196

197197
RasterStatus Rasterizer::Draw(
198-
const std::shared_ptr<LayerTreePipeline>& pipeline,
199-
LayerTreeDiscardCallback discard_callback) {
198+
const std::shared_ptr<LayerTreePipeline>& pipeline) {
200199
TRACE_EVENT0("flutter", "GPURasterizer::Draw");
201200
if (raster_thread_merger_ &&
202201
!raster_thread_merger_->IsOnRasterizingThread()) {
@@ -209,15 +208,16 @@ RasterStatus Rasterizer::Draw(
209208

210209
RasterStatus raster_status = RasterStatus::kFailed;
211210
LayerTreePipeline::Consumer consumer =
212-
[&](std::unique_ptr<LayerTreeItem> item) {
211+
[&raster_status, this,
212+
&delegate = delegate_](std::unique_ptr<LayerTreeItem> item) {
213213
// TODO(dkwingsmt): Use a proper view ID when Rasterizer supports
214214
// multi-view.
215215
int64_t view_id = kFlutterImplicitViewId;
216216
std::unique_ptr<LayerTree> layer_tree = std::move(item->layer_tree);
217217
std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder =
218218
std::move(item->frame_timings_recorder);
219219
float device_pixel_ratio = item->device_pixel_ratio;
220-
if (discard_callback(view_id, *layer_tree.get())) {
220+
if (delegate.ShouldDiscardLayerTree(view_id, *layer_tree.get())) {
221221
raster_status = RasterStatus::kDiscarded;
222222
} else {
223223
raster_status = DoDraw(std::move(frame_timings_recorder),
@@ -259,13 +259,11 @@ RasterStatus Rasterizer::Draw(
259259
switch (consume_result) {
260260
case PipelineConsumeResult::MoreAvailable: {
261261
delegate_.GetTaskRunners().GetRasterTaskRunner()->PostTask(
262-
fml::MakeCopyable(
263-
[weak_this = weak_factory_.GetWeakPtr(), pipeline,
264-
discard_callback = std::move(discard_callback)]() mutable {
265-
if (weak_this) {
266-
weak_this->Draw(pipeline, std::move(discard_callback));
267-
}
268-
}));
262+
[weak_this = weak_factory_.GetWeakPtr(), pipeline]() {
263+
if (weak_this) {
264+
weak_this->Draw(pipeline);
265+
}
266+
});
269267
break;
270268
}
271269
default:

‎shell/common/rasterizer.h‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ class Rasterizer final : public SnapshotDelegate,
119119
const = 0;
120120

121121
virtual const Settings& GetSettings() const = 0;
122+
123+
virtual bool ShouldDiscardLayerTree(int64_t view_id,
124+
const flutter::LayerTree& tree) = 0;
122125
};
123126

124127
//----------------------------------------------------------------------------
@@ -254,9 +257,6 @@ class Rasterizer final : public SnapshotDelegate,
254257

255258
std::shared_ptr<flutter::TextureRegistry> GetTextureRegistry() override;
256259

257-
using LayerTreeDiscardCallback =
258-
std::function<bool(int64_t, flutter::LayerTree&)>;
259-
260260
//----------------------------------------------------------------------------
261261
/// @brief Takes the next item from the layer tree pipeline and executes
262262
/// the raster thread frame workload for that pipeline item to
@@ -285,11 +285,8 @@ class Rasterizer final : public SnapshotDelegate,
285285
///
286286
/// @param[in] pipeline The layer tree pipeline to take the next layer tree
287287
/// to render from.
288-
/// @param[in] discard_callback if specified and returns true, the layer tree
289-
/// is discarded instead of being rendered
290288
///
291-
RasterStatus Draw(const std::shared_ptr<LayerTreePipeline>& pipeline,
292-
LayerTreeDiscardCallback discard_callback = NoDiscard);
289+
RasterStatus Draw(const std::shared_ptr<LayerTreePipeline>& pipeline);
293290

294291
//----------------------------------------------------------------------------
295292
/// @brief The type of the screenshot to obtain of the previously
@@ -585,9 +582,6 @@ class Rasterizer final : public SnapshotDelegate,
585582

586583
void FireNextFrameCallbackIfPresent();
587584

588-
static bool NoDiscard(int64_t view_id, const flutter::LayerTree& layer_tree) {
589-
return false;
590-
}
591585
static bool ShouldResubmitFrame(const RasterStatus& raster_status);
592586

593587
Delegate& delegate_;

‎shell/common/rasterizer_unittests.cc‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ class MockDelegate : public Rasterizer::Delegate {
4343
MOCK_CONST_METHOD0(GetIsGpuDisabledSyncSwitch,
4444
std::shared_ptr<const fml::SyncSwitch>());
4545
MOCK_CONST_METHOD0(GetSettings, const Settings&());
46+
MOCK_METHOD2(ShouldDiscardLayerTree,
47+
bool(int64_t, const flutter::LayerTree&));
4648
};
4749

4850
class MockSurface : public Surface {
@@ -129,7 +131,8 @@ TEST(RasterizerTest, drawEmptyPipeline) {
129131
fml::AutoResetWaitableEvent latch;
130132
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
131133
auto pipeline = std::make_shared<LayerTreePipeline>(/*depth=*/10);
132-
rasterizer->Draw(pipeline, nullptr);
134+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
135+
rasterizer->Draw(pipeline);
133136
latch.Signal();
134137
});
135138
latch.Wait();
@@ -199,8 +202,8 @@ TEST(RasterizerTest,
199202
PipelineProduceResult result =
200203
pipeline->Produce().Complete(std::move(layer_tree_item));
201204
EXPECT_TRUE(result.success);
202-
auto no_discard = [](int64_t, LayerTree&) { return false; };
203-
rasterizer->Draw(pipeline, no_discard);
205+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
206+
rasterizer->Draw(pipeline);
204207
latch.Signal();
205208
});
206209
latch.Wait();
@@ -265,8 +268,8 @@ TEST(
265268
PipelineProduceResult result =
266269
pipeline->Produce().Complete(std::move(layer_tree_item));
267270
EXPECT_TRUE(result.success);
268-
auto no_discard = [](int64_t, LayerTree&) { return false; };
269-
rasterizer->Draw(pipeline, no_discard);
271+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
272+
rasterizer->Draw(pipeline);
270273
latch.Signal();
271274
});
272275
latch.Wait();
@@ -336,8 +339,8 @@ TEST(
336339
PipelineProduceResult result =
337340
pipeline->Produce().Complete(std::move(layer_tree_item));
338341
EXPECT_TRUE(result.success);
339-
auto no_discard = [](int64_t, LayerTree&) { return false; };
340-
rasterizer->Draw(pipeline, no_discard);
342+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
343+
rasterizer->Draw(pipeline);
341344
}
342345

343346
TEST(RasterizerTest,
@@ -410,11 +413,11 @@ TEST(RasterizerTest,
410413
PipelineProduceResult result =
411414
pipeline->Produce().Complete(std::move(layer_tree_item));
412415
EXPECT_TRUE(result.success);
413-
auto no_discard = [](int64_t, LayerTree&) { return false; };
414416

415417
// The Draw() will respectively call BeginFrame(), SubmitFrame() and
416418
// EndFrame() one time.
417-
rasterizer->Draw(pipeline, no_discard);
419+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
420+
rasterizer->Draw(pipeline);
418421

419422
// The DrawLastLayerTree() will respectively call BeginFrame(), SubmitFrame()
420423
// and EndFrame() one more time, totally 2 times.
@@ -460,8 +463,8 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) {
460463
PipelineProduceResult result =
461464
pipeline->Produce().Complete(std::move(layer_tree_item));
462465
EXPECT_TRUE(result.success);
463-
auto no_discard = [](int64_t, LayerTree&) { return false; };
464-
rasterizer->Draw(pipeline, no_discard);
466+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
467+
rasterizer->Draw(pipeline);
465468
latch.Signal();
466469
});
467470
latch.Wait();
@@ -517,8 +520,8 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNotUsedThisFrame) {
517520
pipeline->Produce().Complete(std::move(layer_tree_item));
518521
EXPECT_TRUE(result.success);
519522
// Always discard the layer tree.
520-
auto discard_callback = [](int64_t, LayerTree&) { return true; };
521-
RasterStatus status = rasterizer->Draw(pipeline, discard_callback);
523+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(true));
524+
RasterStatus status = rasterizer->Draw(pipeline);
522525
EXPECT_EQ(status, RasterStatus::kDiscarded);
523526
latch.Signal();
524527
});
@@ -561,8 +564,8 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenPipelineIsEmpty) {
561564
fml::AutoResetWaitableEvent latch;
562565
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
563566
auto pipeline = std::make_shared<LayerTreePipeline>(/*depth=*/10);
564-
auto no_discard = [](int64_t, LayerTree&) { return false; };
565-
RasterStatus status = rasterizer->Draw(pipeline, no_discard);
567+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
568+
RasterStatus status = rasterizer->Draw(pipeline);
566569
EXPECT_EQ(status, RasterStatus::kFailed);
567570
latch.Signal();
568571
});
@@ -619,8 +622,8 @@ TEST(RasterizerTest,
619622
PipelineProduceResult result =
620623
pipeline->Produce().Complete(std::move(layer_tree_item));
621624
EXPECT_TRUE(result.success);
622-
auto no_discard = [](int64_t, LayerTree&) { return false; };
623-
rasterizer->Draw(pipeline, no_discard);
625+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
626+
rasterizer->Draw(pipeline);
624627
latch.Signal();
625628
});
626629
latch.Wait();
@@ -677,8 +680,8 @@ TEST(
677680
PipelineProduceResult result =
678681
pipeline->Produce().Complete(std::move(layer_tree_item));
679682
EXPECT_TRUE(result.success);
680-
auto no_discard = [](int64_t, LayerTree&) { return false; };
681-
RasterStatus status = rasterizer->Draw(pipeline, no_discard);
683+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
684+
RasterStatus status = rasterizer->Draw(pipeline);
682685
EXPECT_EQ(status, RasterStatus::kSuccess);
683686
latch.Signal();
684687
});
@@ -735,8 +738,8 @@ TEST(
735738
PipelineProduceResult result =
736739
pipeline->Produce().Complete(std::move(layer_tree_item));
737740
EXPECT_TRUE(result.success);
738-
auto no_discard = [](int64_t, LayerTree&) { return false; };
739-
RasterStatus status = rasterizer->Draw(pipeline, no_discard);
741+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
742+
RasterStatus status = rasterizer->Draw(pipeline);
740743
EXPECT_EQ(status, RasterStatus::kSuccess);
741744
latch.Signal();
742745
});
@@ -792,8 +795,8 @@ TEST(
792795
PipelineProduceResult result =
793796
pipeline->Produce().Complete(std::move(layer_tree_item));
794797
EXPECT_TRUE(result.success);
795-
auto no_discard = [](int64_t, LayerTree&) { return false; };
796-
RasterStatus status = rasterizer->Draw(pipeline, no_discard);
798+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
799+
RasterStatus status = rasterizer->Draw(pipeline);
797800
EXPECT_EQ(status, RasterStatus::kDiscarded);
798801
latch.Signal();
799802
});
@@ -848,8 +851,8 @@ TEST(
848851
PipelineProduceResult result =
849852
pipeline->Produce().Complete(std::move(layer_tree_item));
850853
EXPECT_TRUE(result.success);
851-
auto no_discard = [](int64_t, LayerTree&) { return false; };
852-
RasterStatus status = rasterizer->Draw(pipeline, no_discard);
854+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
855+
RasterStatus status = rasterizer->Draw(pipeline);
853856
EXPECT_EQ(status, RasterStatus::kFailed);
854857
latch.Signal();
855858
});
@@ -929,10 +932,10 @@ TEST(RasterizerTest,
929932
EXPECT_TRUE(result.success);
930933
EXPECT_EQ(result.is_first_item, i == 0);
931934
}
932-
auto no_discard = [](int64_t, LayerTree&) { return false; };
933935
// Although we only call 'Rasterizer::Draw' once, it will be called twice
934936
// finally because there are two items in the pipeline.
935-
rasterizer->Draw(pipeline, no_discard);
937+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
938+
rasterizer->Draw(pipeline);
936939
});
937940
count_down_latch.Wait();
938941
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
@@ -1100,10 +1103,10 @@ TEST(RasterizerTest, presentationTimeSetWhenVsyncTargetInFuture) {
11001103
EXPECT_TRUE(result.success);
11011104
EXPECT_EQ(result.is_first_item, i == 0);
11021105
}
1103-
auto no_discard = [](int64_t, LayerTree&) { return false; };
11041106
// Although we only call 'Rasterizer::Draw' once, it will be called twice
11051107
// finally because there are two items in the pipeline.
1106-
rasterizer->Draw(pipeline, no_discard);
1108+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
1109+
rasterizer->Draw(pipeline);
11071110
});
11081111

11091112
submit_latch.Wait();
@@ -1181,8 +1184,8 @@ TEST(RasterizerTest, presentationTimeNotSetWhenVsyncTargetInPast) {
11811184
pipeline->Produce().Complete(std::move(layer_tree_item));
11821185
EXPECT_TRUE(result.success);
11831186
EXPECT_EQ(result.is_first_item, true);
1184-
auto no_discard = [](int64_t, LayerTree&) { return false; };
1185-
rasterizer->Draw(pipeline, no_discard);
1187+
ON_CALL(delegate, ShouldDiscardLayerTree).WillByDefault(Return(false));
1188+
rasterizer->Draw(pipeline);
11861189
});
11871190

11881191
submit_latch.Wait();

‎shell/common/shell.cc‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,23 +1218,15 @@ void Shell::OnAnimatorUpdateLatestFrameTargetTime(
12181218
void Shell::OnAnimatorDraw(std::shared_ptr<LayerTreePipeline> pipeline) {
12191219
FML_DCHECK(is_set_up_);
12201220

1221-
auto discard_callback = [this](int64_t view_id, flutter::LayerTree& tree) {
1222-
std::scoped_lock<std::mutex> lock(resize_mutex_);
1223-
auto expected_frame_size = ExpectedFrameSize(view_id);
1224-
return !expected_frame_size.isEmpty() &&
1225-
tree.frame_size() != expected_frame_size;
1226-
};
1227-
12281221
task_runners_.GetRasterTaskRunner()->PostTask(fml::MakeCopyable(
12291222
[&waiting_for_first_frame = waiting_for_first_frame_,
12301223
&waiting_for_first_frame_condition = waiting_for_first_frame_condition_,
12311224
rasterizer = rasterizer_->GetWeakPtr(),
1232-
weak_pipeline = std::weak_ptr<LayerTreePipeline>(pipeline),
1233-
discard_callback = std::move(discard_callback)]() mutable {
1225+
weak_pipeline = std::weak_ptr<LayerTreePipeline>(pipeline)]() mutable {
12341226
if (rasterizer) {
12351227
std::shared_ptr<LayerTreePipeline> pipeline = weak_pipeline.lock();
12361228
if (pipeline) {
1237-
rasterizer->Draw(pipeline, std::move(discard_callback));
1229+
rasterizer->Draw(pipeline);
12381230
}
12391231

12401232
if (waiting_for_first_frame.load()) {
@@ -1591,6 +1583,15 @@ fml::TimePoint Shell::GetLatestFrameTargetTime() const {
15911583
return latest_frame_target_time_.value();
15921584
}
15931585

1586+
// |Rasterizer::Delegate|
1587+
bool Shell::ShouldDiscardLayerTree(int64_t view_id,
1588+
const flutter::LayerTree& tree) {
1589+
std::scoped_lock<std::mutex> lock(resize_mutex_);
1590+
auto expected_frame_size = ExpectedFrameSize(view_id);
1591+
return !expected_frame_size.isEmpty() &&
1592+
tree.frame_size() != expected_frame_size;
1593+
}
1594+
15941595
// |ServiceProtocol::Handler|
15951596
fml::RefPtr<fml::TaskRunner> Shell::GetServiceProtocolHandlerTaskRunner(
15961597
std::string_view method) const {

‎shell/common/shell.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,10 @@ class Shell final : public PlatformView::Delegate,
693693
// |Rasterizer::Delegate|
694694
fml::TimePoint GetLatestFrameTargetTime() const override;
695695

696+
// |Rasterizer::Delegate|
697+
bool ShouldDiscardLayerTree(int64_t view_id,
698+
const flutter::LayerTree& tree) override;
699+
696700
// |ServiceProtocol::Handler|
697701
fml::RefPtr<fml::TaskRunner> GetServiceProtocolHandlerTaskRunner(
698702
std::string_view method) const override;

0 commit comments

Comments
 (0)