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

Commit fb72023

Browse files
author
Chris Yang
authored
Reland "Do not involve external_view_embedder in submit frame process if threads are not merged. #22275" (#22372)
1 parent 2352672 commit fb72023

File tree

5 files changed

+261
-38
lines changed

5 files changed

+261
-38
lines changed

‎shell/common/rasterizer.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
190190
consume_result = PipelineConsumeResult::MoreAvailable;
191191
}
192192

193-
// Merging the thread as we know the next `Draw` should be run on the platform
194-
// thread.
193+
// EndFrame should perform cleanups for the external_view_embedder.
195194
if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) {
196195
surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame,
197196
raster_thread_merger_);
@@ -469,7 +468,8 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
469468
raster_status == RasterStatus::kSkipAndRetry) {
470469
return raster_status;
471470
}
472-
if (external_view_embedder != nullptr) {
471+
if (external_view_embedder != nullptr &&
472+
(!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
473473
FML_DCHECK(!frame->IsSubmitted());
474474
external_view_embedder->SubmitFrame(surface_->GetContext(),
475475
std::move(frame));

‎shell/common/rasterizer_unittests.cc‎

Lines changed: 136 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
#define FML_USED_ON_EMBEDDER
6+
57
#include "flutter/shell/common/rasterizer.h"
68

79
#include "flutter/shell/common/thread_host.h"
810
#include "flutter/testing/testing.h"
911
#include "gmock/gmock.h"
1012

1113
using testing::_;
14+
using testing::ByMove;
1215
using testing::Return;
1316
using testing::ReturnRef;
1417

@@ -92,7 +95,8 @@ TEST(RasterizerTest, drawEmptyPipeline) {
9295
latch.Wait();
9396
}
9497

95-
TEST(RasterizerTest, drawWithExternalViewEmbedder) {
98+
TEST(RasterizerTest,
99+
drawWithExternalViewEmbedderExternalViewEmbedderSubmitFrameCalled) {
96100
std::string test_name =
97101
::testing::UnitTest::GetInstance()->current_test_info()->name();
98102
ThreadHost thread_host("io.flutter.test." + test_name + ".",
@@ -108,14 +112,85 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
108112
EXPECT_CALL(delegate, OnFrameRasterized(_));
109113
auto rasterizer = std::make_unique<Rasterizer>(delegate);
110114
auto surface = std::make_unique<MockSurface>();
115+
111116
MockExternalViewEmbedder external_view_embedder;
112117
EXPECT_CALL(*surface, GetExternalViewEmbedder())
113118
.WillRepeatedly(Return(&external_view_embedder));
119+
120+
auto surface_frame = std::make_unique<SurfaceFrame>(
121+
/*surface=*/nullptr, /*supports_readback=*/true,
122+
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
123+
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
124+
.WillOnce(Return(ByMove(std::move(surface_frame))));
125+
114126
EXPECT_CALL(external_view_embedder,
115-
BeginFrame(SkISize(), nullptr, 2.0,
116-
fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
127+
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
128+
/*device_pixel_ratio=*/2.0,
129+
/*raster_thread_merger=*/
130+
fml::RefPtr<fml::RasterThreadMerger>(nullptr)))
131+
.Times(1);
132+
EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
133+
EXPECT_CALL(
134+
external_view_embedder,
135+
EndFrame(/*should_resubmit_frame=*/false,
136+
/*raster_thread_merger=*/fml::RefPtr<fml::RasterThreadMerger>(
137+
nullptr)))
138+
.Times(1);
139+
140+
rasterizer->Setup(std::move(surface));
141+
fml::AutoResetWaitableEvent latch;
142+
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
143+
auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
144+
auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
145+
/*device_pixel_ratio=*/2.0f);
146+
bool result = pipeline->Produce().Complete(std::move(layer_tree));
147+
EXPECT_TRUE(result);
148+
auto no_discard = [](LayerTree&) { return false; };
149+
rasterizer->Draw(pipeline, no_discard);
150+
latch.Signal();
151+
});
152+
latch.Wait();
153+
}
154+
155+
TEST(
156+
RasterizerTest,
157+
drawWithExternalViewEmbedderAndThreadMergerNotMergedExternalViewEmbedderSubmitFrameNotCalled) {
158+
std::string test_name =
159+
::testing::UnitTest::GetInstance()->current_test_info()->name();
160+
ThreadHost thread_host("io.flutter.test." + test_name + ".",
161+
ThreadHost::Type::Platform | ThreadHost::Type::GPU |
162+
ThreadHost::Type::IO | ThreadHost::Type::UI);
163+
TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(),
164+
thread_host.raster_thread->GetTaskRunner(),
165+
thread_host.ui_thread->GetTaskRunner(),
166+
thread_host.io_thread->GetTaskRunner());
167+
MockDelegate delegate;
168+
EXPECT_CALL(delegate, GetTaskRunners())
169+
.WillRepeatedly(ReturnRef(task_runners));
170+
EXPECT_CALL(delegate, OnFrameRasterized(_));
171+
auto rasterizer = std::make_unique<Rasterizer>(delegate);
172+
auto surface = std::make_unique<MockSurface>();
173+
MockExternalViewEmbedder external_view_embedder;
174+
EXPECT_CALL(*surface, GetExternalViewEmbedder())
175+
.WillRepeatedly(Return(&external_view_embedder));
176+
EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging)
177+
.WillRepeatedly(Return(true));
178+
auto surface_frame = std::make_unique<SurfaceFrame>(
179+
/*surface=*/nullptr, /*supports_readback=*/true,
180+
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
181+
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
182+
.WillOnce(Return(ByMove(std::move(surface_frame))));
183+
117184
EXPECT_CALL(external_view_embedder,
118-
EndFrame(false, fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
185+
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
186+
/*device_pixel_ratio=*/2.0,
187+
/*raster_thread_merger=*/_))
188+
.Times(1);
189+
EXPECT_CALL(external_view_embedder, SubmitFrame).Times(0);
190+
EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
191+
/*raster_thread_merger=*/_))
192+
.Times(1);
193+
119194
rasterizer->Setup(std::move(surface));
120195
fml::AutoResetWaitableEvent latch;
121196
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
@@ -124,12 +199,66 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
124199
/*device_pixel_ratio=*/2.0f);
125200
bool result = pipeline->Produce().Complete(std::move(layer_tree));
126201
EXPECT_TRUE(result);
127-
std::function<bool(LayerTree&)> no_discard = [](LayerTree&) {
128-
return false;
129-
};
202+
auto no_discard = [](LayerTree&) { return false; };
130203
rasterizer->Draw(pipeline, no_discard);
131204
latch.Signal();
132205
});
133206
latch.Wait();
134207
}
208+
209+
TEST(
210+
RasterizerTest,
211+
drawWithExternalViewEmbedderAndThreadsMergedExternalViewEmbedderSubmitFrameCalled) {
212+
std::string test_name =
213+
::testing::UnitTest::GetInstance()->current_test_info()->name();
214+
ThreadHost thread_host("io.flutter.test." + test_name + ".",
215+
ThreadHost::Type::Platform | ThreadHost::Type::GPU |
216+
ThreadHost::Type::IO | ThreadHost::Type::UI);
217+
fml::MessageLoop::EnsureInitializedForCurrentThread();
218+
TaskRunners task_runners("test",
219+
fml::MessageLoop::GetCurrent().GetTaskRunner(),
220+
fml::MessageLoop::GetCurrent().GetTaskRunner(),
221+
thread_host.ui_thread->GetTaskRunner(),
222+
thread_host.io_thread->GetTaskRunner());
223+
224+
MockDelegate delegate;
225+
EXPECT_CALL(delegate, GetTaskRunners())
226+
.WillRepeatedly(ReturnRef(task_runners));
227+
EXPECT_CALL(delegate, OnFrameRasterized(_));
228+
229+
auto rasterizer = std::make_unique<Rasterizer>(delegate);
230+
auto surface = std::make_unique<MockSurface>();
231+
232+
MockExternalViewEmbedder external_view_embedder;
233+
EXPECT_CALL(*surface, GetExternalViewEmbedder())
234+
.WillRepeatedly(Return(&external_view_embedder));
235+
236+
auto surface_frame = std::make_unique<SurfaceFrame>(
237+
/*surface=*/nullptr, /*supports_readback=*/true,
238+
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
239+
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
240+
.WillOnce(Return(ByMove(std::move(surface_frame))));
241+
EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging)
242+
.WillRepeatedly(Return(true));
243+
244+
EXPECT_CALL(external_view_embedder,
245+
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
246+
/*device_pixel_ratio=*/2.0,
247+
/*raster_thread_merger=*/_))
248+
.Times(1);
249+
EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
250+
EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
251+
/*raster_thread_merger=*/_))
252+
.Times(1);
253+
254+
rasterizer->Setup(std::move(surface));
255+
256+
auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
257+
auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
258+
/*device_pixel_ratio=*/2.0f);
259+
bool result = pipeline->Produce().Complete(std::move(layer_tree));
260+
EXPECT_TRUE(result);
261+
auto no_discard = [](LayerTree&) { return false; };
262+
rasterizer->Draw(pipeline, no_discard);
263+
}
135264
} // namespace flutter

