From f9651489f31f34227b12b7b268461e5b26cb0ee7 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 27 Mar 2024 11:55:40 -0700 Subject: [PATCH 1/9] Impl --- flow/embedded_views.cc | 1 + flow/embedded_views.h | 7 +- flow/testing/mock_embedder.cc | 3 +- flow/testing/mock_embedder.h | 3 +- shell/common/rasterizer.cc | 7 +- shell/common/rasterizer_unittests.cc | 73 ++++++++++--------- .../shell_test_external_view_embedder.cc | 2 +- .../shell_test_external_view_embedder.h | 4 +- .../external_view_embedder.cc | 8 +- .../external_view_embedder.h | 4 +- .../external_view_embedder_unittests.cc | 48 ++++++------ .../darwin/ios/ios_external_view_embedder.h | 4 +- .../darwin/ios/ios_external_view_embedder.mm | 11 ++- .../embedder_external_view_embedder.cc | 11 +-- .../embedder_external_view_embedder.h | 4 +- .../fuchsia/flutter/external_view_embedder.cc | 10 +-- .../fuchsia/flutter/external_view_embedder.h | 4 +- shell/testing/tester_main.cc | 3 +- 18 files changed, 103 insertions(+), 104 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 1907762ca0584..ddb85acd2b0c7 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -43,6 +43,7 @@ bool DisplayListEmbedderViewSlice::recording_ended() { } void ExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 5aad6605aaceb..34c71cd9f4019 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -456,16 +456,19 @@ class ExternalViewEmbedder { virtual DlCanvas* CompositeEmbeddedView(int64_t platform_view_id) = 0; // Prepare for a view to be drawn. - virtual void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + virtual void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) = 0; + // Submits the content stored since |PrepareFlutterView| to the specified + // Flutter view. + // // Implementers must submit the frame by calling frame.Submit(). // // This method can mutate the root Skia canvas before submitting the frame. // // It can also allocate frames for overlay surfaces to compose hybrid views. virtual void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame); diff --git a/flow/testing/mock_embedder.cc b/flow/testing/mock_embedder.cc index 3cec9fecdbf34..2e74d2691602d 100644 --- a/flow/testing/mock_embedder.cc +++ b/flow/testing/mock_embedder.cc @@ -29,8 +29,7 @@ void MockViewEmbedder::BeginFrame( const fml::RefPtr& raster_thread_merger) {} // |ExternalViewEmbedder| -void MockViewEmbedder::PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, +void MockViewEmbedder::PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) {} // |ExternalViewEmbedder| diff --git a/flow/testing/mock_embedder.h b/flow/testing/mock_embedder.h index b9038d8af6ffa..1812083fd8960 100644 --- a/flow/testing/mock_embedder.h +++ b/flow/testing/mock_embedder.h @@ -30,8 +30,7 @@ class MockViewEmbedder : public ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index ed7bee5a9376c..060d928f10bca 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -663,8 +663,8 @@ DrawSurfaceStatus Rasterizer::DrawToSurfaceUnsafe( DlCanvas* embedder_root_canvas = nullptr; if (external_view_embedder_) { - external_view_embedder_->PrepareFlutterView( - view_id, layer_tree.frame_size(), device_pixel_ratio); + external_view_embedder_->PrepareFlutterView(layer_tree.frame_size(), + device_pixel_ratio); // TODO(dkwingsmt): Add view ID here. embedder_root_canvas = external_view_embedder_->GetRootCanvas(); } @@ -752,7 +752,8 @@ DrawSurfaceStatus Rasterizer::DrawToSurfaceUnsafe( (!raster_thread_merger_ || raster_thread_merger_->IsMerged())) { FML_DCHECK(!frame->IsSubmitted()); external_view_embedder_->SubmitFlutterView( - surface_->GetContext(), surface_->GetAiksContext(), std::move(frame)); + view_id, surface_->GetContext(), surface_->GetAiksContext(), + std::move(frame)); } else { frame->Submit(); } diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 7265bb887578f..a43d92551971a 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -97,9 +97,7 @@ class MockExternalViewEmbedder : public ExternalViewEmbedder { (override)); MOCK_METHOD(void, PrepareFlutterView, - (int64_t flutter_view_id, - SkISize frame_size, - double device_pixel_ratio), + (SkISize frame_size, double device_pixel_ratio), (override)); MOCK_METHOD(void, PrerollCompositeEmbeddedView, @@ -113,7 +111,8 @@ class MockExternalViewEmbedder : public ExternalViewEmbedder { MOCK_METHOD(DlCanvas*, CompositeEmbeddedView, (int64_t view_id), (override)); MOCK_METHOD(void, SubmitFlutterView, - (GrDirectContext * context, + (int64_t flutter_view_id, + GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame), (override)); @@ -223,12 +222,13 @@ TEST(RasterizerTest, /*raster_thread_merger=*/ fml::RefPtr(nullptr))) .Times(1); + EXPECT_CALL(*external_view_embedder, PrepareFlutterView( + /*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) + .Times(1); EXPECT_CALL(*external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/kImplicitViewId, - /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + SubmitFlutterView(/*flutter_view_id=*/kImplicitViewId, _, _, _)) .Times(1); - EXPECT_CALL(*external_view_embedder, SubmitFlutterView).Times(1); EXPECT_CALL( *external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, @@ -299,12 +299,13 @@ TEST( EXPECT_CALL(*external_view_embedder, BeginFrame(/*context=*/nullptr, /*raster_thread_merger=*/_)) .Times(1); - EXPECT_CALL(*external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/kImplicitViewId, - /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + EXPECT_CALL(*external_view_embedder, PrepareFlutterView( + /*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) .Times(1); - EXPECT_CALL(*external_view_embedder, SubmitFlutterView).Times(0); + EXPECT_CALL(*external_view_embedder, + SubmitFlutterView(/*flutter_view_id=*/kImplicitViewId, _, _, _)) + .Times(0); EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, /*raster_thread_merger=*/_)) .Times(1); @@ -378,12 +379,13 @@ TEST( EXPECT_CALL(*external_view_embedder, BeginFrame(/*context=*/nullptr, /*raster_thread_merger=*/_)) .Times(1); + EXPECT_CALL(*external_view_embedder, PrepareFlutterView( + /*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) + .Times(1); EXPECT_CALL(*external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/kImplicitViewId, - /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + SubmitFlutterView(/*flutter_view_id=*/kImplicitViewId, _, _, _)) .Times(1); - EXPECT_CALL(*external_view_embedder, SubmitFlutterView).Times(1); EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, /*raster_thread_merger=*/_)) .Times(1); @@ -460,12 +462,13 @@ TEST(RasterizerTest, EXPECT_CALL(*external_view_embedder, BeginFrame(/*context=*/nullptr, /*raster_thread_merger=*/_)) .Times(2); + EXPECT_CALL(*external_view_embedder, PrepareFlutterView( + /*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) + .Times(2); EXPECT_CALL(*external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/kImplicitViewId, - /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + SubmitFlutterView(/*flutter_view_id=*/kImplicitViewId, _, _, _)) .Times(2); - EXPECT_CALL(*external_view_embedder, SubmitFlutterView).Times(2); EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, /*raster_thread_merger=*/_)) .Times(2); @@ -575,10 +578,9 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNotUsedThisFrame) { EXPECT_CALL(*external_view_embedder, BeginFrame(/*context=*/nullptr, /*raster_thread_merger=*/_)) .Times(0); - EXPECT_CALL(*external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/kImplicitViewId, - /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + EXPECT_CALL(*external_view_embedder, PrepareFlutterView( + /*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) .Times(0); EXPECT_CALL( *external_view_embedder, @@ -696,17 +698,20 @@ TEST(RasterizerTest, drawMultipleViewsWithExternalViewEmbedder) { EXPECT_CALL(*external_view_embedder, BeginFrame(/*context=*/nullptr, /*raster_thread_merger=*/_)) .Times(1); - EXPECT_CALL( - *external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/0, /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/1.5)) + EXPECT_CALL(*external_view_embedder, + PrepareFlutterView(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/1.5)) .Times(1); - EXPECT_CALL( - *external_view_embedder, - PrepareFlutterView(/*flutter_view_id=*/1, /*frame_size=*/SkISize(), - /*device_pixel_ratio=*/2.0)) + EXPECT_CALL(*external_view_embedder, + SubmitFlutterView(/*flutter_view_id=*/0, _, _, _)) + .Times(1); + EXPECT_CALL(*external_view_embedder, + PrepareFlutterView(/*frame_size=*/SkISize(), + /*device_pixel_ratio=*/2.0)) + .Times(1); + EXPECT_CALL(*external_view_embedder, + SubmitFlutterView(/*flutter_view_id=*/1, _, _, _)) .Times(1); - EXPECT_CALL(*external_view_embedder, SubmitFlutterView).Times(2); EXPECT_CALL(*external_view_embedder, EndFrame(/*should_resubmit_frame=*/false, /*raster_thread_merger=*/_)) .Times(1); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index d03a2a57cde89..dd351b5dfc605 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -46,7 +46,6 @@ void ShellTestExternalViewEmbedder::BeginFrame( // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::PrepareFlutterView( - int64_t flutter_view_id, SkISize frame_size, double device_pixel_ratio) { visited_platform_views_.clear(); @@ -94,6 +93,7 @@ DlCanvas* ShellTestExternalViewEmbedder::CompositeEmbeddedView( // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index d9af75d575b40..b000c239435fb 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -51,8 +51,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| @@ -78,6 +77,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) override; diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 2ec5a9ac198a7..f6ce9eb161273 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -64,10 +64,14 @@ SkRect AndroidExternalViewEmbedder::GetViewRect(int64_t view_id) const { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::SubmitFlutterView"); + // TODO(dkwingsmt): This class only supports rendering into the implicit view. + // Properly support multi-view in the future. + FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); if (!FrameHasPlatformLayers()) { frame->Submit(); @@ -268,12 +272,8 @@ void AndroidExternalViewEmbedder::BeginFrame( // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::PrepareFlutterView( - int64_t flutter_view_id, SkISize frame_size, double device_pixel_ratio) { - // TODO(dkwingsmt): This class only supports rendering into the implicit view. - // Properly support multi-view in the future. - FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); Reset(); // The surface size changed. Therefore, destroy existing surfaces as diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index d601a52fcaccd..ab00870276ffc 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -44,6 +44,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) override; @@ -62,8 +63,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index 0cb9f7ac11824..57eec1e60d2eb 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -147,7 +147,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(10, 20), 1.0); + embedder->PrepareFlutterView(SkISize::Make(10, 20), 1.0); // Push a platform view. embedder->PrerollCompositeEmbeddedView( @@ -201,7 +201,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(100, 100), 1.5); + embedder->PrepareFlutterView(SkISize::Make(100, 100), 1.5); MutatorsStack stack; SkMatrix matrix; @@ -229,7 +229,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRectChangedParams) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(100, 100), 1.5); + embedder->PrepareFlutterView(SkISize::Make(100, 100), 1.5); auto view_id = 0; @@ -316,7 +316,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); // Submits frame if no Android view in the current frame. EXPECT_TRUE(did_submit_frame); @@ -332,7 +332,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); // Add an Android view. MutatorsStack stack1; @@ -386,7 +386,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); // Doesn't submit frame if there aren't Android views in the previous frame. EXPECT_FALSE(did_submit_frame); @@ -402,7 +402,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); // Add an Android view. MutatorsStack stack1; @@ -453,7 +453,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); // Submits frame if there are Android views in the previous frame. EXPECT_TRUE(did_submit_frame); @@ -508,7 +508,7 @@ TEST(AndroidExternalViewEmbedder, OverlayCoverTwoPlatformViews) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); { // Add first Android view. @@ -562,7 +562,7 @@ TEST(AndroidExternalViewEmbedder, OverlayCoverTwoPlatformViews) { }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); @@ -608,7 +608,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); { // Add first Android view. @@ -667,7 +667,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) { }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); @@ -713,7 +713,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); { // Add Android view. @@ -737,7 +737,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) { }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); @@ -758,7 +758,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(10, 20), 1.0); + embedder->PrepareFlutterView(SkISize::Make(10, 20), 1.0); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(0); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); @@ -807,7 +807,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); // Add an Android view. MutatorsStack stack1; @@ -840,7 +840,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); @@ -851,7 +851,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); // Change the frame size. embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(30, 40), 1.0); + embedder->PrepareFlutterView(SkISize::Make(30, 40), 1.0); } TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { @@ -897,7 +897,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { GetThreadMergerFromPlatformThread(&rasterizer_thread); EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); // Add an Android view. MutatorsStack stack1; @@ -929,7 +929,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); @@ -942,7 +942,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { fml::Thread platform_thread("platform"); embedder->BeginFrame(nullptr, GetThreadMergerFromRasterThread(&platform_thread)); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(30, 40), 1.0); + embedder->PrepareFlutterView(SkISize::Make(30, 40), 1.0); } TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { @@ -969,7 +969,7 @@ TEST(AndroidExternalViewEmbedder, DisableThreadMerger) { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, SkISize::Make(10, 20), 1.0); + embedder->PrepareFlutterView(SkISize::Make(10, 20), 1.0); // Push a platform view. embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -1019,7 +1019,7 @@ TEST(AndroidExternalViewEmbedder, Teardown) { GetThreadMergerFromPlatformThread(&rasterizer_thread); embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(kImplicitViewId, frame_size, 1.5); + embedder->PrepareFlutterView(frame_size, 1.5); // Add an Android view. MutatorsStack stack; @@ -1043,7 +1043,7 @@ TEST(AndroidExternalViewEmbedder, Teardown) { SkSurfaces::Null(1000, 1000), framebuffer_info, [](const SurfaceFrame& surface_frame, DlCanvas* canvas) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(gr_context.get(), nullptr, + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.h b/shell/platform/darwin/ios/ios_external_view_embedder.h index 8a47eaea31639..953966378b15a 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.h +++ b/shell/platform/darwin/ios/ios_external_view_embedder.h @@ -36,8 +36,7 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| @@ -55,6 +54,7 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) override; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index 2e13055c2de01..9af5904a4e092 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -37,12 +37,7 @@ const fml::RefPtr& raster_thread_merger) {} // |ExternalViewEmbedder| -void IOSExternalViewEmbedder::PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, - double device_pixel_ratio) { - // TODO(dkwingsmt): This class only supports rendering into the implicit view. - // Properly support multi-view in the future. - FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); +void IOSExternalViewEmbedder::PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) { FML_CHECK(platform_views_controller_); platform_views_controller_->BeginFrame(frame_size); } @@ -74,10 +69,14 @@ // |ExternalViewEmbedder| void IOSExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::SubmitFlutterView"); + // TODO(dkwingsmt): This class only supports rendering into the implicit view. + // Properly support multi-view in the future. + FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); FML_CHECK(platform_views_controller_); platform_views_controller_->SubmitFrame(context, ios_context_, std::move(frame)); TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::DidSubmitFrame"); diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index fe699575e77ea..1e9ddf90c4390 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -59,13 +59,8 @@ void EmbedderExternalViewEmbedder::BeginFrame( // |ExternalViewEmbedder| void EmbedderExternalViewEmbedder::PrepareFlutterView( - int64_t flutter_view_id, SkISize frame_size, double device_pixel_ratio) { - // TODO(dkwingsmt): This class only supports rendering into the implicit - // view. Properly support multi-view in the future. - // https://github.com/flutter/flutter/issues/135530 item 4 - FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); Reset(); pending_frame_size_ = frame_size; @@ -417,6 +412,7 @@ class LayerBuilder { }; // namespace void EmbedderExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { @@ -494,10 +490,7 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( builder.PushLayers(presented_layers); - // TODO(loic-sharma): Currently only supports a single view. - // See https://github.com/flutter/flutter/issues/135530. - presented_layers.InvokePresentCallback(kFlutterImplicitViewId, - present_callback_); + presented_layers.InvokePresentCallback(flutter_view_id, present_callback_); } // See why this is necessary in the comment where this collection in diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index d850fa85e8443..ef51cc1b245ab 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -87,8 +87,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| @@ -101,6 +100,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) override; diff --git a/shell/platform/fuchsia/flutter/external_view_embedder.cc b/shell/platform/fuchsia/flutter/external_view_embedder.cc index af50b423f9d58..4f630c75e2c79 100644 --- a/shell/platform/fuchsia/flutter/external_view_embedder.cc +++ b/shell/platform/fuchsia/flutter/external_view_embedder.cc @@ -114,12 +114,8 @@ void ExternalViewEmbedder::BeginFrame( const fml::RefPtr& raster_thread_merger) {} // |ExternalViewEmbedder| -void ExternalViewEmbedder::PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, +void ExternalViewEmbedder::PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) { - // Fuchsia only supports operating the implicit view for now. - FML_DCHECK(flutter_view_id == flutter::kFlutterImplicitViewId); - // Reset for new view. Reset(); frame_size_ = frame_size; @@ -139,9 +135,13 @@ void ExternalViewEmbedder::EndFrame( } void ExternalViewEmbedder::SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { + // Fuchsia only supports operating the implicit view for now. + FML_DCHECK(flutter_view_id == flutter::kFlutterImplicitViewId); + TRACE_EVENT0("flutter", "ExternalViewEmbedder::SubmitFlutterView"); std::vector> frame_surfaces; std::unordered_map frame_surface_indices; diff --git a/shell/platform/fuchsia/flutter/external_view_embedder.h b/shell/platform/fuchsia/flutter/external_view_embedder.h index 382cadce9c0a2..5c00272499063 100644 --- a/shell/platform/fuchsia/flutter/external_view_embedder.h +++ b/shell/platform/fuchsia/flutter/external_view_embedder.h @@ -77,8 +77,7 @@ class ExternalViewEmbedder final : public flutter::ExternalViewEmbedder { raster_thread_merger) override; // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override; // |ExternalViewEmbedder| @@ -88,6 +87,7 @@ class ExternalViewEmbedder final : public flutter::ExternalViewEmbedder { // |ExternalViewEmbedder| void SubmitFlutterView( + int64_t flutter_view_id, GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) override; diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index d1a206f72e6b1..dac86e7c8d33e 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -159,8 +159,7 @@ class TesterExternalViewEmbedder : public ExternalViewEmbedder { raster_thread_merger) override {} // |ExternalViewEmbedder| - void PrepareFlutterView(int64_t flutter_view_id, - SkISize frame_size, + void PrepareFlutterView(SkISize frame_size, double device_pixel_ratio) override {} // |ExternalViewEmbedder| From b3effc565623afce9d63833af4915c7a4e0d63a3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 27 Mar 2024 12:39:41 -0700 Subject: [PATCH 2/9] Impl: multiview backing store --- flow/embedded_views.cc | 2 + flow/embedded_views.h | 9 ++++ shell/common/rasterizer.cc | 1 + shell/common/rasterizer.h | 9 ++-- shell/platform/embedder/embedder.h | 3 ++ .../embedder_external_view_embedder.cc | 41 +++++++++++-------- .../embedder_external_view_embedder.h | 5 ++- 7 files changed, 49 insertions(+), 21 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index ddb85acd2b0c7..b0d17870a707b 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -42,6 +42,8 @@ bool DisplayListEmbedderViewSlice::recording_ended() { return builder_ == nullptr; } +void ExternalViewEmbedder::CollectView(int64_t view_id) {} + void ExternalViewEmbedder::SubmitFlutterView( int64_t flutter_view_id, GrDirectContext* context, diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 34c71cd9f4019..f49a32c832afa 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -421,6 +421,15 @@ class ExternalViewEmbedder { virtual ~ExternalViewEmbedder() = default; + // Deallocate the resources for displaying a view. + // + // This method must be called when a view is removed from the engine. + // + // When the ExternalViewEmbedder is requested to draw an unrecognized view, it + // implicitly allocates necessary resources. These resources must be + // explicitly deallocated. + virtual void CollectView(int64_t view_id); + // Usually, the root canvas is not owned by the view embedder. However, if // the view embedder wants to provide a canvas to the rasterizer, it may // return one here. This canvas takes priority over the canvas materialized diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 060d928f10bca..9c011808da0ba 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -184,6 +184,7 @@ void Rasterizer::NotifyLowMemoryWarning() const { } void Rasterizer::CollectView(int64_t view_id) { + external_view_embedder_->CollectView(view_id); view_records_.erase(view_id); } diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 04620957ed383..d04b6cd8f9119 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -261,11 +261,12 @@ class Rasterizer final : public SnapshotDelegate, //---------------------------------------------------------------------------- /// @brief Deallocate the resources for displaying a view. /// - /// This method must be called when a view is removed. + /// This method must be called when a view is removed from the + /// engine. /// - /// The rasterizer don't need views to be registered. Last-frame - /// states for views are recorded when layer trees are rasterized - /// to the view and used during `Rasterizer::DrawLastLayerTrees`. + /// When the rasterizer is requested to draw an unrecognized view, + /// it implicitly allocates necessary resources. These resources + /// must be explicitly deallocated. /// /// @param[in] view_id The ID of the view. /// diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index d26e2215adf3a..7dc69ad0f25fb 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1731,6 +1731,9 @@ typedef struct { size_t struct_size; /// The size of the render target the engine expects to render into. FlutterSize size; + /// The identifier for the view that the engine will use this backing store to + /// render into. + FlutterViewId view_id; } FlutterBackingStoreConfig; typedef enum { diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index 1e9ddf90c4390..a3664d7a0f9de 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -29,6 +29,10 @@ EmbedderExternalViewEmbedder::EmbedderExternalViewEmbedder( EmbedderExternalViewEmbedder::~EmbedderExternalViewEmbedder() = default; +void EmbedderExternalViewEmbedder::CollectView(int64_t view_id) { + render_target_caches_.erase(view_id); +} + void EmbedderExternalViewEmbedder::SetSurfaceTransformationCallback( SurfaceTransformationCallback surface_transformation_callback) { surface_transformation_callback_ = std::move(surface_transformation_callback); @@ -114,6 +118,7 @@ DlCanvas* EmbedderExternalViewEmbedder::CompositeEmbeddedView(int64_t view_id) { } static FlutterBackingStoreConfig MakeBackingStoreConfig( + int64_t view_id, const SkISize& backing_store_size) { FlutterBackingStoreConfig config = {}; @@ -121,6 +126,7 @@ static FlutterBackingStoreConfig MakeBackingStoreConfig( config.size.width = backing_store_size.width(); config.size.height = backing_store_size.height(); + config.view_id = view_id; return config; } @@ -284,6 +290,10 @@ class Layer { /// Implements https://flutter.dev/go/optimized-platform-view-layers class LayerBuilder { public: + using RenderTargetProvider = + std::function( + const SkISize& frame_size)>; + explicit LayerBuilder(SkISize frame_size) : frame_size_(frame_size) { layers_.push_back(Layer()); } @@ -304,13 +314,10 @@ class LayerBuilder { } /// Prepares the render targets for all layers that have Flutter contents. - void PrepareBackingStore( - const std::function( - FlutterBackingStoreConfig)>& target_provider) { - auto config = MakeBackingStoreConfig(frame_size_); + void PrepareBackingStore(RenderTargetProvider target_provider) { for (auto& layer : layers_) { if (layer.has_flutter_contents()) { - layer.SetRenderTarget(target_provider(config)); + layer.SetRenderTarget(target_provider(frame_size_)); } } } @@ -416,6 +423,9 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { + // Get the view's cache, or create one if the view doesn't have a cache yet. + EmbedderRenderTargetCache& render_target_cache = + render_target_caches_[flutter_view_id]; SkRect _rect = SkRect::MakeIWH(pending_frame_size_.width(), pending_frame_size_.height()); pending_surface_transformation_.mapRect(&_rect); @@ -427,17 +437,16 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( builder.AddExternalView(view.get()); } - builder.PrepareBackingStore([&](FlutterBackingStoreConfig config) { - std::unique_ptr target; + builder.PrepareBackingStore([&](const SkISize& frame_size) { if (!avoid_backing_store_cache_) { - target = render_target_cache_.GetRenderTarget( - EmbedderExternalView::RenderTargetDescriptor( - SkISize{static_cast(config.size.width), - static_cast(config.size.height)})); - } - if (target != nullptr) { - return target; + std::unique_ptr target = + render_target_cache.GetRenderTarget( + EmbedderExternalView::RenderTargetDescriptor(frame_size)); + if (target != nullptr) { + return target; + } } + auto config = MakeBackingStoreConfig(flutter_view_id, frame_size); return create_render_target_callback_(context, aiks_context, config); }); @@ -454,7 +463,7 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( // // @warning: Embedder may trample on our OpenGL context here. auto deferred_cleanup_render_targets = - render_target_cache_.ClearAllRenderTargetsInCache(); + render_target_cache.ClearAllRenderTargetsInCache(); // The OpenGL context could have been trampled by the embedder at this point // as it attempted to collect old render targets and create new ones. Tell @@ -502,7 +511,7 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( auto render_targets = builder.ClearAndCollectRenderTargets(); for (auto& render_target : render_targets) { if (!avoid_backing_store_cache_) { - render_target_cache_.CacheRenderTarget(std::move(render_target)); + render_target_cache.CacheRenderTarget(std::move(render_target)); } } diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index ef51cc1b245ab..5b8ebc22481a2 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -66,6 +66,9 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { /// ~EmbedderExternalViewEmbedder() override; + // |ExternalViewEmbedder| + void CollectView(int64_t view_id) override; + //---------------------------------------------------------------------------- /// @brief Sets the surface transformation callback used by the external /// view embedder to ask the platform for the per frame root @@ -118,7 +121,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { SkMatrix pending_surface_transformation_; EmbedderExternalView::PendingViews pending_views_; std::vector composition_order_; - EmbedderRenderTargetCache render_target_cache_; + std::unordered_map render_target_caches_; void Reset(); From 51f87c26eb894ba4b393ce45ddc07d1e60043d52 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 28 Mar 2024 12:08:18 -0700 Subject: [PATCH 3/9] Tidy errors --- shell/platform/embedder/embedder_external_view_embedder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index a3664d7a0f9de..5c2202bf23cef 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -314,7 +314,7 @@ class LayerBuilder { } /// Prepares the render targets for all layers that have Flutter contents. - void PrepareBackingStore(RenderTargetProvider target_provider) { + void PrepareBackingStore(const RenderTargetProvider& target_provider) { for (auto& layer : layers_) { if (layer.has_flutter_contents()) { layer.SetRenderTarget(target_provider(frame_size_)); From bac9c6df83ebf1733e9ab581b7147fc465ddf146 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 3 Apr 2024 10:33:27 -0700 Subject: [PATCH 4/9] Fix collect view with no eve --- shell/common/rasterizer.cc | 4 +++- shell/common/rasterizer.h | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 9c011808da0ba..1c2cfd2e6a4bf 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -184,7 +184,9 @@ void Rasterizer::NotifyLowMemoryWarning() const { } void Rasterizer::CollectView(int64_t view_id) { - external_view_embedder_->CollectView(view_id); + if (external_view_embedder_) { + external_view_embedder_->CollectView(view_id); + } view_records_.erase(view_id); } diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index d04b6cd8f9119..949402257f4a1 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -261,8 +261,8 @@ class Rasterizer final : public SnapshotDelegate, //---------------------------------------------------------------------------- /// @brief Deallocate the resources for displaying a view. /// - /// This method must be called when a view is removed from the - /// engine. + /// This method must be called on the raster task runner when a + /// view is removed from the engine. /// /// When the rasterizer is requested to draw an unrecognized view, /// it implicitly allocates necessary resources. These resources From 44d26c42ae2221b1bccee7a73aa9ea29da17c21c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 3 Apr 2024 15:28:16 -0700 Subject: [PATCH 5/9] Add unit test --- shell/platform/embedder/embedder.h | 8 ++ .../embedder/tests/embedder_unittests.cc | 108 ++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 563151a033716..702b1d4923043 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1896,9 +1896,13 @@ typedef struct { /// `FlutterBackingStore::struct_size` when specifying a new backing store to /// the engine. This only matters if the embedder expects to be used with /// engines older than the version whose headers it used during compilation. + /// + /// The callback should return true if the operation was successful. FlutterBackingStoreCreateCallback create_backing_store_callback; /// A callback invoked by the engine to release the backing store. The /// embedder may collect any resources associated with the backing store. + /// + /// The callback should return true if the operation was successful. FlutterBackingStoreCollectCallback collect_backing_store_callback; /// Callback invoked by the engine to composite the contents of each layer /// onto the implicit view. @@ -1910,6 +1914,8 @@ typedef struct { /// Only one of `present_layers_callback` and `present_view_callback` may be /// provided. Providing both is an error and engine initialization will /// terminate. + /// + /// The callback should return true if the operation was successful. FlutterLayersPresentCallback present_layers_callback; /// Avoid caching backing stores provided by this compositor. bool avoid_backing_store_cache; @@ -1919,6 +1925,8 @@ typedef struct { /// Only one of `present_layers_callback` and `present_view_callback` may be /// provided. Providing both is an error and engine initialization will /// terminate. + /// + /// The callback should return true if the operation was successful. FlutterPresentViewCallback present_view_callback; } FlutterCompositor; diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 17110a8adf67f..768abfff2264d 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1814,6 +1814,114 @@ TEST_F(EmbedderTest, CanRenderMultipleViews) { latch123.Wait(); } +TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { + // This test involves two views: the implicit view, and the OtherView as + // defined here. + constexpr FlutterViewId kOtherViewId = 123; + auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + + EmbedderConfigBuilder builder(context); + builder.SetDartEntrypoint("render_all_views"); + builder.SetSoftwareRendererConfig(SkISize::Make(800, 600)); + builder.SetCompositor(); + + EmbedderTestBackingStoreProducer producer( + context.GetCompositor().GetGrContext(), + EmbedderTestBackingStoreProducer::RenderTargetType::kSoftwareBuffer); + + struct CompositorUserData { + // The two latches wait for the signal for the two views. The first waits + // for the implicit view. The second waits for the OtherView. + fml::AutoResetWaitableEvent latch_implicit; + fml::AutoResetWaitableEvent latch_other; + EmbedderTestBackingStoreProducer* producer; + } compositor_user_data; + + compositor_user_data.producer = &producer; + + builder.GetCompositor() = FlutterCompositor{ + .struct_size = sizeof(FlutterCompositor), + .user_data = reinterpret_cast(&compositor_user_data), + .create_backing_store_callback = + [](const FlutterBackingStoreConfig* config, + FlutterBackingStore* backing_store_out, void* user_data) { + EXPECT_TRUE(config->view_id == 0 || + config->view_id == kOtherViewId); + auto producer = + reinterpret_cast(user_data)->producer; + bool result = producer->Create(config, backing_store_out); + // Use user_data to record the view that the store is created for. + backing_store_out->user_data = + reinterpret_cast(config->view_id); + return result; + }, + .collect_backing_store_callback = [](const FlutterBackingStore* renderer, + void* user_data) { return true; }, + .present_layers_callback = nullptr, + .avoid_backing_store_cache = false, + .present_view_callback = + [](const FlutterPresentViewInfo* info) { + if (info->layers_count != 1) { + return true; + } + // Verify that the given layer's backing store has the same view ID + // as the target view. + int64_t store_view_id = reinterpret_cast( + info->layers[0]->backing_store->user_data); + EXPECT_EQ(store_view_id, info->view_id); + auto compositor_user_data = + reinterpret_cast(info->user_data); + switch (info->view_id) { + case 0: + compositor_user_data->latch_implicit.Signal(); + break; + case kOtherViewId: + compositor_user_data->latch_other.Signal(); + break; + default: + FML_UNREACHABLE(); + } + return true; + }, + }; + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // Give the implicit view a non-zero size so that it renders something. + FlutterWindowMetricsEvent metrics0 = { + .struct_size = sizeof(FlutterWindowMetricsEvent), + .width = 800, + .height = 600, + .pixel_ratio = 1.0, + .view_id = 0, + }; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &metrics0), + kSuccess); + + // Add the other view. + FlutterWindowMetricsEvent metrics123 = { + .struct_size = sizeof(FlutterWindowMetricsEvent), + .width = 800, + .height = 600, + .pixel_ratio = 1.0, + .view_id = kOtherViewId, + }; + + FlutterAddViewInfo add_view_info = {}; + add_view_info.struct_size = sizeof(FlutterAddViewInfo); + add_view_info.view_id = 123; + add_view_info.view_metrics = &metrics123; + add_view_info.add_view_callback = [](const FlutterAddViewResult* result) { + ASSERT_TRUE(result->added); + }; + + ASSERT_EQ(FlutterEngineAddView(engine.get(), &add_view_info), kSuccess); + + compositor_user_data.latch_implicit.Wait(); + compositor_user_data.latch_other.Wait(); +} + TEST_F(EmbedderTest, CanUpdateLocales) { auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); EmbedderConfigBuilder builder(context); From 80f7e6ad39da48c04901309fed2542710ac49c19 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 3 Apr 2024 16:09:51 -0700 Subject: [PATCH 6/9] Better test --- .../embedder/tests/embedder_unittests.cc | 122 +++++++++++++----- 1 file changed, 92 insertions(+), 30 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 768abfff2264d..d398b05515f52 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1814,10 +1814,23 @@ TEST_F(EmbedderTest, CanRenderMultipleViews) { latch123.Wait(); } -TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { - // This test involves two views: the implicit view, and the OtherView as - // defined here. - constexpr FlutterViewId kOtherViewId = 123; +//------------------------------------------------------------------------------ +/// Test that the backing store is created with the correct view ID, is used +/// for the correct view, and is cached according to their views. +/// +/// The test involves two frames: +/// 1. The first frame renders the implicit view and the second view. +/// 2. The second frame renders the implicit view and the third view. +/// +/// The test verifies that: +/// - Each backing store is created with a valid view ID. +/// - Each backing store is presented for the view that it was created for. +/// - Both frames render the expected sets of views. +/// - By the end of frame 1, only 2 backing stores were created. +/// - By the end of frame 2, only 3 backing stores were created. +TEST_F(EmbedderTest, BackingStoresCorrespondToTheirViews) { + constexpr FlutterViewId kSecondViewId = 123; + constexpr FlutterViewId kThirdViewId = 456; auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); EmbedderConfigBuilder builder(context); @@ -1829,15 +1842,23 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { context.GetCompositor().GetGrContext(), EmbedderTestBackingStoreProducer::RenderTargetType::kSoftwareBuffer); + // The variables needed by the callbacks of the compositor. struct CompositorUserData { - // The two latches wait for the signal for the two views. The first waits - // for the implicit view. The second waits for the OtherView. - fml::AutoResetWaitableEvent latch_implicit; - fml::AutoResetWaitableEvent latch_other; EmbedderTestBackingStoreProducer* producer; - } compositor_user_data; - - compositor_user_data.producer = &producer; + // Each latch is signaled when its corresponding view is presented. + fml::AutoResetWaitableEvent latch_implicit; + fml::AutoResetWaitableEvent latch_second; + fml::AutoResetWaitableEvent latch_third; + // Whether the respective view should be rendered in the frame. + bool second_expected; + bool third_expected; + // The total number of backing stores created to verify caching. + int backing_stores_created; + }; + CompositorUserData compositor_user_data{ + .producer = &producer, + .backing_stores_created = 0, + }; builder.GetCompositor() = FlutterCompositor{ .struct_size = sizeof(FlutterCompositor), @@ -1845,12 +1866,17 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { .create_backing_store_callback = [](const FlutterBackingStoreConfig* config, FlutterBackingStore* backing_store_out, void* user_data) { + // Verify that the backing store comes with the correct view ID. EXPECT_TRUE(config->view_id == 0 || - config->view_id == kOtherViewId); - auto producer = - reinterpret_cast(user_data)->producer; - bool result = producer->Create(config, backing_store_out); - // Use user_data to record the view that the store is created for. + config->view_id == kSecondViewId || + config->view_id == kThirdViewId); + auto compositor_user_data = + reinterpret_cast(user_data); + compositor_user_data->backing_stores_created += 1; + bool result = compositor_user_data->producer->Create( + config, backing_store_out); + // The created backing store has a user_data that records the view + // that the store is created for. backing_store_out->user_data = reinterpret_cast(config->view_id); return result; @@ -1861,9 +1887,7 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { .avoid_backing_store_cache = false, .present_view_callback = [](const FlutterPresentViewInfo* info) { - if (info->layers_count != 1) { - return true; - } + EXPECT_EQ(info->layers_count, 1u); // Verify that the given layer's backing store has the same view ID // as the target view. int64_t store_view_id = reinterpret_cast( @@ -1871,12 +1895,18 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { EXPECT_EQ(store_view_id, info->view_id); auto compositor_user_data = reinterpret_cast(info->user_data); + // Verify that the respective views are rendered. switch (info->view_id) { case 0: compositor_user_data->latch_implicit.Signal(); break; - case kOtherViewId: - compositor_user_data->latch_other.Signal(); + case kSecondViewId: + EXPECT_TRUE(compositor_user_data->second_expected); + compositor_user_data->latch_second.Signal(); + break; + case kThirdViewId: + EXPECT_TRUE(compositor_user_data->third_expected); + compositor_user_data->latch_third.Signal(); break; default: FML_UNREACHABLE(); @@ -1885,33 +1915,39 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { }, }; + compositor_user_data.second_expected = true; + compositor_user_data.third_expected = false; + + /*=== First frame ===*/ + auto engine = builder.LaunchEngine(); ASSERT_TRUE(engine.is_valid()); // Give the implicit view a non-zero size so that it renders something. - FlutterWindowMetricsEvent metrics0 = { + FlutterWindowMetricsEvent metrics_implicit = { .struct_size = sizeof(FlutterWindowMetricsEvent), .width = 800, .height = 600, .pixel_ratio = 1.0, .view_id = 0, }; - ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &metrics0), - kSuccess); + ASSERT_EQ( + FlutterEngineSendWindowMetricsEvent(engine.get(), &metrics_implicit), + kSuccess); - // Add the other view. - FlutterWindowMetricsEvent metrics123 = { + // Add the second view. + FlutterWindowMetricsEvent metrics_add = { .struct_size = sizeof(FlutterWindowMetricsEvent), .width = 800, .height = 600, .pixel_ratio = 1.0, - .view_id = kOtherViewId, + .view_id = kSecondViewId, }; FlutterAddViewInfo add_view_info = {}; add_view_info.struct_size = sizeof(FlutterAddViewInfo); - add_view_info.view_id = 123; - add_view_info.view_metrics = &metrics123; + add_view_info.view_id = kSecondViewId; + add_view_info.view_metrics = &metrics_add; add_view_info.add_view_callback = [](const FlutterAddViewResult* result) { ASSERT_TRUE(result->added); }; @@ -1919,7 +1955,33 @@ TEST_F(EmbedderTest, BackingStoresAreCreatedWithViewId) { ASSERT_EQ(FlutterEngineAddView(engine.get(), &add_view_info), kSuccess); compositor_user_data.latch_implicit.Wait(); - compositor_user_data.latch_other.Wait(); + compositor_user_data.latch_second.Wait(); + + /*=== Second frame ===*/ + + compositor_user_data.second_expected = false; + compositor_user_data.third_expected = true; + EXPECT_EQ(compositor_user_data.backing_stores_created, 2); + + // Remove the second view + FlutterRemoveViewInfo remove_view_info = {}; + remove_view_info.struct_size = sizeof(FlutterRemoveViewInfo); + remove_view_info.view_id = kSecondViewId; + remove_view_info.remove_view_callback = + [](const FlutterRemoveViewResult* result) { + ASSERT_TRUE(result->removed); + }; + ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &remove_view_info), kSuccess); + + // Add the third view. + add_view_info.view_id = kThirdViewId; + metrics_add.view_id = kThirdViewId; + ASSERT_EQ(FlutterEngineAddView(engine.get(), &add_view_info), kSuccess); + // Adding the view should have scheduled a frame. + + compositor_user_data.latch_implicit.Wait(); + compositor_user_data.latch_third.Wait(); + EXPECT_EQ(compositor_user_data.backing_stores_created, 3); } TEST_F(EmbedderTest, CanUpdateLocales) { From 29095d3f3a844c9bab4612dc5acdc2e713e2d9c6 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 3 Apr 2024 16:14:04 -0700 Subject: [PATCH 7/9] More comment --- shell/platform/embedder/tests/embedder_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index d398b05515f52..a0701574ab0cf 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1827,7 +1827,8 @@ TEST_F(EmbedderTest, CanRenderMultipleViews) { /// - Each backing store is presented for the view that it was created for. /// - Both frames render the expected sets of views. /// - By the end of frame 1, only 2 backing stores were created. -/// - By the end of frame 2, only 3 backing stores were created. +/// - By the end of frame 2, only 3 backing stores were created. This ensures +/// that the backing store for the 2nd view is not reused for the 3rd view. TEST_F(EmbedderTest, BackingStoresCorrespondToTheirViews) { constexpr FlutterViewId kSecondViewId = 123; constexpr FlutterViewId kThirdViewId = 456; From 2c1bf39ce85a733bdb0cbf2f747e67fe997f8eca Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 3 Apr 2024 16:56:44 -0700 Subject: [PATCH 8/9] Comment --- shell/platform/embedder/embedder_external_view_embedder.cc | 3 ++- shell/platform/embedder/embedder_external_view_embedder.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index 5c2202bf23cef..75631cb3513d4 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -423,7 +423,8 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { - // Get the view's cache, or create one if the view doesn't have a cache yet. + // The unordered_map render_target_cache creates a new entry if the view ID is + // unrecognized. EmbedderRenderTargetCache& render_target_cache = render_target_caches_[flutter_view_id]; SkRect _rect = SkRect::MakeIWH(pending_frame_size_.width(), diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index 5b8ebc22481a2..e11e7c2a17f8a 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -121,6 +121,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { SkMatrix pending_surface_transformation_; EmbedderExternalView::PendingViews pending_views_; std::vector composition_order_; + // The render target cache for each view. Each key is a view ID. std::unordered_map render_target_caches_; void Reset(); From c9340b6973efd0141d508c508e62da2f2893b78e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 4 Apr 2024 13:09:58 -0700 Subject: [PATCH 9/9] better comment --- shell/platform/embedder/embedder_external_view_embedder.h | 2 +- shell/platform/embedder/embedder_render_target_cache.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index e11e7c2a17f8a..63f383f8bb4fd 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -121,7 +121,7 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { SkMatrix pending_surface_transformation_; EmbedderExternalView::PendingViews pending_views_; std::vector composition_order_; - // The render target cache for each view. Each key is a view ID. + // The render target caches for views. Each key is a view ID. std::unordered_map render_target_caches_; void Reset(); diff --git a/shell/platform/embedder/embedder_render_target_cache.h b/shell/platform/embedder/embedder_render_target_cache.h index fb3251a5b9fdf..04dd9de727871 100644 --- a/shell/platform/embedder/embedder_render_target_cache.h +++ b/shell/platform/embedder/embedder_render_target_cache.h @@ -19,6 +19,10 @@ namespace flutter { /// @brief A cache used to reference render targets that are owned by the /// embedder but needed by th engine to render a frame. /// +/// A map of class is managed by EmbedderExternalViewEmbedder. Each +/// instance of this class manages the cached render targets for a +/// view. +/// class EmbedderRenderTargetCache { public: EmbedderRenderTargetCache();