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

Do not involve external_view_embedder in submit frame process if threads are not merged. #22275

Merged
merged 7 commits into from
Nov 6, 2020
Merged

Do not involve external_view_embedder in submit frame process if threads are not merged. #22275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1a9d81
Do not involve external_view_embedder in submit frame process if thre…
Nov 3, 2020
8fa56f3
link todo to issue
Nov 5, 2020
4a1e8f5
rasterizer tests
Nov 5, 2020
b357f74
ensure thread initlized
Nov 5, 2020
dbb7b63
reviews
Nov 6, 2020
ed75ac6
Merge branch 'master' into escape
Nov 6, 2020
ce7e971
merge mater
Nov 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions shell/common/rasterizer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
consume_result = PipelineConsumeResult::MoreAvailable;
}

// Merging the thread as we know the next `Draw` should be run on the platform
// thread.
// EndFrame should perform cleanups for the external_view_embedder.
if (external_view_embedder_) {
external_view_embedder_->EndFrame(should_resubmit_frame,
raster_thread_merger_);
Expand Down Expand Up @@ -467,7 +466,8 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
raster_status == RasterStatus::kSkipAndRetry) {
return raster_status;
}
if (external_view_embedder_) {
if (external_view_embedder_ != nullptr &&
(!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
FML_DCHECK(!frame->IsSubmitted());
external_view_embedder_->SubmitFrame(surface_->GetContext(),
std::move(frame));
Expand Down
142 changes: 135 additions & 7 deletions shell/common/rasterizer_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#define FML_USED_ON_EMBEDDER

#include "flutter/shell/common/rasterizer.h"

#include "flutter/shell/common/thread_host.h"
#include "flutter/testing/testing.h"
#include "gmock/gmock.h"

using testing::_;
using testing::ByMove;
using testing::Return;
using testing::ReturnRef;

Expand Down Expand Up @@ -92,7 +95,8 @@ TEST(RasterizerTest, drawEmptyPipeline) {
latch.Wait();
}

TEST(RasterizerTest, drawWithExternalViewEmbedder) {
TEST(RasterizerTest,
drawWithExternalViewEmbedder_ExternalViewEmbedderSubmitFrameCalled) {
std::string test_name =
::testing::UnitTest::GetInstance()->current_test_info()->name();
ThreadHost thread_host("io.flutter.test." + test_name + ".",
Expand All @@ -111,11 +115,81 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
std::make_shared<MockExternalViewEmbedder>();
rasterizer->SetExternalViewEmbedder(external_view_embedder);

auto surface_frame = std::make_unique<SurfaceFrame>(
/*surface=*/nullptr, /*supports_readback=*/true,
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
.WillOnce(Return(ByMove(std::move(surface_frame))));

EXPECT_CALL(*external_view_embedder,
BeginFrame(SkISize(), nullptr, 2.0,
fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
/*device_pixel_ratio=*/2.0,
/*raster_thread_merger=*/
fml::RefPtr<fml::RasterThreadMerger>(nullptr)))
.Times(1);
EXPECT_CALL(*external_view_embedder, SubmitFrame).Times(1);
EXPECT_CALL(
*external_view_embedder,
EndFrame(/*should_resubmit_frame=*/false,
/*raster_thread_merger=*/fml::RefPtr<fml::RasterThreadMerger>(
nullptr)))
.Times(1);

rasterizer->Setup(std::move(surface));
fml::AutoResetWaitableEvent latch;
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
/*device_pixel_ratio=*/2.0f);
bool result = pipeline->Produce().Complete(std::move(layer_tree));
EXPECT_TRUE(result);
auto no_discard = [](LayerTree&) { return false; };
rasterizer->Draw(pipeline, no_discard);
latch.Signal();
});
latch.Wait();
}

TEST(
RasterizerTest,
drawWithExternalViewEmbedderAndThreadMergerNotMerged_ExternalViewEmbedderSubmitFrameNotCalled) {
std::string test_name =
::testing::UnitTest::GetInstance()->current_test_info()->name();
ThreadHost thread_host("io.flutter.test." + test_name + ".",
ThreadHost::Type::Platform | ThreadHost::Type::GPU |
ThreadHost::Type::IO | ThreadHost::Type::UI);
TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(),
thread_host.raster_thread->GetTaskRunner(),
thread_host.ui_thread->GetTaskRunner(),
thread_host.io_thread->GetTaskRunner());
MockDelegate delegate;
EXPECT_CALL(delegate, GetTaskRunners())
.WillRepeatedly(ReturnRef(task_runners));
EXPECT_CALL(delegate, OnFrameRasterized(_));
auto rasterizer = std::make_unique<Rasterizer>(delegate);
auto surface = std::make_unique<MockSurface>();
std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
std::make_shared<MockExternalViewEmbedder>();
rasterizer->SetExternalViewEmbedder(external_view_embedder);
EXPECT_CALL(*external_view_embedder, SupportsDynamicThreadMerging)
.WillRepeatedly(Return(true));
auto surface_frame = std::make_unique<SurfaceFrame>(
/*surface=*/nullptr, /*supports_readback=*/true,
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
.WillOnce(Return(ByMove(std::move(surface_frame))));

EXPECT_CALL(*external_view_embedder,
EndFrame(false, fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
/*device_pixel_ratio=*/2.0,
/*raster_thread_merger=*/_))
.Times(1);
EXPECT_CALL(*external_view_embedder, SubmitFrame).Times(0);
EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
/*raster_thread_merger=*/_))
.Times(1);

rasterizer->Setup(std::move(surface));
fml::AutoResetWaitableEvent latch;
thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
Expand All @@ -124,12 +198,66 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
/*device_pixel_ratio=*/2.0f);
bool result = pipeline->Produce().Complete(std::move(layer_tree));
EXPECT_TRUE(result);
std::function<bool(LayerTree&)> no_discard = [](LayerTree&) {
return false;
};
auto no_discard = [](LayerTree&) { return false; };
rasterizer->Draw(pipeline, no_discard);
latch.Signal();
});
latch.Wait();
}