‎shell/common/shell_unittests.cc‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,20 +1051,25 @@ TEST_F(ShellTest,
10511051
auto settings = CreateSettingsForFixture();
10521052
fml::AutoResetWaitableEvent end_frame_latch;
10531053
std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
1054-
1054+
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
10551055
auto end_frame_callback =
10561056
[&](bool should_resubmit_frame,
10571057
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
1058-
external_view_embedder->UpdatePostPrerollResult(
1059-
PostPrerollResult::kSuccess);
1058+
if (!raster_thread_merger_ref) {
1059+
raster_thread_merger_ref = raster_thread_merger;
1060+
}
1061+
if (should_resubmit_frame && !raster_thread_merger->IsMerged()) {
1062+
raster_thread_merger->MergeWithLease(10);
1063+
external_view_embedder->UpdatePostPrerollResult(
1064+
PostPrerollResult::kSuccess);
1065+
}
10601066
end_frame_latch.Signal();
10611067
};
10621068
external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
10631069
end_frame_callback, PostPrerollResult::kResubmitFrame, true);
10641070

10651071
auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
10661072
false, external_view_embedder);
1067-
10681073
PlatformViewNotifyCreated(shell.get());
10691074

10701075
auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -1074,13 +1079,18 @@ TEST_F(ShellTest,
10741079
ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
10751080

10761081
PumpOneFrame(shell.get());
1077-
// `EndFrame` changed the post preroll result to `kSuccess`.
1082+
// `EndFrame` changed the post preroll result to `kSuccess` and merged the
1083+
// threads. During the frame, the threads are not merged, So no
1084+
// `external_view_embedder->GetSubmittedFrameCount()` is called.
10781085
end_frame_latch.Wait();
1079-
ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
1086+
ASSERT_TRUE(raster_thread_merger_ref->IsMerged());
1087+
ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
10801088

1089+
// This is the resubmitted frame, which threads are also merged.
10811090
end_frame_latch.Wait();
1082-
ASSERT_EQ(2, external_view_embedder->GetSubmittedFrameCount());
1091+
ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
10831092

1093+
PlatformViewNotifyDestroyed(shell.get());
10841094
DestroyShell(std::move(shell));
10851095
}
10861096

