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 5ccd649abab69..a45a8ae897ed1 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -265,15 +265,17 @@ void AndroidExternalViewEmbedder::BeginFrame( // The surface size changed. Therefore, destroy existing surfaces as // the existing surfaces in the pool can't be recycled. - if (frame_size_ != frame_size) { + if (frame_size_ != frame_size && raster_thread_merger->IsOnPlatformThread()) { surface_pool_->DestroyLayers(jni_facade_); } - frame_size_ = frame_size; - device_pixel_ratio_ = device_pixel_ratio; + surface_pool_->SetFrameSize(frame_size); // JNI method must be called on the platform thread. if (raster_thread_merger->IsOnPlatformThread()) { jni_facade_->FlutterViewBeginFrame(); } + + frame_size_ = frame_size; + device_pixel_ratio_ = device_pixel_ratio; } // |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 5e8e5b6bcb01a..f69645f33dcf2 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 @@ -557,6 +557,91 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { raster_thread_merger); } +TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { + auto jni_mock = std::make_shared(); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + + auto window = fml::MakeRefCounted(nullptr); + auto gr_context = GrDirectContext::MakeMock(nullptr); + auto frame_size = SkISize::Make(1000, 1000); + auto surface_factory = + [gr_context, window, frame_size]( + std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto surface_frame_1 = std::make_unique( + SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { + return true; + }); + + auto surface_mock = std::make_unique(); + EXPECT_CALL(*surface_mock, AcquireFrame(frame_size)) + .WillOnce(Return(ByMove(std::move(surface_frame_1)))); + + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())) + .WillOnce(Return(ByMove(std::move(surface_mock)))); + + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + + return android_surface_mock; + }; + + auto embedder = std::make_unique( + android_context, jni_mock, surface_factory); + + // ------------------ First frame ------------------ // + { + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); + embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); + + // Add an Android view. + MutatorsStack stack1; + // TODO(egarciad): Investigate why Flow applies the device pixel ratio to + // the offsetPixels, but not the sizePoints. + auto view_params_1 = std::make_unique( + SkMatrix(), SkSize::Make(200, 200), stack1); + + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1)); + + // This simulates Flutter UI that intersects with the Android view. + embedder->CompositeEmbeddedView(0)->drawRect( + SkRect::MakeXYWH(50, 50, 200, 200), SkPaint()); + + // Create a new overlay surface. + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + // The JNI call to display the Android view. + EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(0, 0, 0, 200, 200, + 300, 300, stack1)); + EXPECT_CALL(*jni_mock, + FlutterViewDisplayOverlaySurface(0, 50, 50, 200, 200)); + + auto surface_frame = + std::make_unique(SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, + SkCanvas* canvas) { return true; }); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); + embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); + } + + // Changing the frame size from the raster thread does not make JNI calls. + + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); + + embedder->BeginFrame(SkISize::Make(30, 40), nullptr, 1.0, + GetThreadMergerFromRasterThread()); +} + TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { auto jni_mock = std::make_shared(); diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index bee34109b95b1..eccd2d671da5c 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -24,6 +24,11 @@ std::shared_ptr SurfacePool::GetLayer( std::shared_ptr android_context, std::shared_ptr jni_facade, const AndroidSurface::Factory& surface_factory) { + // Destroy current layers in the pool if the frame size has changed. + if (requested_frame_size_ != current_frame_size_) { + DestroyLayers(jni_facade); + } + intptr_t gr_context_key = reinterpret_cast(gr_context); // Allocate a new surface if there isn't one available. if (available_layer_index_ >= layers_.size()) { @@ -63,6 +68,7 @@ std::shared_ptr SurfacePool::GetLayer( layer->surface = std::move(surface); } available_layer_index_++; + current_frame_size_ = requested_frame_size_; return layer; } @@ -87,4 +93,8 @@ std::vector> SurfacePool::GetUnusedLayers() { return results; } +void SurfacePool::SetFrameSize(SkISize frame_size) { + requested_frame_size_ = frame_size; +} + } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index 190119c75cf25..44236747f9331 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -67,6 +67,11 @@ class SurfacePool { // Destroys all the layers in the pool. void DestroyLayers(std::shared_ptr jni_facade); + // Sets the frame size used by the layers in the pool. + // If the current layers in the pool have a different frame size, + // then they are deallocated as soon as |GetLayer| is called. + void SetFrameSize(SkISize frame_size); + private: // The index of the entry in the layers_ vector that determines the beginning // of the unused layers. For example, consider the following vector: @@ -81,7 +86,15 @@ class SurfacePool { // This indicates that entries starting from 1 can be reused meanwhile the // entry at position 0 cannot be reused. size_t available_layer_index_ = 0; + + // The layers in the pool. std::vector> layers_; + + // The frame size of the layers in the pool. + SkISize current_frame_size_; + + // The frame size to be used by future layers. + SkISize requested_frame_size_; }; } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc index 18ebb9a285fcf..6940c8c75e048 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -202,5 +202,46 @@ TEST(SurfacePool, DestroyLayers) { ASSERT_TRUE(pool->GetUnusedLayers().empty()); } +TEST(SurfacePool, DestroyLayers__frameSizeChanged) { + auto pool = std::make_unique(); + auto jni_mock = std::make_shared(); + + auto gr_context = GrDirectContext::MakeMock(nullptr); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + + auto window = fml::MakeRefCounted(nullptr); + + auto surface_factory = + [gr_context, window](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + return android_surface_mock; + }; + pool->SetFrameSize(SkISize::Make(10, 10)); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .Times(1) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + + pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); + + pool->SetFrameSize(SkISize::Make(20, 20)); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .Times(1) + .WillOnce(Return( + ByMove(std::make_unique( + 1, window)))); + pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); + + ASSERT_TRUE(pool->GetUnusedLayers().empty()); +} + } // namespace testing } // namespace flutter