TEST(
RasterizerTest,
drawWithExternalViewEmbedderAndThreadsMerged_ExternalViewEmbedderSubmitFrameCalled) {
std::string test_name =
::testing::UnitTest::GetInstance()->current_test_info()->name();
ThreadHost thread_host("io.flutter.test." + test_name + ".",
ThreadHost::Type::Platform | ThreadHost::Type::GPU |
ThreadHost::Type::IO | ThreadHost::Type::UI);
fml::MessageLoop::EnsureInitializedForCurrentThread();
TaskRunners task_runners("test",
fml::MessageLoop::GetCurrent().GetTaskRunner(),
fml::MessageLoop::GetCurrent().GetTaskRunner(),
thread_host.ui_thread->GetTaskRunner(),
thread_host.io_thread->GetTaskRunner());

MockDelegate delegate;
EXPECT_CALL(delegate, GetTaskRunners())
.WillRepeatedly(ReturnRef(task_runners));
EXPECT_CALL(delegate, OnFrameRasterized(_));

auto rasterizer = std::make_unique<Rasterizer>(delegate);
auto surface = std::make_unique<MockSurface>();

std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
std::make_shared<MockExternalViewEmbedder>();
rasterizer->SetExternalViewEmbedder(external_view_embedder);

auto surface_frame = std::make_unique<SurfaceFrame>(
/*surface=*/nullptr, /*supports_readback=*/true,
/*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
EXPECT_CALL(*surface, AcquireFrame(SkISize()))
.WillOnce(Return(ByMove(std::move(surface_frame))));
EXPECT_CALL(*external_view_embedder, SupportsDynamicThreadMerging)
.WillRepeatedly(Return(true));

EXPECT_CALL(*external_view_embedder,
BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
/*device_pixel_ratio=*/2.0,
/*raster_thread_merger=*/_))
.Times(1);
EXPECT_CALL(*external_view_embedder, SubmitFrame).Times(1);
EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
/*raster_thread_merger=*/_))
.Times(1);

rasterizer->Setup(std::move(surface));

auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
/*device_pixel_ratio=*/2.0f);
bool result = pipeline->Produce().Complete(std::move(layer_tree));
EXPECT_TRUE(result);
auto no_discard = [](LayerTree&) { return false; };
rasterizer->Draw(pipeline, no_discard);
}
} // namespace flutter
63 changes: 47 additions & 16 deletions shell/common/shell_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,20 +1051,25 @@ TEST_F(ShellTest,
auto settings = CreateSettingsForFixture();
fml::AutoResetWaitableEvent end_frame_latch;
std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;

fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
auto end_frame_callback =
[&](bool should_resubmit_frame,
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
external_view_embedder->UpdatePostPrerollResult(
PostPrerollResult::kSuccess);
if (!raster_thread_merger_ref) {
raster_thread_merger_ref = raster_thread_merger;
Copy link

@blasten blasten Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future improvements: this would be much easier if we were to create, and supply the thread merger in the test. Right now, this dependency is hardcoded in the rasterizer, which creates patterns like this.

}
if (should_resubmit_frame && !raster_thread_merger->IsMerged()) {
raster_thread_merger->MergeWithLease(10);
external_view_embedder->UpdatePostPrerollResult(
PostPrerollResult::kSuccess);
}
end_frame_latch.Signal();
};
external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
end_frame_callback, PostPrerollResult::kResubmitFrame, true);

auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
false, external_view_embedder);

PlatformViewNotifyCreated(shell.get());

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

PumpOneFrame(shell.get());
// `EndFrame` changed the post preroll result to `kSuccess`.
// `EndFrame` changed the post preroll result to `kSuccess` and merged the
// threads. During the frame, the threads are not merged, So no
// `external_view_embedder->GetSubmittedFrameCount()` is called.
end_frame_latch.Wait();
ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
ASSERT_TRUE(raster_thread_merger_ref->IsMerged());
ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());

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

PlatformViewNotifyDestroyed(shell.get());
DestroyShell(std::move(shell));
}

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

fml::AutoResetWaitableEvent end_frame_latch;
std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
auto end_frame_callback =
[&](bool should_merge_thread,
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
if (!raster_thread_merger_ref) {
raster_thread_merger_ref = raster_thread_merger;
}
if (should_merge_thread) {
// TODO(cyanglaz): This test used external_view_embedder so we need to
// merge the threads here. However, the scenario it is testing is
// unrelated to platform views. We should consider to update this test
// so it doesn't require external_view_embedder.
// https://github.com/flutter/flutter/issues/69895
raster_thread_merger->MergeWithLease(10);
external_view_embedder->UpdatePostPrerollResult(
PostPrerollResult::kSuccess);
}
end_frame_latch.Signal();
};

auto end_frame_callback = [&](bool, fml::RefPtr<fml::RasterThreadMerger>) {
end_frame_latch.Signal();
};

std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder =
std::make_shared<ShellTestExternalViewEmbedder>(
std::move(end_frame_callback), PostPrerollResult::kSuccess, true);
external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
std::move(end_frame_callback), PostPrerollResult::kResubmitFrame, true);

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

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

fml::WeakPtr<RuntimeDelegate> runtime_delegate = shell->GetEngine();

PumpOneFrame(shell.get(), static_cast<double>(wrong_size.width()),
static_cast<double>(wrong_size.height()));

end_frame_latch.Wait();

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

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

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

end_frame_latch.Wait();
ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
Copy link

@blasten blasten Nov 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is it possible to assert that threads are merged before this assert? Same where you have the comment // Threads not merged, can that be an assert?

Copy link
Contributor Author

@cyanglaz cyanglaz Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

@cyanglaz cyanglaz Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we can't assert if threads are not merged here, the threads are merged at the end of the frame. So it is actually merged here.
My comment was misleading, it should say that during the frame, the threads are not merged, they are merged at the end of the frame.
Updated the comment, PTAL

ASSERT_EQ(expected_size, external_view_embedder->GetLastSubmittedFrameSize());

PlatformViewNotifyDestroyed(shell.get());
DestroyShell(std::move(shell));
}

Expand Down
25 changes: 13 additions & 12 deletions shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@

void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack,
UIView* embedded_view) {
if (flutter_view_ == nullptr) {
return;
}
FML_DCHECK(CATransform3DEqualToTransform(embedded_view.layer.transform, CATransform3DIdentity));
ResetAnchor(embedded_view.layer);
ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview;
Expand Down Expand Up @@ -398,7 +401,6 @@

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

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

// Do nothing if the view doesn't need to be composited.
if (views_to_recomposite_.count(view_id) == 0) {
Expand Down Expand Up @@ -469,12 +470,11 @@
bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context,
std::shared_ptr<IOSContext> ios_context,
std::unique_ptr<SurfaceFrame> frame) {
FML_DCHECK(flutter_view_);

// Any UIKit related code has to run on main thread.
// When on a non-main thread, we only allow the rest of the method to run if there is no
// Pending UIView operations.
FML_DCHECK([[NSThread currentThread] isMainThread] || !HasPlatformViewThisOrNextFrame());
FML_DCHECK([[NSThread currentThread] isMainThread]);
if (flutter_view_ == nullptr) {
return frame->Submit();
}

DisposeViews();

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

did_submit &= frame->Submit();

Expand Down Expand Up @@ -599,7 +597,10 @@

void FlutterPlatformViewsController::EndFrame(
bool should_resubmit_frame,
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {}
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
// Reset the composition order, so next frame starts empty.
composition_order_.clear();
}

std::shared_ptr<FlutterPlatformViewLayer> FlutterPlatformViewsController::GetLayer(
GrDirectContext* gr_context,
Expand Down
Loading