@@ -2020,14 +2030,29 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
20202030
SkISize expected_size = SkISize::Make(400, 200);
20212031

20222032
fml::AutoResetWaitableEvent end_frame_latch;
2033+
std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
2034+
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
2035+
auto end_frame_callback =
2036+
[&](bool should_merge_thread,
2037+
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
2038+
if (!raster_thread_merger_ref) {
2039+
raster_thread_merger_ref = raster_thread_merger;
2040+
}
2041+
if (should_merge_thread) {
2042+
// TODO(cyanglaz): This test used external_view_embedder so we need to
2043+
// merge the threads here. However, the scenario it is testing is
2044+
// unrelated to platform views. We should consider to update this test
2045+
// so it doesn't require external_view_embedder.
2046+
// https://github.com/flutter/flutter/issues/69895
2047+
raster_thread_merger->MergeWithLease(10);
2048+
external_view_embedder->UpdatePostPrerollResult(
2049+
PostPrerollResult::kSuccess);
2050+
}
2051+
end_frame_latch.Signal();
2052+
};
20232053

2024-
auto end_frame_callback = [&](bool, fml::RefPtr<fml::RasterThreadMerger>) {
2025-
end_frame_latch.Signal();
2026-
};
2027-
2028-
std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder =
2029-
std::make_shared<ShellTestExternalViewEmbedder>(
2030-
std::move(end_frame_callback), PostPrerollResult::kSuccess, true);
2054+
external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
2055+
std::move(end_frame_callback), PostPrerollResult::kResubmitFrame, true);
20312056

20322057
std::unique_ptr<Shell> shell = CreateShell(
20332058
settings, GetTaskRunnersForFixture(), false, external_view_embedder);
@@ -2048,23 +2073,29 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
20482073

20492074
RunEngine(shell.get(), std::move(configuration));
20502075

2051-
fml::WeakPtr<RuntimeDelegate> runtime_delegate = shell->GetEngine();
2052-
20532076
PumpOneFrame(shell.get(), static_cast<double>(wrong_size.width()),
20542077
static_cast<double>(wrong_size.height()));
20552078

20562079
end_frame_latch.Wait();
20572080

20582081
ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
20592082

2083+
// Threads will be merged at the end of this frame.
20602084
PumpOneFrame(shell.get(), static_cast<double>(expected_size.width()),
20612085
static_cast<double>(expected_size.height()));
20622086

20632087
end_frame_latch.Wait();
2088+
// Even the threads are merged at the end of the frame,
2089+
// during the frame, the threads are not merged,
2090+
// So no `external_view_embedder->GetSubmittedFrameCount()` is called.
2091+
ASSERT_TRUE(raster_thread_merger_ref->IsMerged());
2092+
ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
20642093

2094+
end_frame_latch.Wait();
20652095
ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
20662096
ASSERT_EQ(expected_size, external_view_embedder->GetLastSubmittedFrameSize());
20672097

2098+
PlatformViewNotifyDestroyed(shell.get());
20682099
DestroyShell(std::move(shell));
20692100
}
20702101

‎shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,9 @@
342342

343343
void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack,
344344
UIView* embedded_view) {
345+
if (flutter_view_ == nullptr) {
346+
return;
347+
}
345348
FML_DCHECK(CATransform3DEqualToTransform(embedded_view.layer.transform, CATransform3DIdentity));
346349
ResetAnchor(embedded_view.layer);
347350
ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview;
@@ -398,7 +401,6 @@
398401

399402
void FlutterPlatformViewsController::CompositeWithParams(int view_id,
400403
const EmbeddedViewParams& params) {
401-
FML_DCHECK(flutter_view_);
402404
CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height());
403405
UIView* touchInterceptor = touch_interceptors_[view_id].get();
404406
touchInterceptor.layer.transform = CATransform3DIdentity;
@@ -419,9 +421,8 @@
419421
}
420422

421423
SkCanvas* FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) {
422-
FML_DCHECK(flutter_view_);
423-
// TODO(amirh): assert that this is running on the platform thread once we support the iOS
424-
// embedded views thread configuration.
424+
// Any UIKit related code has to run on main thread.
425+
FML_DCHECK([[NSThread currentThread] isMainThread]);
425426

426427
// Do nothing if the view doesn't need to be composited.
427428
if (views_to_recomposite_.count(view_id) == 0) {
@@ -469,12 +470,11 @@
469470
bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context,
470471
std::shared_ptr<IOSContext> ios_context,
471472
std::unique_ptr<SurfaceFrame> frame) {
472-
FML_DCHECK(flutter_view_);
473-
474473
// Any UIKit related code has to run on main thread.
475-
// When on a non-main thread, we only allow the rest of the method to run if there is no
476-
// Pending UIView operations.
477-
FML_DCHECK([[NSThread currentThread] isMainThread] || !HasPlatformViewThisOrNextFrame());
474+
FML_DCHECK([[NSThread currentThread] isMainThread]);
475+
if (flutter_view_ == nullptr) {
476+
return frame->Submit();
477+
}
478478

479479
DisposeViews();
480480

@@ -558,8 +558,6 @@
558558
BringLayersIntoView(platform_view_layers);
559559
// Mark all layers as available, so they can be used in the next frame.
560560
layer_pool_->RecycleLayers();
561-
// Reset the composition order, so next frame starts empty.
562-
composition_order_.clear();
563561

564562
did_submit &= frame->Submit();
565563

@@ -599,7 +597,10 @@
599597

600598
void FlutterPlatformViewsController::EndFrame(
601599
bool should_resubmit_frame,
602-
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {}
600+
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
601+
// Reset the composition order, so next frame starts empty.
602+
composition_order_.clear();
603+
}
603604

604605
std::shared_ptr<FlutterPlatformViewLayer> FlutterPlatformViewsController::GetLayer(
605606
GrDirectContext* gr_context,

0 commit comments

Comments
 (0)