From 24ad601192a379f0236b5a091e97699c0472364b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 14 Jul 2024 11:26:07 -0700 Subject: [PATCH 01/21] [Android] support platform view HC without merged raster thread. --- .../swapchain/khr/khr_swapchain_impl_vk.cc | 2 +- .../external_view_embedder.cc | 176 +++++++++--------- .../external_view_embedder.h | 4 +- .../embedding/android/FlutterImageView.java | 3 +- .../embedding/android/FlutterView.java | 4 +- .../flutter/embedding/engine/FlutterJNI.java | 6 +- .../platform/PlatformViewsController.java | 7 +- 7 files changed, 99 insertions(+), 103 deletions(-) diff --git a/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc index 8be88832bb470..8794bf45ab06d 100644 --- a/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc @@ -471,7 +471,7 @@ bool KHRSwapchainImplVK::Present( break; default: VALIDATION_LOG << "Could not present queue: " << vk::to_string(result); - break; + return false; } return true; 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 f6ce9eb161273..171582de1f418 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -3,9 +3,12 @@ // found in the LICENSE file. #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" +#include +#include "flow/surface_frame.h" #include "flutter/common/constants.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/trace_event.h" +#include "fml/make_copyable.h" namespace flutter { @@ -78,7 +81,6 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( return; } - std::unordered_map overlay_layers; DlCanvas* background_canvas = frame->Canvas(); auto current_frame_view_count = composition_order_.size(); @@ -86,6 +88,12 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( // below. DlAutoCanvasRestore save(background_canvas, /*do_save=*/true); + struct OverlayDisplayInfo { + std::shared_ptr layer; + SkRect rect; + }; + std::vector overlay_layers; + for (size_t i = 0; i < current_frame_view_count; i++) { int64_t view_id = composition_order_[i]; EmbedderViewSlice* slice = slices_.at(view_id).get(); @@ -133,11 +141,39 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( // // For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4, 5}. full_joined_rect.set(full_joined_rect.roundOut()); - overlay_layers.insert({view_id, full_joined_rect}); + // Clip the background canvas, so it doesn't contain any of the pixels // drawn on the overlay layer. background_canvas->ClipRect(full_joined_rect, DlCanvas::ClipOp::kDifference); + + auto overlay_layer = CreateSurfaceIfNeeded(context, // + view_id, // + slices_.at(view_id).get(), // + full_joined_rect // + ); + if (!overlay_layer) { + continue; + } + + std::unique_ptr frame = + overlay_layer->surface->AcquireFrame(frame_size_); + if (!frame) { + continue; + } + + DlCanvas* overlay_canvas = frame->Canvas(); + + // Offset the picture since its absolute position on the scene is + // determined by the position of the overlay view. + overlay_canvas->Clear(DlColor::kTransparent()); + overlay_canvas->Translate(-full_joined_rect.x(), -full_joined_rect.y()); + slice->render_into(overlay_canvas); + + frame->set_submit_info({.frame_boundary = false}); + frame->Submit(); + overlay_layers.push_back( + {.layer = overlay_layer, .rect = full_joined_rect}); } slice->render_into(background_canvas); } @@ -155,91 +191,64 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( frame->Submit(); } - for (int64_t view_id : composition_order_) { - SkRect view_rect = GetViewRect(view_id); - const EmbeddedViewParams& params = view_params_.at(view_id); - // Display the platform view. If it's already displayed, then it's - // just positioned and sized. - jni_facade_->FlutterViewOnDisplayPlatformView( - view_id, // - view_rect.x(), // - view_rect.y(), // - view_rect.width(), // - view_rect.height(), // - params.sizePoints().width() * device_pixel_ratio_, - params.sizePoints().height() * device_pixel_ratio_, - params.mutatorsStack() // - ); - std::unordered_map::const_iterator overlay = - overlay_layers.find(view_id); - if (overlay == overlay_layers.end()) { - continue; - } - std::unique_ptr frame = - CreateSurfaceIfNeeded(context, // - view_id, // - slices_.at(view_id).get(), // - overlay->second // - ); - if (should_submit_current_frame) { - frame->Submit(); - } - } + surface_pool_->RecycleLayers(); + task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( + [&, composition_order = composition_order_, view_params = view_params_, + overlay_layers = std::move(overlay_layers)]() { + jni_facade_->FlutterViewBeginFrame(); + + size_t i = 0; + for (int64_t view_id : composition_order) { + const EmbeddedViewParams& params = view_params.at(view_id); + auto view_rect = + SkRect::MakeXYWH(params.finalBoundingRect().x(), // + params.finalBoundingRect().y(), // + params.finalBoundingRect().width(), // + params.finalBoundingRect().height() // + ); + + // Display the platform view. If it's already displayed, then it's + // just positioned and sized. + jni_facade_->FlutterViewOnDisplayPlatformView( + view_id, // + view_rect.x(), // + view_rect.y(), // + view_rect.width(), // + view_rect.height(), // + params.sizePoints().width() * device_pixel_ratio_, + params.sizePoints().height() * device_pixel_ratio_, + params.mutatorsStack() // + ); + + if (i < overlay_layers.size()) { + auto& data = overlay_layers[i]; + jni_facade_->FlutterViewDisplayOverlaySurface(data.layer->id, // + data.rect.x(), // + data.rect.y(), // + data.rect.width(), // + data.rect.height() // + ); + i++; + } + } + + jni_facade_->FlutterViewEndFrame(); + })); } // |ExternalViewEmbedder| -std::unique_ptr +std::shared_ptr AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrDirectContext* context, int64_t view_id, EmbedderViewSlice* slice, const SkRect& rect) { - std::shared_ptr layer = surface_pool_->GetLayer( - context, android_context_, jni_facade_, surface_factory_); - - std::unique_ptr frame = - layer->surface->AcquireFrame(frame_size_); - // Display the overlay surface. If it's already displayed, then it's - // just positioned and sized. - jni_facade_->FlutterViewDisplayOverlaySurface(layer->id, // - rect.x(), // - rect.y(), // - rect.width(), // - rect.height() // - ); - DlCanvas* overlay_canvas = frame->Canvas(); - overlay_canvas->Clear(DlColor::kTransparent()); - // Offset the picture since its absolute position on the scene is determined - // by the position of the overlay view. - overlay_canvas->Translate(-rect.x(), -rect.y()); - slice->render_into(overlay_canvas); - return frame; + return surface_pool_->GetLayer(context, android_context_, jni_facade_, + surface_factory_); } // |ExternalViewEmbedder| PostPrerollResult AndroidExternalViewEmbedder::PostPrerollAction( const fml::RefPtr& raster_thread_merger) { - if (!FrameHasPlatformLayers()) { - return PostPrerollResult::kSuccess; - } - if (!raster_thread_merger->IsMerged()) { - // The raster thread merger may be disabled if the rasterizer is being - // created or teared down. - // - // In such cases, the current frame is dropped, and a new frame is attempted - // with the same layer tree. - // - // Eventually, the frame is submitted once this method returns `kSuccess`. - // At that point, the raster tasks are handled on the platform thread. - CancelFrame(); - raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); - return PostPrerollResult::kSkipAndRetryFrame; - } - raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); - // Surface switch requires to resubmit the frame. - // TODO(egarciad): https://github.com/flutter/flutter/issues/65652 - if (previous_frame_view_count_ == 0) { - return PostPrerollResult::kResubmitFrame; - } return PostPrerollResult::kSuccess; } @@ -263,12 +272,7 @@ void AndroidExternalViewEmbedder::Reset() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::BeginFrame( GrDirectContext* context, - const fml::RefPtr& raster_thread_merger) { - // JNI method must be called on the platform thread. - if (raster_thread_merger->IsOnPlatformThread()) { - jni_facade_->FlutterViewBeginFrame(); - } -} + const fml::RefPtr& raster_thread_merger) {} // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::PrepareFlutterView( @@ -295,17 +299,11 @@ void AndroidExternalViewEmbedder::CancelFrame() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::EndFrame( bool should_resubmit_frame, - const fml::RefPtr& raster_thread_merger) { - surface_pool_->RecycleLayers(); - // JNI method must be called on the platform thread. - if (raster_thread_merger->IsOnPlatformThread()) { - jni_facade_->FlutterViewEndFrame(); - } -} + const fml::RefPtr& raster_thread_merger) {} // |ExternalViewEmbedder| bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { - return true; + return false; } // |ExternalViewEmbedder| 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 ab00870276ffc..e45c996e956e7 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -139,9 +139,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // Whether the layer tree in the current frame has platform layers. bool FrameHasPlatformLayers(); - // Creates a Surface when needed or recycles an existing one. - // Finally, draws the picture on the frame's canvas. - std::unique_ptr CreateSurfaceIfNeeded(GrDirectContext* context, + std::shared_ptr CreateSurfaceIfNeeded(GrDirectContext* context, int64_t view_id, EmbedderViewSlice* slice, const SkRect& rect); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java index df9d6b394dd7d..3ae5551536a55 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java @@ -197,7 +197,7 @@ public boolean acquireLatestImage() { // or some special Android devices so the calls to `invalidate()` queued up // until the device produces a new frame. // 3. While the engine will also stop producing frames, there is a race condition. - final Image newImage = imageReader.acquireLatestImage(); + final Image newImage = imageReader.acquireNextImage(); if (newImage != null) { // Only close current image after acquiring valid new image closeCurrentImage(); @@ -237,6 +237,7 @@ public void closeImageReader() { @Override protected void onDraw(Canvas canvas) { super.onDraw(canvas); + if (currentImage != null) { updateCurrentBitmap(); } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 7272b5125ba05..2b55cd872b606 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1346,7 +1346,9 @@ public void attachOverlaySurfaceToRender(@NonNull FlutterImageView view) { public boolean acquireLatestImageViewFrame() { if (flutterImageView != null) { - return flutterImageView.acquireLatestImage(); + boolean result = flutterImageView.acquireLatestImage(); + flutterImageView.invalidate(); + return result; } return false; } diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index e24953bd8ee1b..50605e4ad2049 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1221,7 +1221,7 @@ public void onDisplayOverlaySurface(int id, int x, int y, int width, int height) @SuppressWarnings("unused") @UiThread public void onBeginFrame() { - ensureRunningOnMainThread(); + ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to begin the frame"); @@ -1243,7 +1243,6 @@ public void onEndFrame() { @SuppressWarnings("unused") @UiThread public FlutterOverlaySurface createOverlaySurface() { - ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to position an overlay surface"); @@ -1356,7 +1355,6 @@ public void requestDartDeferredLibrary(int loadingUnitId) { if (deferredComponentManager != null) { deferredComponentManager.installDeferredComponent(loadingUnitId, null); } else { - // TODO(garyq): Add link to setup/instructions guide wiki. Log.e( TAG, "No DeferredComponentManager found. Android setup must be completed before using split AOT deferred components."); @@ -1459,7 +1457,6 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } - // TODO(mattcarroll): determine if this is nonull or nullable @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); @@ -1467,7 +1464,6 @@ public Bitmap getBitmap() { return nativeGetBitmap(nativeShellHolderId); } - // TODO(mattcarroll): determine if this is nonull or nullable private native Bitmap nativeGetBitmap(long nativeShellHolderId); /** diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 9c78a3f030135..efd2fc2075118 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -987,16 +987,16 @@ private static PlatformViewRenderTarget makePlatformViewRenderTarget( TextureRegistry textureRegistry) { if (enableSurfaceProducerRenderTarget && Build.VERSION.SDK_INT >= API_LEVELS.API_29) { final TextureRegistry.SurfaceProducer textureEntry = textureRegistry.createSurfaceProducer(); - Log.i(TAG, "PlatformView is using SurfaceProducer backend"); + Log.v(TAG, "PlatformView is using SurfaceProducer backend"); return new SurfaceProducerPlatformViewRenderTarget(textureEntry); } if (enableImageRenderTarget && Build.VERSION.SDK_INT >= API_LEVELS.API_29) { final TextureRegistry.ImageTextureEntry textureEntry = textureRegistry.createImageTexture(); - Log.i(TAG, "PlatformView is using ImageReader backend"); + Log.v(TAG, "PlatformView is using ImageReader backend"); return new ImageReaderPlatformViewRenderTarget(textureEntry); } final TextureRegistry.SurfaceTextureEntry textureEntry = textureRegistry.createSurfaceTexture(); - Log.i(TAG, "PlatformView is using SurfaceTexture backend"); + Log.v(TAG, "PlatformView is using SurfaceTexture backend"); return new SurfaceTexturePlatformViewRenderTarget(textureEntry); } @@ -1282,6 +1282,7 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { if (currentFrameUsedOverlayLayerIds.contains(overlayId)) { flutterView.attachOverlaySurfaceToRender(overlayView); final boolean didAcquireOverlaySurfaceImage = overlayView.acquireLatestImage(); + overlayView.invalidate(); isFrameRenderedUsingImageReaders &= didAcquireOverlaySurfaceImage; } else { // If the background surface isn't rendered by the image view, then the From 99e9e26fa1a38af9a3fcad4699ff9fafea798901 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 14 Jul 2024 11:26:30 -0700 Subject: [PATCH 02/21] ++ --- .../android/io/flutter/embedding/engine/FlutterJNI.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 50605e4ad2049..ab1a7703651e1 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1221,7 +1221,7 @@ public void onDisplayOverlaySurface(int id, int x, int y, int width, int height) @SuppressWarnings("unused") @UiThread public void onBeginFrame() { - ensureRunningOnMainThread(); + ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to begin the frame"); From 796a80493d1ca8b864f93bed46a6d7f86a4d7bac Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jul 2024 11:26:10 -0700 Subject: [PATCH 03/21] Add adjustments. --- shell/common/animator.cc | 14 +------------- .../external_view_embedder.cc | 1 + .../plugin/platform/PlatformViewsController.java | 2 +- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index e8329c11906e2..032a0048cb758 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -28,21 +28,9 @@ Animator::Animator(Delegate& delegate, : delegate_(delegate), task_runners_(task_runners), waiter_(std::move(waiter)), -#if SHELL_ENABLE_METAL layer_tree_pipeline_(std::make_shared(2)), -#else // SHELL_ENABLE_METAL - // TODO(dnfield): We should remove this logic and set the pipeline depth - // back to 2 in this case. See - // https://github.com/flutter/engine/pull/9132 for discussion. - layer_tree_pipeline_(std::make_shared( - task_runners.GetPlatformTaskRunner() == - task_runners.GetRasterTaskRunner() - ? 1 - : 2)), -#endif // SHELL_ENABLE_METAL pending_frame_semaphore_(1), - weak_factory_(this) { -} + weak_factory_(this) {} Animator::~Animator() = default; 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 171582de1f418..3ababec4a9ca5 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -195,6 +195,7 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( [&, composition_order = composition_order_, view_params = view_params_, overlay_layers = std::move(overlay_layers)]() { + TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); jni_facade_->FlutterViewBeginFrame(); size_t i = 0; diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index efd2fc2075118..b7086ff126a1d 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -599,7 +599,7 @@ public long configureForTextureLayerComposition( @NonNull PlatformView platformView, @NonNull PlatformViewsChannel.PlatformViewCreationRequest request) { // This mode attaches the view to the Android view hierarchy and record its drawing - // operations, so they can be forwarded to a GL texture that is composed by the + // operations, so they can be forwarded to a GL texture that is composited by the // Flutter engine. // API level 23 is required to use Surface#lockHardwareCanvas(). From 9686e34d5d6bac109c10ba6bd0503d9fb45e5bd0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jul 2024 12:57:15 -0700 Subject: [PATCH 04/21] cleanups. --- .../external_view_embedder.cc | 19 ++++++++++++------- .../embedding/android/FlutterImageView.java | 1 - .../embedding/android/FlutterView.java | 4 +--- .../platform/PlatformViewsController.java | 1 - 4 files changed, 13 insertions(+), 12 deletions(-) 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 3ababec4a9ca5..3e418863b61ce 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -174,6 +174,8 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( frame->Submit(); overlay_layers.push_back( {.layer = overlay_layer, .rect = full_joined_rect}); + } else { + overlay_layers.push_back({}); } slice->render_into(background_canvas); } @@ -195,7 +197,8 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( [&, composition_order = composition_order_, view_params = view_params_, overlay_layers = std::move(overlay_layers)]() { - TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); + TRACE_EVENT0("flutter", + "AndroidExternalViewEmbedder::RenderNativeViews"); jni_facade_->FlutterViewBeginFrame(); size_t i = 0; @@ -223,12 +226,14 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( if (i < overlay_layers.size()) { auto& data = overlay_layers[i]; - jni_facade_->FlutterViewDisplayOverlaySurface(data.layer->id, // - data.rect.x(), // - data.rect.y(), // - data.rect.width(), // - data.rect.height() // - ); + if (data.layer) { + jni_facade_->FlutterViewDisplayOverlaySurface(data.layer->id, // + data.rect.x(), // + data.rect.y(), // + data.rect.width(), // + data.rect.height() // + ); + } i++; } } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java index 3ae5551536a55..d6d778206e2c3 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java @@ -237,7 +237,6 @@ public void closeImageReader() { @Override protected void onDraw(Canvas canvas) { super.onDraw(canvas); - if (currentImage != null) { updateCurrentBitmap(); } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 2b55cd872b606..7272b5125ba05 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1346,9 +1346,7 @@ public void attachOverlaySurfaceToRender(@NonNull FlutterImageView view) { public boolean acquireLatestImageViewFrame() { if (flutterImageView != null) { - boolean result = flutterImageView.acquireLatestImage(); - flutterImageView.invalidate(); - return result; + return flutterImageView.acquireLatestImage(); } return false; } diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index b7086ff126a1d..9ade706e7263d 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1282,7 +1282,6 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { if (currentFrameUsedOverlayLayerIds.contains(overlayId)) { flutterView.attachOverlaySurfaceToRender(overlayView); final boolean didAcquireOverlaySurfaceImage = overlayView.acquireLatestImage(); - overlayView.invalidate(); isFrameRenderedUsingImageReaders &= didAcquireOverlaySurfaceImage; } else { // If the background surface isn't rendered by the image view, then the From 1bf35101fd1f16be65ad9abe927fb0aa0dfc0c27 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jul 2024 12:58:04 -0700 Subject: [PATCH 05/21] ++ --- .../external_view_embedder/external_view_embedder.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 3e418863b61ce..230e214b1f192 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -227,11 +227,12 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( if (i < overlay_layers.size()) { auto& data = overlay_layers[i]; if (data.layer) { - jni_facade_->FlutterViewDisplayOverlaySurface(data.layer->id, // - data.rect.x(), // - data.rect.y(), // - data.rect.width(), // - data.rect.height() // + jni_facade_->FlutterViewDisplayOverlaySurface( + data.layer->id, // + data.rect.x(), // + data.rect.y(), // + data.rect.width(), // + data.rect.height() // ); } i++; From 11b8ed3b8f02f8d05d6509ab7d78351a1fa7b2dc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 16 Jul 2024 14:23:57 -0700 Subject: [PATCH 06/21] adjustmens --- .../external_view_embedder/external_view_embedder.cc | 8 +++++++- .../external_view_embedder/external_view_embedder.h | 1 + .../io/flutter/embedding/android/FlutterImageView.java | 4 ++-- .../io/flutter/embedding/android/FlutterView.java | 4 ++-- .../flutter/plugin/platform/PlatformViewsController.java | 9 ++++----- 5 files changed, 16 insertions(+), 10 deletions(-) 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 230e214b1f192..b0b5031625a30 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -3,7 +3,9 @@ // found in the LICENSE file. #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" + #include + #include "flow/surface_frame.h" #include "flutter/common/constants.h" #include "flutter/fml/synchronization/waitable_event.h" @@ -76,10 +78,11 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( // Properly support multi-view in the future. FML_DCHECK(flutter_view_id == kFlutterImplicitViewId); - if (!FrameHasPlatformLayers()) { + if (!FrameHasPlatformLayers() && !had_platform_views_) { frame->Submit(); return; } + had_platform_views_ = FrameHasPlatformLayers(); DlCanvas* background_canvas = frame->Canvas(); auto current_frame_view_count = composition_order_.size(); @@ -256,6 +259,9 @@ AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrDirectContext* context, // |ExternalViewEmbedder| PostPrerollResult AndroidExternalViewEmbedder::PostPrerollAction( const fml::RefPtr& raster_thread_merger) { + if (previous_frame_view_count_ == 0) { + return PostPrerollResult::kResubmitFrame; + } return PostPrerollResult::kSuccess; } 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 e45c996e956e7..1296695592b91 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -115,6 +115,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // The order of composition. Each entry contains a unique id for the platform // view. std::vector composition_order_; + bool had_platform_views_ = false; // The |EmbedderViewSlice| implementation keyed off the platform view id, // which contains any subsequent operations until the next platform view or diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java index d6d778206e2c3..49416b3f1963a 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java @@ -165,7 +165,7 @@ public void detachFromRenderer() { setAlpha(0.0f); // Drop the latest image as it shouldn't render this image if this view is // attached to the renderer again. - acquireLatestImage(); + acquireNextImage(); // Clear drawings. currentBitmap = null; @@ -187,7 +187,7 @@ public void resume() { * Acquires the next image to be drawn to the {@link android.graphics.Canvas}. Returns true if * there's an image available in the queue. */ - public boolean acquireLatestImage() { + public boolean acquireNextImage() { if (!isAttachedToFlutterRenderer) { return false; } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 7272b5125ba05..5c1440d3021ad 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1344,9 +1344,9 @@ public void attachOverlaySurfaceToRender(@NonNull FlutterImageView view) { } } - public boolean acquireLatestImageViewFrame() { + public boolean acquireNextImageViewFrame() { if (flutterImageView != null) { - return flutterImageView.acquireLatestImage(); + return flutterImageView.acquireNextImage(); } return false; } diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 9ade706e7263d..bd7d860d9e64d 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1269,9 +1269,9 @@ public void onEndFrame() { // If one of the surfaces doesn't have an image, the frame may be incomplete and must be // dropped. // For example, a toolbar widget painted by Flutter may not be rendered. - final boolean isFrameRenderedUsingImageReaders = - flutterViewConvertedToImageView && flutterView.acquireLatestImageViewFrame(); - finishFrame(isFrameRenderedUsingImageReaders); + flutterView.acquireNextImageViewFrame(); + + finishFrame(flutterViewConvertedToImageView); } private void finishFrame(boolean isFrameRenderedUsingImageReaders) { @@ -1281,8 +1281,7 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { if (currentFrameUsedOverlayLayerIds.contains(overlayId)) { flutterView.attachOverlaySurfaceToRender(overlayView); - final boolean didAcquireOverlaySurfaceImage = overlayView.acquireLatestImage(); - isFrameRenderedUsingImageReaders &= didAcquireOverlaySurfaceImage; + isFrameRenderedUsingImageReaders |= overlayView.acquireNextImage(); } else { // If the background surface isn't rendered by the image view, then the // overlay surfaces can be detached from the rendered. From 4713f8181e78ea41841f69341db5dc265e9faaf7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 7 Aug 2024 14:52:09 -0700 Subject: [PATCH 07/21] ++ --- .../external_view_embedder.cc | 37 +++++++++---------- .../embedding/android/FlutterImageView.java | 6 +-- .../embedding/android/FlutterView.java | 4 +- .../platform/PlatformViewsController.java | 4 +- 4 files changed, 25 insertions(+), 26 deletions(-) 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 56daf97237358..d748849b4b4c0 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -114,7 +114,7 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( auto overlay_layer = CreateSurfaceIfNeeded(context, // view_id, // slices_.at(view_id).get(), // - rect // + rect // ); if (!overlay_layer) { continue; @@ -142,7 +142,8 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( surface_pool_->RecycleLayers(); task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( [&, composition_order = composition_order_, view_params = view_params_, - overlay_layers = std::move(overlay_layers), layers = std::move(layers)]() { + overlay_layers = std::move(overlay_layers), + layers = std::move(layers)]() { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); jni_facade_->FlutterViewBeginFrame(); @@ -169,24 +170,22 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( params.mutatorsStack() // ); - if (i < overlay_layers.size()) { - auto overlay_rect = overlay_layers.find(i); - if (overlay_rect == overlay_layers.end()) { - continue; - } - SkRect rect = overlay_rect->second; - auto maybe_layer = layers.find(i); - if (maybe_layer == layers.end()) { - continue; - } - jni_facade_->FlutterViewDisplayOverlaySurface( - maybe_layer->second->id, // - rect.x(), // - rect.y(), // - rect.width(), // - rect.height() // - ); + auto overlay_rect = overlay_layers.find(view_id); + if (overlay_rect == overlay_layers.end()) { + continue; } + SkRect rect = overlay_rect->second; + auto maybe_layer = layers.find(view_id); + if (maybe_layer == layers.end()) { + continue; + } + jni_facade_->FlutterViewDisplayOverlaySurface( + maybe_layer->second->id, // + rect.x(), // + rect.y(), // + rect.width(), // + rect.height() // + ); } jni_facade_->FlutterViewEndFrame(); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java index 49416b3f1963a..df9d6b394dd7d 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterImageView.java @@ -165,7 +165,7 @@ public void detachFromRenderer() { setAlpha(0.0f); // Drop the latest image as it shouldn't render this image if this view is // attached to the renderer again. - acquireNextImage(); + acquireLatestImage(); // Clear drawings. currentBitmap = null; @@ -187,7 +187,7 @@ public void resume() { * Acquires the next image to be drawn to the {@link android.graphics.Canvas}. Returns true if * there's an image available in the queue. */ - public boolean acquireNextImage() { + public boolean acquireLatestImage() { if (!isAttachedToFlutterRenderer) { return false; } @@ -197,7 +197,7 @@ public boolean acquireNextImage() { // or some special Android devices so the calls to `invalidate()` queued up // until the device produces a new frame. // 3. While the engine will also stop producing frames, there is a race condition. - final Image newImage = imageReader.acquireNextImage(); + final Image newImage = imageReader.acquireLatestImage(); if (newImage != null) { // Only close current image after acquiring valid new image closeCurrentImage(); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 5c1440d3021ad..7272b5125ba05 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1344,9 +1344,9 @@ public void attachOverlaySurfaceToRender(@NonNull FlutterImageView view) { } } - public boolean acquireNextImageViewFrame() { + public boolean acquireLatestImageViewFrame() { if (flutterImageView != null) { - return flutterImageView.acquireNextImage(); + return flutterImageView.acquireLatestImage(); } return false; } diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index bd7d860d9e64d..a7f85dc2d654f 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -1269,7 +1269,7 @@ public void onEndFrame() { // If one of the surfaces doesn't have an image, the frame may be incomplete and must be // dropped. // For example, a toolbar widget painted by Flutter may not be rendered. - flutterView.acquireNextImageViewFrame(); + flutterView.acquireLatestImageViewFrame(); finishFrame(flutterViewConvertedToImageView); } @@ -1281,7 +1281,7 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { if (currentFrameUsedOverlayLayerIds.contains(overlayId)) { flutterView.attachOverlaySurfaceToRender(overlayView); - isFrameRenderedUsingImageReaders |= overlayView.acquireNextImage(); + isFrameRenderedUsingImageReaders |= overlayView.acquireLatestImage(); } else { // If the background surface isn't rendered by the image view, then the // overlay surfaces can be detached from the rendered. From 8cbfa98063bef985c239a454fa1b784b0bfbf35f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 7 Aug 2024 15:40:52 -0700 Subject: [PATCH 08/21] ++ --- .../external_view_embedder.cc | 50 +++++---- .../external_view_embedder.h | 5 - .../external_view_embedder/surface_pool.cc | 100 +++++++----------- .../external_view_embedder/surface_pool.h | 60 +++++------ .../flutter/embedding/engine/FlutterJNI.java | 1 + 5 files changed, 91 insertions(+), 125 deletions(-) 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 d748849b4b4c0..0233ce1589e2f 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -5,6 +5,7 @@ #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" #include +#include #include #include "flow/surface_frame.h" @@ -13,6 +14,7 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/trace_event.h" #include "fml/make_copyable.h" +#include "fml/synchronization/count_down_latch.h" #include "shell/platform/android/external_view_embedder/surface_pool.h" namespace flutter { @@ -109,13 +111,27 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( frame->Submit(); } + // Overlay layers must be created or destroyed on the platfor thread. + bool destroy_all_layers = + surface_pool_->CheckLayerProperties(context, frame_size_); + if (destroy_all_layers || surface_pool_->size() < overlay_layers.size()) { + auto latch = std::make_shared(1u); + task_runners_.GetPlatformTaskRunner()->PostTask([&]() { + if (destroy_all_layers) { + surface_pool_->DestroyLayers(jni_facade_); + } + for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { + surface_pool_->CreateLayer(context, android_context_, jni_facade_, + surface_factory_); + } + latch->CountDown(); + }); + latch->Wait(); + } + std::unordered_map> layers; for (const auto& [view_id, rect] : overlay_layers) { - auto overlay_layer = CreateSurfaceIfNeeded(context, // - view_id, // - slices_.at(view_id).get(), // - rect // - ); + auto overlay_layer = surface_pool_->GetNextLayer(); if (!overlay_layer) { continue; } @@ -140,7 +156,7 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( } surface_pool_->RecycleLayers(); - task_runners_.GetPlatformTaskRunner()->PostTask(fml::MakeCopyable( + task_runners_.GetPlatformTaskRunner()->PostTask( [&, composition_order = composition_order_, view_params = view_params_, overlay_layers = std::move(overlay_layers), layers = std::move(layers)]() { @@ -189,17 +205,7 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( } jni_facade_->FlutterViewEndFrame(); - })); -} - -// |ExternalViewEmbedder| -std::shared_ptr -AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrDirectContext* context, - int64_t view_id, - EmbedderViewSlice* slice, - const SkRect& rect) { - return surface_pool_->GetLayer(context, android_context_, jni_facade_, - surface_factory_); + }); } // |ExternalViewEmbedder| @@ -238,14 +244,6 @@ void AndroidExternalViewEmbedder::PrepareFlutterView( SkISize frame_size, double device_pixel_ratio) { Reset(); - - // The surface size changed. Therefore, destroy existing surfaces as - // the existing surfaces in the pool can't be recycled. - if (frame_size_ != frame_size) { - DestroySurfaces(); - } - surface_pool_->SetFrameSize(frame_size); - frame_size_ = frame_size; device_pixel_ratio_ = device_pixel_ratio; } @@ -272,7 +270,7 @@ void AndroidExternalViewEmbedder::Teardown() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::DestroySurfaces() { - if (!surface_pool_->HasLayers()) { + if (surface_pool_->size() == 0) { return; } fml::AutoResetWaitableEvent latch; 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 1296695592b91..6fafa7ece605d 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -139,11 +139,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // Whether the layer tree in the current frame has platform layers. bool FrameHasPlatformLayers(); - - std::shared_ptr CreateSurfaceIfNeeded(GrDirectContext* context, - int64_t view_id, - EmbedderViewSlice* slice, - const SkRect& rect); }; } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 6e71e97247c9b..5bef419809aa0 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -21,78 +21,58 @@ SurfacePool::SurfacePool() = default; SurfacePool::~SurfacePool() = default; -std::shared_ptr SurfacePool::GetLayer( +bool SurfacePool::CheckLayerProperties(GrDirectContext* gr_context, + SkISize frame_size) { + intptr_t gr_context_key = reinterpret_cast(gr_context); + bool destroy_all_layers = + gr_context_key != gr_context_key_ || frame_size != current_frame_size_; + current_frame_size_ = frame_size; + gr_context_key_ = gr_context_key; + return destroy_all_layers; +} + +std::shared_ptr SurfacePool::GetNextLayer() { + if (available_layer_index_ >= layers_.size()) { + return nullptr; + } + return layers_[available_layer_index_++]; +} + +void SurfacePool::CreateLayer( GrDirectContext* gr_context, const AndroidContext& android_context, const std::shared_ptr& jni_facade, const std::shared_ptr& surface_factory) { - std::lock_guard lock(mutex_); - // Destroy current layers in the pool if the frame size has changed. - if (requested_frame_size_ != current_frame_size_) { - DestroyLayersLocked(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()) { - std::unique_ptr android_surface = - surface_factory->CreateSurface(); - - FML_CHECK(android_surface && android_surface->IsValid()) - << "Could not create an OpenGL, Vulkan or Software surface to set up " - "rendering."; + std::unique_ptr android_surface = + surface_factory->CreateSurface(); - std::unique_ptr java_metadata = - jni_facade->FlutterViewCreateOverlaySurface(); + FML_CHECK(android_surface && android_surface->IsValid()) + << "Could not create an OpenGL, Vulkan or Software surface to set up " + "rendering."; - FML_CHECK(java_metadata->window); - android_surface->SetNativeWindow(java_metadata->window); + std::unique_ptr java_metadata = + jni_facade->FlutterViewCreateOverlaySurface(); - std::unique_ptr surface = - android_surface->CreateGPUSurface(gr_context); + FML_CHECK(java_metadata->window); + android_surface->SetNativeWindow(java_metadata->window); - std::shared_ptr layer = - std::make_shared(java_metadata->id, // - std::move(android_surface), // - std::move(surface) // - ); - layer->gr_context_key = gr_context_key; - layers_.push_back(layer); - } + std::unique_ptr surface = + android_surface->CreateGPUSurface(gr_context); - std::shared_ptr layer = layers_[available_layer_index_]; - // Since the surfaces are recycled, it's possible that the GrContext is - // different. - if (gr_context_key != layer->gr_context_key) { - layer->gr_context_key = gr_context_key; - // The overlay already exists, but the GrContext was changed so we need to - // recreate the rendering surface with the new GrContext. - std::unique_ptr surface = - layer->android_surface->CreateGPUSurface(gr_context); - layer->surface = std::move(surface); - } - available_layer_index_++; - current_frame_size_ = requested_frame_size_; - return layer; + std::shared_ptr layer = + std::make_shared(java_metadata->id, // + std::move(android_surface), // + std::move(surface) // + ); + layers_.push_back(layer); } void SurfacePool::RecycleLayers() { - std::lock_guard lock(mutex_); available_layer_index_ = 0; } -bool SurfacePool::HasLayers() { - std::lock_guard lock(mutex_); - return !layers_.empty(); -} - void SurfacePool::DestroyLayers( const std::shared_ptr& jni_facade) { - std::lock_guard lock(mutex_); - DestroyLayersLocked(jni_facade); -} - -void SurfacePool::DestroyLayersLocked( - const std::shared_ptr& jni_facade) { if (layers_.empty()) { return; } @@ -101,8 +81,11 @@ void SurfacePool::DestroyLayersLocked( available_layer_index_ = 0; } +size_t SurfacePool::size() const { + return layers_.size(); +} + std::vector> SurfacePool::GetUnusedLayers() { - std::lock_guard lock(mutex_); std::vector> results; for (size_t i = available_layer_index_; i < layers_.size(); i++) { results.push_back(layers_[i]); @@ -110,9 +93,4 @@ std::vector> SurfacePool::GetUnusedLayers() { return results; } -void SurfacePool::SetFrameSize(SkISize frame_size) { - std::lock_guard lock(mutex_); - 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 5f09d6db916d5..468e0283a45c0 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ -#include - #include "flutter/flow/surface.h" #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/surface/android_surface.h" @@ -34,13 +32,6 @@ struct OverlayLayer { // A GPU surface. This may change when the overlay is recycled. std::unique_ptr surface; - - // The `GrContext` that is currently used by the overlay surfaces. - // We track this to know when the GrContext for the Flutter app has changed - // so we can update the overlay with the new context. - // - // This may change when the overlay is recycled. - intptr_t gr_context_key; }; class SurfacePool { @@ -49,32 +40,37 @@ class SurfacePool { ~SurfacePool(); - // Gets a layer from the pool if available, or allocates a new one. - // Finally, it marks the layer as used. That is, it increments - // `available_layer_index_`. - std::shared_ptr GetLayer( + /// @brief Returns whether the cached layers are still valid. + /// + /// If the frame size or layer has changed, then all layers must be + /// destroyed and recreated. + bool CheckLayerProperties(GrDirectContext* gr_context, SkISize frame_size); + + /// @brief Gets a layer from the pool if available. + /// + /// The layer is marked as used until [RecycleLayers] is called. + std::shared_ptr GetNextLayer(); + + /// @brief Create a new overlay layer. + /// + /// This method can only be called on the Platform thread. + void CreateLayer( GrDirectContext* gr_context, const AndroidContext& android_context, const std::shared_ptr& jni_facade, const std::shared_ptr& surface_factory); - // Gets the layers in the pool that aren't currently used. - // This method doesn't mark the layers as unused. + /// @brief Removes unused layers from the pool. Returns the unused layers. std::vector> GetUnusedLayers(); - // Marks the layers in the pool as available for reuse. + /// @brief Marks the layers in the pool as available for reuse. void RecycleLayers(); - // Destroys all the layers in the pool. - void DestroyLayers(const 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); + /// @brief The count of layers currently in the pool. + size_t size() const; - // Returns true if the current pool has layers in use. - bool HasLayers(); + /// @brief Destroys all the layers in the pool. + void DestroyLayers(const std::shared_ptr& jni_facade); private: // The index of the entry in the layers_ vector that determines the beginning @@ -97,14 +93,12 @@ class SurfacePool { // 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_; - - // Used to guard public methods. - std::mutex mutex_; - - void DestroyLayersLocked( - const std::shared_ptr& jni_facade); + // The `GrContext` that is currently used by the overlay surfaces. + // We track this to know when the GrContext for the Flutter app has changed + // so we can update the overlay with the new context. + // + // This may change when the overlay is recycled. + intptr_t gr_context_key_; }; } // namespace flutter diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index ab1a7703651e1..8edb708f65545 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1243,6 +1243,7 @@ public void onEndFrame() { @SuppressWarnings("unused") @UiThread public FlutterOverlaySurface createOverlaySurface() { + ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to position an overlay surface"); From 5eb5f48c1bd436751f004b7da831e78c5ffc27b8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 7 Aug 2024 16:35:13 -0700 Subject: [PATCH 09/21] unittest updates. --- .../external_view_embedder/surface_pool.cc | 5 +- .../surface_pool_unittests.cc | 111 ++++++++++-------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 5bef419809aa0..45a4ca7bd222d 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -24,8 +24,9 @@ SurfacePool::~SurfacePool() = default; bool SurfacePool::CheckLayerProperties(GrDirectContext* gr_context, SkISize frame_size) { intptr_t gr_context_key = reinterpret_cast(gr_context); - bool destroy_all_layers = - gr_context_key != gr_context_key_ || frame_size != current_frame_size_; + bool destroy_all_layers = (gr_context_key != gr_context_key_ || + frame_size != current_frame_size_) && + layers_.size() > 0; current_frame_size_ = frame_size; gr_context_key_ = gr_context_key; return destroy_all_layers; 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 d0c505675fe09..15d03cc85b89d 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -10,6 +10,7 @@ #include "flutter/shell/platform/android/surface/android_surface_mock.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "include/core/SkSize.h" #include "third_party/skia/include/gpu/GrDirectContext.h" namespace flutter { @@ -58,13 +59,13 @@ TEST(SurfacePool, GetLayerAllocateOneLayer) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - auto layer = pool->GetLayer(gr_context.get(), *android_context, jni_mock, - surface_factory); + EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{100, 100})); + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); + auto layer = pool->GetNextLayer(); - ASSERT_TRUE(pool->HasLayers()); - ASSERT_NE(nullptr, layer); - ASSERT_EQ(reinterpret_cast(gr_context.get()), - layer->gr_context_key); + EXPECT_TRUE(pool->size() > 0); + EXPECT_NE(nullptr, layer); } TEST(SurfacePool, GetUnusedLayers) { @@ -89,15 +90,17 @@ TEST(SurfacePool, GetUnusedLayers) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - auto layer = pool->GetLayer(gr_context.get(), *android_context, jni_mock, - surface_factory); - ASSERT_EQ(0UL, pool->GetUnusedLayers().size()); + EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{100, 100})); + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); + EXPECT_EQ(0UL, pool->GetUnusedLayers().size()); + auto layer = pool->GetNextLayer(); pool->RecycleLayers(); - ASSERT_TRUE(pool->HasLayers()); - ASSERT_EQ(1UL, pool->GetUnusedLayers().size()); - ASSERT_EQ(layer, pool->GetUnusedLayers()[0]); + EXPECT_EQ(pool->size(), 1u); + EXPECT_EQ(1UL, pool->GetUnusedLayers().size()); + EXPECT_EQ(layer, pool->GetUnusedLayers()[0]); } TEST(SurfacePool, GetLayerRecycle) { @@ -128,21 +131,24 @@ TEST(SurfacePool, GetLayerRecycle) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - auto layer_1 = pool->GetLayer(gr_context_1.get(), *android_context, jni_mock, - surface_factory); - + EXPECT_FALSE( + pool->CheckLayerProperties(gr_context_1.get(), SkISize{100, 100})); + pool->CreateLayer(gr_context_1.get(), *android_context, jni_mock, + surface_factory); + auto layer_1 = pool->GetNextLayer(); pool->RecycleLayers(); - auto layer_2 = pool->GetLayer(gr_context_2.get(), *android_context, jni_mock, - surface_factory); + EXPECT_TRUE( + pool->CheckLayerProperties(gr_context_2.get(), SkISize{100, 100})); + pool->DestroyLayers(jni_mock); + + pool->CreateLayer(gr_context_2.get(), *android_context, jni_mock, + surface_factory); + auto layer_2 = pool->GetNextLayer(); - ASSERT_TRUE(pool->HasLayers()); - ASSERT_NE(nullptr, layer_1); - ASSERT_EQ(layer_1, layer_2); - ASSERT_EQ(reinterpret_cast(gr_context_2.get()), - layer_1->gr_context_key); - ASSERT_EQ(reinterpret_cast(gr_context_2.get()), - layer_2->gr_context_key); + EXPECT_TRUE(pool->size() > 0); + EXPECT_NE(nullptr, layer_1); + EXPECT_NE(layer_1, layer_2); } TEST(SurfacePool, GetLayerAllocateTwoLayers) { @@ -171,17 +177,21 @@ TEST(SurfacePool, GetLayerAllocateTwoLayers) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - auto layer_1 = pool->GetLayer(gr_context.get(), *android_context, jni_mock, - surface_factory); - auto layer_2 = pool->GetLayer(gr_context.get(), *android_context, jni_mock, - surface_factory); - - ASSERT_TRUE(pool->HasLayers()); - ASSERT_NE(nullptr, layer_1); - ASSERT_NE(nullptr, layer_2); - ASSERT_NE(layer_1, layer_2); - ASSERT_EQ(0, layer_1->id); - ASSERT_EQ(1, layer_2->id); + + EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{100, 100})); + for (auto i = 0; i < 2; i++) { + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); + } + auto layer_1 = pool->GetNextLayer(); + auto layer_2 = pool->GetNextLayer(); + + EXPECT_EQ(pool->size(), 1u); + EXPECT_NE(nullptr, layer_1); + EXPECT_NE(nullptr, layer_2); + EXPECT_NE(layer_1, layer_2); + EXPECT_EQ(0, layer_1->id); + EXPECT_EQ(1, layer_2->id); } TEST(SurfacePool, DestroyLayers) { @@ -210,14 +220,16 @@ TEST(SurfacePool, DestroyLayers) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - pool->GetLayer(gr_context.get(), *android_context, jni_mock, surface_factory); + EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{100, 100})); + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); - ASSERT_TRUE(pool->HasLayers()); + EXPECT_EQ(pool->size(), 1u); pool->DestroyLayers(jni_mock); - ASSERT_FALSE(pool->HasLayers()); + EXPECT_EQ(pool->size(), 0u); ASSERT_TRUE(pool->GetUnusedLayers().empty()); } @@ -239,7 +251,8 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) { EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); - pool->SetFrameSize(SkISize::Make(10, 10)); + EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{10, 10})); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .Times(1) @@ -247,23 +260,27 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) { ByMove(std::make_unique( 0, window)))); - ASSERT_FALSE(pool->HasLayers()); - - pool->GetLayer(gr_context.get(), *android_context, jni_mock, surface_factory); + EXPECT_EQ(pool->size(), 0u); + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); - ASSERT_TRUE(pool->HasLayers()); + EXPECT_EQ(pool->size(), 1u); + EXPECT_TRUE(pool->CheckLayerProperties(gr_context.get(), SkISize{20, 20})); + pool->DestroyLayers(jni_mock); - 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()); - ASSERT_TRUE(pool->HasLayers()); + pool->CreateLayer(gr_context.get(), *android_context, jni_mock, + surface_factory); + pool->GetNextLayer(); + + EXPECT_TRUE(pool->GetUnusedLayers().empty()); + EXPECT_EQ(pool->size(), 1u); } } // namespace testing From 1caee35e17ccb42f9c226fd863993411e7655731 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 7 Aug 2024 17:21:58 -0700 Subject: [PATCH 10/21] ++ --- .../external_view_embedder.cc | 32 ++++++----- .../external_view_embedder_unittests.cc | 55 ------------------- 2 files changed, 19 insertions(+), 68 deletions(-) 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 0233ce1589e2f..2756981f742c1 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -116,17 +116,20 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( surface_pool_->CheckLayerProperties(context, frame_size_); if (destroy_all_layers || surface_pool_->size() < overlay_layers.size()) { auto latch = std::make_shared(1u); - task_runners_.GetPlatformTaskRunner()->PostTask([&]() { - if (destroy_all_layers) { - surface_pool_->DestroyLayers(jni_facade_); - } - for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { - surface_pool_->CreateLayer(context, android_context_, jni_facade_, - surface_factory_); - } - latch->CountDown(); - }); - latch->Wait(); + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetPlatformTaskRunner(), [&]() { + if (destroy_all_layers) { + surface_pool_->DestroyLayers(jni_facade_); + } + for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { + surface_pool_->CreateLayer(context, android_context_, jni_facade_, + surface_factory_); + } + latch->CountDown(); + }); + if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { + latch->Wait(); + } } std::unordered_map> layers; @@ -156,7 +159,8 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( } surface_pool_->RecycleLayers(); - task_runners_.GetPlatformTaskRunner()->PostTask( + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetPlatformTaskRunner(), [&, composition_order = composition_order_, view_params = view_params_, overlay_layers = std::move(overlay_layers), layers = std::move(layers)]() { @@ -279,7 +283,9 @@ void AndroidExternalViewEmbedder::DestroySurfaces() { surface_pool_->DestroyLayers(jni_facade_); latch.Signal(); }); - latch.Wait(); + if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { + latch.Wait(); + } } } // namespace flutter 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 20282ba848333..3b5a9db30054a 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 @@ -134,61 +134,6 @@ TEST(AndroidExternalViewEmbedder, CancelFrame) { ASSERT_EQ(embedder->CompositeEmbeddedView(0), nullptr); } -TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { - auto jni_mock = std::make_shared(); - auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); - auto embedder = std::make_unique( - android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); - - fml::Thread rasterizer_thread("rasterizer"); - auto raster_thread_merger = - GetThreadMergerFromPlatformThread(&rasterizer_thread); - ASSERT_FALSE(raster_thread_merger->IsMerged()); - - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(SkISize::Make(10, 20), 1.0); - - // Push a platform view. - embedder->PrerollCompositeEmbeddedView( - 0, std::make_unique()); - - auto postpreroll_result = embedder->PostPrerollAction(raster_thread_merger); - ASSERT_EQ(PostPrerollResult::kSkipAndRetryFrame, postpreroll_result); - - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); - embedder->EndFrame(/*should_resubmit_frame=*/true, raster_thread_merger); - - ASSERT_TRUE(raster_thread_merger->IsMerged()); - - int pending_frames = 0; - while (raster_thread_merger->IsMerged()) { - raster_thread_merger->DecrementLease(); - pending_frames++; - } - ASSERT_EQ(10, pending_frames); // kDefaultMergedLeaseDuration -} - -TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { - auto jni_mock = std::make_shared(); - auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); - auto embedder = std::make_unique( - android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); - - fml::Thread rasterizer_thread("rasterizer"); - auto raster_thread_merger = - GetThreadMergerFromPlatformThread(&rasterizer_thread); - ASSERT_FALSE(raster_thread_merger->IsMerged()); - - PostPrerollResult result = embedder->PostPrerollAction(raster_thread_merger); - ASSERT_EQ(PostPrerollResult::kSuccess, result); - - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); - embedder->EndFrame(/*should_resubmit_frame=*/true, raster_thread_merger); - - ASSERT_FALSE(raster_thread_merger->IsMerged()); -} - TEST(AndroidExternalViewEmbedder, PlatformViewRect) { auto jni_mock = std::make_shared(); From 7eeae7993021f95e677a0148fc8bc65bd1b9899e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 7 Aug 2024 19:31:13 -0700 Subject: [PATCH 11/21] test fixes. --- .../external_view_embedder.cc | 5 +- .../external_view_embedder_unittests.cc | 146 ++---------------- .../surface_pool_unittests.cc | 32 ++-- 3 files changed, 38 insertions(+), 145 deletions(-) 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 2756981f742c1..edbfde22a2fe9 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -13,7 +13,6 @@ #include "flutter/common/constants.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/trace_event.h" -#include "fml/make_copyable.h" #include "fml/synchronization/count_down_latch.h" #include "shell/platform/android/external_view_embedder/surface_pool.h" @@ -215,6 +214,10 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( // |ExternalViewEmbedder| PostPrerollResult AndroidExternalViewEmbedder::PostPrerollAction( const fml::RefPtr& raster_thread_merger) { + if (!FrameHasPlatformLayers()) { + return PostPrerollResult::kSuccess; + } + if (previous_frame_view_count_ == 0) { return PostPrerollResult::kResubmitFrame; } 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 3b5a9db30054a..0e11f9d5336bb 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 @@ -144,7 +144,6 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); embedder->PrepareFlutterView(SkISize::Make(100, 100), 1.5); @@ -172,7 +171,6 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRectChangedParams) { auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(nullptr, raster_thread_merger); embedder->PrepareFlutterView(SkISize::Make(100, 100), 1.5); @@ -272,7 +270,6 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { auto postpreroll_result = embedder->PostPrerollAction(raster_thread_merger); ASSERT_EQ(PostPrerollResult::kSuccess, postpreroll_result); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } @@ -411,7 +408,6 @@ TEST(AndroidExternalViewEmbedder, SubmitFlutterView) { auto postpreroll_result = embedder->PostPrerollAction(raster_thread_merger); ASSERT_EQ(PostPrerollResult::kSuccess, postpreroll_result); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } } @@ -514,10 +510,10 @@ TEST(AndroidExternalViewEmbedder, OverlayCoverTwoPlatformViews) { [](const SurfaceFrame& surface_frame) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(1); embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } @@ -621,10 +617,10 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) { [](const SurfaceFrame& surface_frame) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(1); embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } @@ -693,10 +689,11 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) { [](const SurfaceFrame& surface_frame) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } @@ -768,8 +765,6 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { // 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); @@ -798,149 +793,34 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { }, [](const SurfaceFrame& surface_frame) { return true; }, /*frame_size=*/SkISize::Make(800, 600)); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, std::move(surface_frame)); - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); // Change the frame size. embedder->BeginFrame(nullptr, raster_thread_merger); embedder->PrepareFlutterView(SkISize::Make(30, 40), 1.0); -} - -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); - SurfaceFrame::FramebufferInfo framebuffer_info; - auto surface_factory = std::make_shared( - [gr_context, window, frame_size, framebuffer_info]() { - auto surface_frame_1 = std::make_unique( - SkSurfaces::Null(1000, 1000), framebuffer_info, - [](const SurfaceFrame& surface_frame, DlCanvas* canvas) { - return true; - }, - [](const SurfaceFrame& surface_frame) { return true; }, - /*frame_size=*/SkISize::Make(800, 600)); - - 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, GetTaskRunnersForFixture()); - - // ------------------ First frame ------------------ // { - fml::Thread rasterizer_thread("rasterizer"); - auto raster_thread_merger = - GetThreadMergerFromPlatformThread(&rasterizer_thread); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(frame_size, 1.5); - - // 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), DlPaint()); - - // 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, 150, 150)); - + SurfaceFrame::FramebufferInfo framebuffer_info; auto surface_frame = std::make_unique( - SkSurfaces::Null(1000, 1000), framebuffer_info, + SkSurfaces::Null(30, 40), framebuffer_info, [](const SurfaceFrame& surface_frame, DlCanvas* canvas) { return true; }, [](const SurfaceFrame& surface_frame) { return true; }, - /*frame_size=*/SkISize::Make(800, 600)); - embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, - std::move(surface_frame)); + /*frame_size=*/SkISize::Make(30, 40)); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); - embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); + embedder->SubmitFlutterView(kImplicitViewId, gr_context.get(), nullptr, + std::move(surface_frame)); } - - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); - - fml::Thread platform_thread("platform"); - embedder->BeginFrame(nullptr, - GetThreadMergerFromRasterThread(&platform_thread)); - embedder->PrepareFlutterView(SkISize::Make(30, 40), 1.0); -} - -TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { - auto jni_mock = std::make_shared(); - auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); - auto embedder = std::make_unique( - android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); - ASSERT_TRUE(embedder->SupportsDynamicThreadMerging()); -} - -TEST(AndroidExternalViewEmbedder, DisableThreadMerger) { - auto jni_mock = std::make_shared(); - auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); - auto embedder = std::make_unique( - android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); - - fml::Thread platform_thread("platform"); - auto raster_thread_merger = GetThreadMergerFromRasterThread(&platform_thread); - ASSERT_FALSE(raster_thread_merger->IsMerged()); - - // The shell may disable the thread merger during `OnPlatformViewDestroyed`. - raster_thread_merger->Disable(); - - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); - - embedder->BeginFrame(nullptr, raster_thread_merger); - embedder->PrepareFlutterView(SkISize::Make(10, 20), 1.0); - // Push a platform view. - embedder->PrerollCompositeEmbeddedView( - 0, std::make_unique()); - - auto postpreroll_result = embedder->PostPrerollAction(raster_thread_merger); - ASSERT_EQ(PostPrerollResult::kSkipAndRetryFrame, postpreroll_result); - - EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(0); - embedder->EndFrame(/*should_resubmit_frame=*/true, raster_thread_merger); - - ASSERT_FALSE(raster_thread_merger->IsMerged()); } TEST(AndroidExternalViewEmbedder, Teardown) { 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 15d03cc85b89d..830bc73d4e6a3 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -5,7 +5,6 @@ #include #include "flutter/shell/platform/android/external_view_embedder/surface_pool.h" -#include "flutter/fml/make_copyable.h" #include "flutter/shell/platform/android/jni/jni_mock.h" #include "flutter/shell/platform/android/surface/android_surface_mock.h" #include "gmock/gmock.h" @@ -93,9 +92,9 @@ TEST(SurfacePool, GetUnusedLayers) { EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{100, 100})); pool->CreateLayer(gr_context.get(), *android_context, jni_mock, surface_factory); - EXPECT_EQ(0UL, pool->GetUnusedLayers().size()); auto layer = pool->GetNextLayer(); + EXPECT_EQ(0UL, pool->GetUnusedLayers().size()); pool->RecycleLayers(); EXPECT_EQ(pool->size(), 1u); @@ -110,27 +109,38 @@ TEST(SurfacePool, GetLayerRecycle) { auto jni_mock = std::make_shared(); auto window = fml::MakeRefCounted(nullptr); EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .Times(2) .WillOnce(Return( ByMove(std::make_unique( - 0, window)))); + 0, window)))) + .WillOnce(Return( + ByMove(std::make_unique( + 1, window)))); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); + bool first = true; auto gr_context_2 = GrDirectContext::MakeMock(nullptr); auto surface_factory = std::make_shared( - [gr_context_1, gr_context_2, window]() { + [gr_context_1, gr_context_2, &first, window]() { auto android_surface_mock = std::make_unique(); // Allocate two GPU surfaces for each gr context. - EXPECT_CALL(*android_surface_mock, - CreateGPUSurface(gr_context_1.get())); - EXPECT_CALL(*android_surface_mock, - CreateGPUSurface(gr_context_2.get())); + if (first) { + EXPECT_CALL(*android_surface_mock, + CreateGPUSurface(gr_context_1.get())); + } else { + EXPECT_CALL(*android_surface_mock, + CreateGPUSurface(gr_context_2.get())); + } + first = false; // Set the native window once. EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); return android_surface_mock; }); + EXPECT_FALSE( pool->CheckLayerProperties(gr_context_1.get(), SkISize{100, 100})); pool->CreateLayer(gr_context_1.get(), *android_context, jni_mock, @@ -186,7 +196,7 @@ TEST(SurfacePool, GetLayerAllocateTwoLayers) { auto layer_1 = pool->GetNextLayer(); auto layer_2 = pool->GetNextLayer(); - EXPECT_EQ(pool->size(), 1u); + EXPECT_EQ(pool->size(), 2u); EXPECT_NE(nullptr, layer_1); EXPECT_NE(nullptr, layer_2); EXPECT_NE(layer_1, layer_2); @@ -253,7 +263,7 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) { }); EXPECT_FALSE(pool->CheckLayerProperties(gr_context.get(), SkISize{10, 10})); - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .Times(1) .WillOnce(Return( @@ -268,7 +278,7 @@ TEST(SurfacePool, DestroyLayersFrameSizeChanged) { EXPECT_TRUE(pool->CheckLayerProperties(gr_context.get(), SkISize{20, 20})); pool->DestroyLayers(jni_mock); - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); + // EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .Times(1) .WillOnce(Return( From 93bf93e68c105eb9470b131a7f10b969b964f00b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 10:55:34 -0700 Subject: [PATCH 12/21] only create surfaces on raster thread. --- .../android/android_surface_gl_skia.cc | 15 ++++----- .../external_view_embedder.cc | 33 +++++-------------- .../flutter/embedding/engine/FlutterJNI.java | 14 +++++--- .../android/surface/android_surface.cc | 1 - 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/shell/platform/android/android_surface_gl_skia.cc b/shell/platform/android/android_surface_gl_skia.cc index 6a5dda8a06082..a0ce4592d8c12 100644 --- a/shell/platform/android/android_surface_gl_skia.cc +++ b/shell/platform/android/android_surface_gl_skia.cc @@ -52,15 +52,14 @@ std::unique_ptr AndroidSurfaceGLSkia::CreateGPUSurface( if (gr_context) { return std::make_unique(sk_ref_sp(gr_context), this, true); - } else { - sk_sp main_skia_context = - android_context_->GetMainSkiaContext(); - if (!main_skia_context) { - main_skia_context = GPUSurfaceGLSkia::MakeGLContext(this); - android_context_->SetMainSkiaContext(main_skia_context); - } - return std::make_unique(main_skia_context, this, true); } + sk_sp main_skia_context = + android_context_->GetMainSkiaContext(); + if (!main_skia_context) { + main_skia_context = GPUSurfaceGLSkia::MakeGLContext(this); + android_context_->SetMainSkiaContext(main_skia_context); + } + return std::make_unique(main_skia_context, this, true); } bool AndroidSurfaceGLSkia::OnScreenSurfaceResize(const SkISize& size) { 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 edbfde22a2fe9..971f79ac8076d 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -110,24 +110,17 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( frame->Submit(); } - // Overlay layers must be created or destroyed on the platfor thread. bool destroy_all_layers = surface_pool_->CheckLayerProperties(context, frame_size_); if (destroy_all_layers || surface_pool_->size() < overlay_layers.size()) { - auto latch = std::make_shared(1u); - fml::TaskRunner::RunNowOrPostTask( - task_runners_.GetPlatformTaskRunner(), [&]() { - if (destroy_all_layers) { - surface_pool_->DestroyLayers(jni_facade_); - } - for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { - surface_pool_->CreateLayer(context, android_context_, jni_facade_, - surface_factory_); - } - latch->CountDown(); - }); - if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { - latch->Wait(); + if (destroy_all_layers) { + surface_pool_->DestroyLayers(jni_facade_); + } + for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { + surface_pool_->CreateLayer(context, // + android_context_, // + jni_facade_, // + surface_factory_); // } } @@ -280,15 +273,7 @@ void AndroidExternalViewEmbedder::DestroySurfaces() { if (surface_pool_->size() == 0) { return; } - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(), - [&]() { - surface_pool_->DestroyLayers(jni_facade_); - latch.Signal(); - }); - if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { - latch.Wait(); - } + surface_pool_->DestroyLayers(jni_facade_); } } // namespace flutter diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 8edb708f65545..91b0c288f67e6 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1241,9 +1241,8 @@ public void onEndFrame() { } @SuppressWarnings("unused") - @UiThread public FlutterOverlaySurface createOverlaySurface() { - ensureRunningOnMainThread(); + ensureNotRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to position an overlay surface"); @@ -1252,9 +1251,8 @@ public FlutterOverlaySurface createOverlaySurface() { } @SuppressWarnings("unused") - @UiThread public void destroyOverlaySurfaces() { - ensureRunningOnMainThread(); + ensureNotRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to destroy an overlay surface"); @@ -1491,6 +1489,14 @@ private void ensureRunningOnMainThread() { } } + private void ensureNotRunningOnMainThread() { + if (Looper.myLooper() == mainLooper) { + throw new RuntimeException( + "Method must not be called from UI thread. Current thread: " + + Thread.currentThread().getName()); + } + } + /** * Delegate responsible for creating and updating Android-side caches of Flutter's semantics tree * and custom accessibility actions. diff --git a/shell/platform/android/surface/android_surface.cc b/shell/platform/android/surface/android_surface.cc index e301491b5fce0..f7f0be6361417 100644 --- a/shell/platform/android/surface/android_surface.cc +++ b/shell/platform/android/surface/android_surface.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "flutter/shell/platform/android/surface/android_surface.h" -#include "flutter/fml/logging.h" namespace flutter { From 4b0cf8c094a7a21d1ca008dd44ab5e65f2dceab1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 11:51:33 -0700 Subject: [PATCH 13/21] ++ --- .../io/flutter/embedding/engine/FlutterJNI.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 91b0c288f67e6..2ec8f6177fe29 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1242,7 +1242,6 @@ public void onEndFrame() { @SuppressWarnings("unused") public FlutterOverlaySurface createOverlaySurface() { - ensureNotRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to position an overlay surface"); @@ -1252,7 +1251,6 @@ public FlutterOverlaySurface createOverlaySurface() { @SuppressWarnings("unused") public void destroyOverlaySurfaces() { - ensureNotRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to destroy an overlay surface"); @@ -1489,14 +1487,6 @@ private void ensureRunningOnMainThread() { } } - private void ensureNotRunningOnMainThread() { - if (Looper.myLooper() == mainLooper) { - throw new RuntimeException( - "Method must not be called from UI thread. Current thread: " - + Thread.currentThread().getName()); - } - } - /** * Delegate responsible for creating and updating Android-side caches of Flutter's semantics tree * and custom accessibility actions. From fa586a324b46b7b01b5a8bb5cd537f57b06422a5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 19:21:36 -0700 Subject: [PATCH 14/21] make sure we dont render when the engine is gone. --- .../android/android_shell_holder_unittests.cc | 1 + .../external_view_embedder.cc | 9 ++++++++ .../embedding/android/FlutterView.java | 1 - .../flutter/embedding/engine/FlutterJNI.java | 10 ++++++++ .../platform/PlatformViewsController.java | 4 ++++ .../android/jni/platform_view_android_jni.h | 2 ++ .../android/platform_view_android_jni_impl.cc | 23 ++++++++++++++++++- .../android/platform_view_android_jni_impl.h | 2 ++ 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 0a1f1ed3fcf5d..626f5746729e2 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -80,6 +80,7 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack), (override)); + MOCK_METHOD(bool, IsRendererAttached, (), (override)); MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), 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 971f79ac8076d..c7f6d4759b50c 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -158,6 +158,9 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( layers = std::move(layers)]() { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); + if (!jni_facade_->IsRendererAttached()) { + return; + } jni_facade_->FlutterViewBeginFrame(); for (int64_t view_id : composition_order) { @@ -266,6 +269,12 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::Teardown() { DestroySurfaces(); + // Post a platform task to ensure that the task runner has flushed all + // platform view composition. + auto latch = std::make_shared(1u); + fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(), + [&latch]() { latch->CountDown(); }); + latch->Wait(); } // |ExternalViewEmbedder| diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 7272b5125ba05..1806309d0d35b 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1355,7 +1355,6 @@ public boolean acquireLatestImageViewFrame() { * Returns true if this {@code FlutterView} is currently attached to a {@link * io.flutter.embedding.engine.FlutterEngine}. */ - @VisibleForTesting public boolean isAttachedToFlutterEngine() { return flutterEngine != null && flutterEngine.getRenderer() == renderSurface.getAttachedRenderer(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 2ec8f6177fe29..8b8f3139d8c3d 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1454,6 +1454,16 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } + @SuppressWarnings("unused") + @UiThread + public boolean isRendererAttached() { + ensureRunningOnMainThread(); + if (platformViewsController == null) { + return false; + } + return platformViewsController.isRendererAttached(); + } + @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index a7f85dc2d654f..5a153d42defa8 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -832,6 +832,10 @@ public void attachToView(@NonNull FlutterView newFlutterView) { } } + public boolean isRendererAttached() { + return flutterView != null && flutterView.isAttachedToFlutterEngine(); + } + /** * Detaches the controller from {@link FlutterView}. * diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index 356b6776fc631..d3ec0e53be79c 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -153,6 +153,8 @@ class PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack) = 0; + virtual bool IsRendererAttached() = 0; + //---------------------------------------------------------------------------- /// @brief Positions and sizes an overlay surface in hybrid composition. /// diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 8a3f0d7bd5989..0993834248e14 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -132,7 +132,7 @@ static jmethodID g_request_dart_deferred_library_method = nullptr; // Called By Java static jmethodID g_on_display_platform_view_method = nullptr; -// static jmethodID g_on_composite_platform_view_method = nullptr; +static jmethodID g_is_renderer_attached_method = nullptr; static jmethodID g_on_display_overlay_surface_method = nullptr; @@ -1101,6 +1101,13 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } + g_is_renderer_attached_method = + env->GetMethodID(g_flutter_jni_class->obj(), "isRendererAttached", "()Z"); + if (g_is_renderer_attached_method == nullptr) { + FML_LOG(ERROR) << "Could not locate isRendererAttached method"; + return false; + } + g_on_begin_frame_method = env->GetMethodID(g_flutter_jni_class->obj(), "onBeginFrame", "()V"); @@ -1618,6 +1625,20 @@ void PlatformViewAndroidJNIImpl::HardwareBufferClose( FML_CHECK(fml::jni::CheckException(env)); } +bool PlatformViewAndroidJNIImpl::IsRendererAttached() { + JNIEnv* env = fml::jni::AttachCurrentThread(); + + auto java_object = java_object_.get(env); + if (java_object.is_null()) { + return false; + } + + jboolean result = + env->CallBooleanMethod(java_object.obj(), g_is_renderer_attached_method); + FML_CHECK(fml::jni::CheckException(env)); + return result; +} + void PlatformViewAndroidJNIImpl::FlutterViewOnDisplayPlatformView( int view_id, int x, diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 73cb6a43cffdb..3e8f88b9d5a0f 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -81,6 +81,8 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void FlutterViewEndFrame() override; + bool IsRendererAttached() override; + std::unique_ptr FlutterViewCreateOverlaySurface() override; From 401d3a4641ad39ae06b155d2714c3f8a78cf5d7b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 19:58:04 -0700 Subject: [PATCH 15/21] remove extra method. --- .../android/android_shell_holder_unittests.cc | 1 - .../android/android_surface_gl_skia.cc | 15 ++++++------ .../external_view_embedder.cc | 3 --- .../embedding/android/FlutterView.java | 1 + .../flutter/embedding/engine/FlutterJNI.java | 10 -------- .../platform/PlatformViewsController.java | 4 ---- .../android/jni/platform_view_android_jni.h | 2 -- .../android/platform_view_android_jni_impl.cc | 23 ------------------- .../android/platform_view_android_jni_impl.h | 2 -- 9 files changed, 9 insertions(+), 52 deletions(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 626f5746729e2..0a1f1ed3fcf5d 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -80,7 +80,6 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack), (override)); - MOCK_METHOD(bool, IsRendererAttached, (), (override)); MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), diff --git a/shell/platform/android/android_surface_gl_skia.cc b/shell/platform/android/android_surface_gl_skia.cc index a0ce4592d8c12..6a5dda8a06082 100644 --- a/shell/platform/android/android_surface_gl_skia.cc +++ b/shell/platform/android/android_surface_gl_skia.cc @@ -52,14 +52,15 @@ std::unique_ptr AndroidSurfaceGLSkia::CreateGPUSurface( if (gr_context) { return std::make_unique(sk_ref_sp(gr_context), this, true); + } else { + sk_sp main_skia_context = + android_context_->GetMainSkiaContext(); + if (!main_skia_context) { + main_skia_context = GPUSurfaceGLSkia::MakeGLContext(this); + android_context_->SetMainSkiaContext(main_skia_context); + } + return std::make_unique(main_skia_context, this, true); } - sk_sp main_skia_context = - android_context_->GetMainSkiaContext(); - if (!main_skia_context) { - main_skia_context = GPUSurfaceGLSkia::MakeGLContext(this); - android_context_->SetMainSkiaContext(main_skia_context); - } - return std::make_unique(main_skia_context, this, true); } bool AndroidSurfaceGLSkia::OnScreenSurfaceResize(const SkISize& size) { 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 c7f6d4759b50c..3f4700bc16b60 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -158,9 +158,6 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( layers = std::move(layers)]() { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); - if (!jni_facade_->IsRendererAttached()) { - return; - } jni_facade_->FlutterViewBeginFrame(); for (int64_t view_id : composition_order) { diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 1806309d0d35b..7272b5125ba05 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1355,6 +1355,7 @@ public boolean acquireLatestImageViewFrame() { * Returns true if this {@code FlutterView} is currently attached to a {@link * io.flutter.embedding.engine.FlutterEngine}. */ + @VisibleForTesting public boolean isAttachedToFlutterEngine() { return flutterEngine != null && flutterEngine.getRenderer() == renderSurface.getAttachedRenderer(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 8b8f3139d8c3d..2ec8f6177fe29 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1454,16 +1454,6 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } - @SuppressWarnings("unused") - @UiThread - public boolean isRendererAttached() { - ensureRunningOnMainThread(); - if (platformViewsController == null) { - return false; - } - return platformViewsController.isRendererAttached(); - } - @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 5a153d42defa8..a7f85dc2d654f 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -832,10 +832,6 @@ public void attachToView(@NonNull FlutterView newFlutterView) { } } - public boolean isRendererAttached() { - return flutterView != null && flutterView.isAttachedToFlutterEngine(); - } - /** * Detaches the controller from {@link FlutterView}. * diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index d3ec0e53be79c..356b6776fc631 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -153,8 +153,6 @@ class PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack) = 0; - virtual bool IsRendererAttached() = 0; - //---------------------------------------------------------------------------- /// @brief Positions and sizes an overlay surface in hybrid composition. /// diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 0993834248e14..cf773cb82e4fb 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -132,8 +132,6 @@ static jmethodID g_request_dart_deferred_library_method = nullptr; // Called By Java static jmethodID g_on_display_platform_view_method = nullptr; -static jmethodID g_is_renderer_attached_method = nullptr; - static jmethodID g_on_display_overlay_surface_method = nullptr; static jmethodID g_overlay_surface_id_method = nullptr; @@ -1101,13 +1099,6 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } - g_is_renderer_attached_method = - env->GetMethodID(g_flutter_jni_class->obj(), "isRendererAttached", "()Z"); - if (g_is_renderer_attached_method == nullptr) { - FML_LOG(ERROR) << "Could not locate isRendererAttached method"; - return false; - } - g_on_begin_frame_method = env->GetMethodID(g_flutter_jni_class->obj(), "onBeginFrame", "()V"); @@ -1625,20 +1616,6 @@ void PlatformViewAndroidJNIImpl::HardwareBufferClose( FML_CHECK(fml::jni::CheckException(env)); } -bool PlatformViewAndroidJNIImpl::IsRendererAttached() { - JNIEnv* env = fml::jni::AttachCurrentThread(); - - auto java_object = java_object_.get(env); - if (java_object.is_null()) { - return false; - } - - jboolean result = - env->CallBooleanMethod(java_object.obj(), g_is_renderer_attached_method); - FML_CHECK(fml::jni::CheckException(env)); - return result; -} - void PlatformViewAndroidJNIImpl::FlutterViewOnDisplayPlatformView( int view_id, int x, diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 3e8f88b9d5a0f..73cb6a43cffdb 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -81,8 +81,6 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void FlutterViewEndFrame() override; - bool IsRendererAttached() override; - std::unique_ptr FlutterViewCreateOverlaySurface() override; From 708b34e64bc0349b804a4a9f88b4a56cfc59b13b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 20:50:02 -0700 Subject: [PATCH 16/21] add back JNI check. --- .../android/android_shell_holder_unittests.cc | 1 + .../external_view_embedder.cc | 3 +++ .../embedding/android/FlutterView.java | 1 - .../flutter/embedding/engine/FlutterJNI.java | 10 ++++++++ .../platform/PlatformViewsController.java | 4 ++++ shell/platform/android/jni/jni_mock.h | 2 ++ .../android/jni/platform_view_android_jni.h | 8 +++++++ .../android/platform_view_android_jni_impl.cc | 23 +++++++++++++++++++ .../android/platform_view_android_jni_impl.h | 2 ++ 9 files changed, 53 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 0a1f1ed3fcf5d..626f5746729e2 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -80,6 +80,7 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack), (override)); + MOCK_METHOD(bool, IsRendererAttached, (), (override)); MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), 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 3f4700bc16b60..c7f6d4759b50c 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -158,6 +158,9 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( layers = std::move(layers)]() { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); + if (!jni_facade_->IsRendererAttached()) { + return; + } jni_facade_->FlutterViewBeginFrame(); for (int64_t view_id : composition_order) { diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 7272b5125ba05..1806309d0d35b 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1355,7 +1355,6 @@ public boolean acquireLatestImageViewFrame() { * Returns true if this {@code FlutterView} is currently attached to a {@link * io.flutter.embedding.engine.FlutterEngine}. */ - @VisibleForTesting public boolean isAttachedToFlutterEngine() { return flutterEngine != null && flutterEngine.getRenderer() == renderSurface.getAttachedRenderer(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 2ec8f6177fe29..8b8f3139d8c3d 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1454,6 +1454,16 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } + @SuppressWarnings("unused") + @UiThread + public boolean isRendererAttached() { + ensureRunningOnMainThread(); + if (platformViewsController == null) { + return false; + } + return platformViewsController.isRendererAttached(); + } + @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index a7f85dc2d654f..5a153d42defa8 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -832,6 +832,10 @@ public void attachToView(@NonNull FlutterView newFlutterView) { } } + public boolean isRendererAttached() { + return flutterView != null && flutterView.isAttachedToFlutterEngine(); + } + /** * Detaches the controller from {@link FlutterView}. * diff --git a/shell/platform/android/jni/jni_mock.h b/shell/platform/android/jni/jni_mock.h index cf5b329256e6c..e9a2c29205fcb 100644 --- a/shell/platform/android/jni/jni_mock.h +++ b/shell/platform/android/jni/jni_mock.h @@ -98,6 +98,8 @@ class JNIMock final : public PlatformViewAndroidJNI { MutatorsStack mutators_stack), (override)); + MOCK_METHOD(bool, IsRendererAttached, (), (override)); + MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index 356b6776fc631..e493b08c17d0d 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -153,6 +153,14 @@ class PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack) = 0; + //---------------------------------------------------------------------------- + /// @brief Verifies that the flutter view is still attached to the + /// engine. + /// + /// @note Must be called from the platform thread. + /// + virtual bool IsRendererAttached() = 0; + //---------------------------------------------------------------------------- /// @brief Positions and sizes an overlay surface in hybrid composition. /// diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index cf773cb82e4fb..0993834248e14 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -132,6 +132,8 @@ static jmethodID g_request_dart_deferred_library_method = nullptr; // Called By Java static jmethodID g_on_display_platform_view_method = nullptr; +static jmethodID g_is_renderer_attached_method = nullptr; + static jmethodID g_on_display_overlay_surface_method = nullptr; static jmethodID g_overlay_surface_id_method = nullptr; @@ -1099,6 +1101,13 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } + g_is_renderer_attached_method = + env->GetMethodID(g_flutter_jni_class->obj(), "isRendererAttached", "()Z"); + if (g_is_renderer_attached_method == nullptr) { + FML_LOG(ERROR) << "Could not locate isRendererAttached method"; + return false; + } + g_on_begin_frame_method = env->GetMethodID(g_flutter_jni_class->obj(), "onBeginFrame", "()V"); @@ -1616,6 +1625,20 @@ void PlatformViewAndroidJNIImpl::HardwareBufferClose( FML_CHECK(fml::jni::CheckException(env)); } +bool PlatformViewAndroidJNIImpl::IsRendererAttached() { + JNIEnv* env = fml::jni::AttachCurrentThread(); + + auto java_object = java_object_.get(env); + if (java_object.is_null()) { + return false; + } + + jboolean result = + env->CallBooleanMethod(java_object.obj(), g_is_renderer_attached_method); + FML_CHECK(fml::jni::CheckException(env)); + return result; +} + void PlatformViewAndroidJNIImpl::FlutterViewOnDisplayPlatformView( int view_id, int x, diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 73cb6a43cffdb..3e8f88b9d5a0f 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -81,6 +81,8 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void FlutterViewEndFrame() override; + bool IsRendererAttached() override; + std::unique_ptr FlutterViewCreateOverlaySurface() override; From ec2bd9276345fe5dadc7e293fa7b214988f4db96 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 21:58:59 -0700 Subject: [PATCH 17/21] try again. --- .../android/android_shell_holder_unittests.cc | 1 - .../external_view_embedder.cc | 5 +--- .../embedding/android/FlutterView.java | 1 + .../flutter/embedding/engine/FlutterJNI.java | 10 -------- .../platform/PlatformViewsController.java | 4 ---- shell/platform/android/jni/jni_mock.h | 2 -- .../android/jni/platform_view_android_jni.h | 8 ------- .../android/platform_view_android_jni_impl.cc | 23 ------------------- .../android/platform_view_android_jni_impl.h | 2 -- 9 files changed, 2 insertions(+), 54 deletions(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 626f5746729e2..0a1f1ed3fcf5d 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -80,7 +80,6 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack), (override)); - MOCK_METHOD(bool, IsRendererAttached, (), (override)); MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), 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 c7f6d4759b50c..78c3aa14aa6cd 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -158,9 +158,6 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( layers = std::move(layers)]() { TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); - if (!jni_facade_->IsRendererAttached()) { - return; - } jni_facade_->FlutterViewBeginFrame(); for (int64_t view_id : composition_order) { @@ -268,13 +265,13 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::Teardown() { - DestroySurfaces(); // Post a platform task to ensure that the task runner has flushed all // platform view composition. auto latch = std::make_shared(1u); fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(), [&latch]() { latch->CountDown(); }); latch->Wait(); + DestroySurfaces(); } // |ExternalViewEmbedder| diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 1806309d0d35b..7272b5125ba05 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1355,6 +1355,7 @@ public boolean acquireLatestImageViewFrame() { * Returns true if this {@code FlutterView} is currently attached to a {@link * io.flutter.embedding.engine.FlutterEngine}. */ + @VisibleForTesting public boolean isAttachedToFlutterEngine() { return flutterEngine != null && flutterEngine.getRenderer() == renderSurface.getAttachedRenderer(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 8b8f3139d8c3d..2ec8f6177fe29 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1454,16 +1454,6 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } - @SuppressWarnings("unused") - @UiThread - public boolean isRendererAttached() { - ensureRunningOnMainThread(); - if (platformViewsController == null) { - return false; - } - return platformViewsController.isRendererAttached(); - } - @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 5a153d42defa8..a7f85dc2d654f 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -832,10 +832,6 @@ public void attachToView(@NonNull FlutterView newFlutterView) { } } - public boolean isRendererAttached() { - return flutterView != null && flutterView.isAttachedToFlutterEngine(); - } - /** * Detaches the controller from {@link FlutterView}. * diff --git a/shell/platform/android/jni/jni_mock.h b/shell/platform/android/jni/jni_mock.h index e9a2c29205fcb..cf5b329256e6c 100644 --- a/shell/platform/android/jni/jni_mock.h +++ b/shell/platform/android/jni/jni_mock.h @@ -98,8 +98,6 @@ class JNIMock final : public PlatformViewAndroidJNI { MutatorsStack mutators_stack), (override)); - MOCK_METHOD(bool, IsRendererAttached, (), (override)); - MOCK_METHOD(void, FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index e493b08c17d0d..356b6776fc631 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -153,14 +153,6 @@ class PlatformViewAndroidJNI { int viewHeight, MutatorsStack mutators_stack) = 0; - //---------------------------------------------------------------------------- - /// @brief Verifies that the flutter view is still attached to the - /// engine. - /// - /// @note Must be called from the platform thread. - /// - virtual bool IsRendererAttached() = 0; - //---------------------------------------------------------------------------- /// @brief Positions and sizes an overlay surface in hybrid composition. /// diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 0993834248e14..cf773cb82e4fb 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -132,8 +132,6 @@ static jmethodID g_request_dart_deferred_library_method = nullptr; // Called By Java static jmethodID g_on_display_platform_view_method = nullptr; -static jmethodID g_is_renderer_attached_method = nullptr; - static jmethodID g_on_display_overlay_surface_method = nullptr; static jmethodID g_overlay_surface_id_method = nullptr; @@ -1101,13 +1099,6 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } - g_is_renderer_attached_method = - env->GetMethodID(g_flutter_jni_class->obj(), "isRendererAttached", "()Z"); - if (g_is_renderer_attached_method == nullptr) { - FML_LOG(ERROR) << "Could not locate isRendererAttached method"; - return false; - } - g_on_begin_frame_method = env->GetMethodID(g_flutter_jni_class->obj(), "onBeginFrame", "()V"); @@ -1625,20 +1616,6 @@ void PlatformViewAndroidJNIImpl::HardwareBufferClose( FML_CHECK(fml::jni::CheckException(env)); } -bool PlatformViewAndroidJNIImpl::IsRendererAttached() { - JNIEnv* env = fml::jni::AttachCurrentThread(); - - auto java_object = java_object_.get(env); - if (java_object.is_null()) { - return false; - } - - jboolean result = - env->CallBooleanMethod(java_object.obj(), g_is_renderer_attached_method); - FML_CHECK(fml::jni::CheckException(env)); - return result; -} - void PlatformViewAndroidJNIImpl::FlutterViewOnDisplayPlatformView( int view_id, int x, diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 3e8f88b9d5a0f..73cb6a43cffdb 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -81,8 +81,6 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void FlutterViewEndFrame() override; - bool IsRendererAttached() override; - std::unique_ptr FlutterViewCreateOverlaySurface() override; From 31518cf4e00fe7982d82e59b3f4f996f7724915f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 8 Aug 2024 23:15:11 -0700 Subject: [PATCH 18/21] ++ --- .../java/dev/flutter/scenariosui/MemoryLeakTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java index 72df6e3f1bc13..17fa0c07d843c 100644 --- a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java +++ b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java @@ -28,7 +28,7 @@ public class MemoryLeakTests { public void platformViewHybridComposition_launchActivityFinishAndLaunchAgain() throws Exception { Intent intent = new Intent(Intent.ACTION_MAIN); intent.putExtra("scenario_name", "platform_view"); - intent.putExtra("use_android_view", true); + // intent.putExtra("use_android_view", true); intent.putExtra("view_type", PlatformViewsActivity.TEXT_VIEW_PV); activityRule.launchActivity(intent); From 3e98c59d3ad5dbc092c39892c4716362948bb778 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 9 Aug 2024 13:41:26 -0700 Subject: [PATCH 19/21] changes --- .../android/android_shell_holder_unittests.cc | 107 +----------------- .../external_view_embedder.cc | 31 +++-- .../flutter/embedding/engine/FlutterJNI.java | 13 +++ .../platform/PlatformViewsController.java | 9 +- shell/platform/android/jni/jni_mock.h | 2 +- .../android/jni/platform_view_android_jni.h | 6 + .../android/platform_view_android_jni_impl.cc | 23 ++++ .../android/platform_view_android_jni_impl.h | 2 + .../flutter/scenariosui/MemoryLeakTests.java | 2 +- 9 files changed, 81 insertions(+), 114 deletions(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 0a1f1ed3fcf5d..17bd459e92d25 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -7,108 +7,11 @@ #include "flutter/shell/platform/android/android_shell_holder.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "shell/platform/android/jni/platform_view_android_jni.h" +#include "shell/platform/android/jni/jni_mock.h" namespace flutter { namespace testing { namespace { -class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { - public: - MOCK_METHOD(void, - FlutterViewHandlePlatformMessage, - (std::unique_ptr message, - int responseId), - (override)); - MOCK_METHOD(void, - FlutterViewHandlePlatformMessageResponse, - (int responseId, std::unique_ptr data), - (override)); - MOCK_METHOD(void, - FlutterViewUpdateSemantics, - (std::vector buffer, - std::vector strings, - std::vector> string_attribute_args), - (override)); - MOCK_METHOD(void, - FlutterViewUpdateCustomAccessibilityActions, - (std::vector actions_buffer, - std::vector strings), - (override)); - MOCK_METHOD(void, FlutterViewOnFirstFrame, (), (override)); - MOCK_METHOD(void, FlutterViewOnPreEngineRestart, (), (override)); - MOCK_METHOD(void, - SurfaceTextureAttachToGLContext, - (JavaLocalRef surface_texture, int textureId), - (override)); - MOCK_METHOD(bool, - SurfaceTextureShouldUpdate, - (JavaLocalRef surface_texture), - (override)); - MOCK_METHOD(void, - SurfaceTextureUpdateTexImage, - (JavaLocalRef surface_texture), - (override)); - MOCK_METHOD(SkM44, - SurfaceTextureGetTransformMatrix, - (JavaLocalRef surface_texture), - (override)); - MOCK_METHOD(void, - SurfaceTextureDetachFromGLContext, - (JavaLocalRef surface_texture), - (override)); - MOCK_METHOD(JavaLocalRef, - ImageProducerTextureEntryAcquireLatestImage, - (JavaLocalRef image_texture_entry), - (override)); - MOCK_METHOD(JavaLocalRef, - ImageGetHardwareBuffer, - (JavaLocalRef image), - (override)); - MOCK_METHOD(void, ImageClose, (JavaLocalRef image), (override)); - MOCK_METHOD(void, - HardwareBufferClose, - (JavaLocalRef hardware_buffer), - (override)); - MOCK_METHOD(void, - FlutterViewOnDisplayPlatformView, - (int view_id, - int x, - int y, - int width, - int height, - int viewWidth, - int viewHeight, - MutatorsStack mutators_stack), - (override)); - MOCK_METHOD(void, - FlutterViewDisplayOverlaySurface, - (int surface_id, int x, int y, int width, int height), - (override)); - MOCK_METHOD(void, FlutterViewBeginFrame, (), (override)); - MOCK_METHOD(void, FlutterViewEndFrame, (), (override)); - MOCK_METHOD(std::unique_ptr, - FlutterViewCreateOverlaySurface, - (), - (override)); - MOCK_METHOD(void, FlutterViewDestroyOverlaySurfaces, (), (override)); - MOCK_METHOD(std::unique_ptr>, - FlutterViewComputePlatformResolvedLocale, - (std::vector supported_locales_data), - (override)); - MOCK_METHOD(double, GetDisplayRefreshRate, (), (override)); - MOCK_METHOD(double, GetDisplayWidth, (), (override)); - MOCK_METHOD(double, GetDisplayHeight, (), (override)); - MOCK_METHOD(double, GetDisplayDensity, (), (override)); - MOCK_METHOD(bool, - RequestDartDeferredLibrary, - (int loading_unit_id), - (override)); - MOCK_METHOD(double, - FlutterViewGetScaledFontSize, - (double font_size, int configuration_id), - (const, override)); -}; - class MockPlatformMessageResponse : public PlatformMessageResponse { public: static fml::RefPtr Create() { @@ -122,7 +25,7 @@ class MockPlatformMessageResponse : public PlatformMessageResponse { TEST(AndroidShellHolder, Create) { Settings settings; settings.enable_software_rendering = false; - auto jni = std::make_shared(); + auto jni = std::make_shared(); auto holder = std::make_unique(settings, jni); EXPECT_NE(holder.get(), nullptr); EXPECT_TRUE(holder->IsValid()); @@ -135,7 +38,7 @@ TEST(AndroidShellHolder, Create) { TEST(AndroidShellHolder, HandlePlatformMessage) { Settings settings; settings.enable_software_rendering = false; - auto jni = std::make_shared(); + auto jni = std::make_shared(); auto holder = std::make_unique(settings, jni); EXPECT_NE(holder.get(), nullptr); EXPECT_TRUE(holder->IsValid()); @@ -164,7 +67,7 @@ TEST(AndroidShellHolder, HandlePlatformMessage) { TEST(AndroidShellHolder, CreateWithMergedPlatformAndUIThread) { Settings settings; settings.merged_platform_ui_thread = true; - auto jni = std::make_shared(); + auto jni = std::make_shared(); auto holder = std::make_unique(settings, jni); auto window = fml::MakeRefCounted( nullptr, /*is_fake_window=*/true); @@ -178,7 +81,7 @@ TEST(AndroidShellHolder, CreateWithMergedPlatformAndUIThread) { TEST(AndroidShellHolder, CreateWithUnMergedPlatformAndUIThread) { Settings settings; settings.merged_platform_ui_thread = false; - auto jni = std::make_shared(); + auto jni = std::make_shared(); auto holder = std::make_unique(settings, jni); auto window = fml::MakeRefCounted( nullptr, /*is_fake_window=*/true); 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 78c3aa14aa6cd..9b12e6685336c 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -14,6 +14,7 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/trace_event.h" #include "fml/synchronization/count_down_latch.h" +#include "fml/task_runner.h" #include "shell/platform/android/external_view_embedder/surface_pool.h" namespace flutter { @@ -110,17 +111,27 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( frame->Submit(); } + // Layers must be created and destroyed on the platform thread. If there are + // not sufficient overlays, or if the overlay properties changed then a task + // must be posted to the platform thread to construct them. bool destroy_all_layers = surface_pool_->CheckLayerProperties(context, frame_size_); if (destroy_all_layers || surface_pool_->size() < overlay_layers.size()) { - if (destroy_all_layers) { - surface_pool_->DestroyLayers(jni_facade_); - } - for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { - surface_pool_->CreateLayer(context, // - android_context_, // - jni_facade_, // - surface_factory_); // + auto latch = std::make_shared(1u); + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetPlatformTaskRunner(), [&]() { + if (destroy_all_layers) { + surface_pool_->DestroyLayers(jni_facade_); + } + for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { + surface_pool_->CreateLayer(context, // + android_context_, // + jni_facade_, // + surface_factory_); // + } + }); + if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { + latch->Wait(); } } @@ -156,6 +167,10 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( [&, composition_order = composition_order_, view_params = view_params_, overlay_layers = std::move(overlay_layers), layers = std::move(layers)]() { + if (!jni_facade_->IsAttachedToView()) { + return; + } + TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::RenderNativeViews"); jni_facade_->FlutterViewBeginFrame(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 2ec8f6177fe29..107d5a5b56684 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1241,7 +1241,9 @@ public void onEndFrame() { } @SuppressWarnings("unused") + @UiThread public FlutterOverlaySurface createOverlaySurface() { + ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to position an overlay surface"); @@ -1250,7 +1252,9 @@ public FlutterOverlaySurface createOverlaySurface() { } @SuppressWarnings("unused") + @UiThread public void destroyOverlaySurfaces() { + ensureRunningOnMainThread(); if (platformViewsController == null) { throw new RuntimeException( "platformViewsController must be set before attempting to destroy an overlay surface"); @@ -1454,6 +1458,15 @@ public void onDisplayPlatformView( viewId, x, y, width, height, viewWidth, viewHeight, mutatorsStack); } + @UiThread + public boolean isAttachedToView() { + ensureRunningOnMainThread(); + if (platformViewsController == null) { + return false; + } + return platformViewsController.isAttachedToView(); + } + @UiThread public Bitmap getBitmap() { ensureRunningOnMainThread(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index a7f85dc2d654f..cd4ccfd7a0d90 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -801,12 +801,17 @@ public void detach() { if (platformViewsChannel != null) { platformViewsChannel.setPlatformViewsHandler(null); } - destroyOverlaySurfaces(); + // destroyOverlaySurfaces(); platformViewsChannel = null; context = null; textureRegistry = null; } + /** Whether the platform view controller is attached to an engine. */ + public boolean isAttachedToView() { + return flutterView != null; + } + /** * Attaches the controller to a {@link FlutterView}. * @@ -851,7 +856,7 @@ public void detachFromView() { flutterView.removeView(view); } - destroyOverlaySurfaces(); + // destroyOverlaySurfaces(); removeOverlaySurfaces(); flutterView = null; flutterViewConvertedToImageView = false; diff --git a/shell/platform/android/jni/jni_mock.h b/shell/platform/android/jni/jni_mock.h index cf5b329256e6c..b9f636a630b01 100644 --- a/shell/platform/android/jni/jni_mock.h +++ b/shell/platform/android/jni/jni_mock.h @@ -102,7 +102,7 @@ class JNIMock final : public PlatformViewAndroidJNI { FlutterViewDisplayOverlaySurface, (int surface_id, int x, int y, int width, int height), (override)); - + MOCK_METHOD(bool, IsAttachedToView, (), (override)); MOCK_METHOD(void, FlutterViewBeginFrame, (), (override)); MOCK_METHOD(void, FlutterViewEndFrame, (), (override)); diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index 356b6776fc631..2b2a8c0b6c900 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -84,6 +84,12 @@ class PlatformViewAndroidJNI { /// virtual void FlutterViewOnPreEngineRestart() = 0; + //---------------------------------------------------------------------------- + /// @brief Whether the platform view controller is actively connected to + /// a FlutterView instance. + /// + virtual bool IsAttachedToView() = 0; + //---------------------------------------------------------------------------- /// @brief Attach the SurfaceTexture to the OpenGL ES context that is /// current on the calling thread. diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index cf773cb82e4fb..cb6497f00e2cf 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -134,6 +134,8 @@ static jmethodID g_on_display_platform_view_method = nullptr; static jmethodID g_on_display_overlay_surface_method = nullptr; +static jmethodID g_is_flutter_view_attached_method = nullptr; + static jmethodID g_overlay_surface_id_method = nullptr; static jmethodID g_overlay_surface_surface_method = nullptr; @@ -1107,6 +1109,13 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } + g_is_flutter_view_attached_method = + env->GetMethodID(g_flutter_jni_class->obj(), "isAttachedToView", "()Z"); + if (g_is_flutter_view_attached_method == nullptr) { + FML_LOG(ERROR) << "Could not locate isAttachedToView method"; + return false; + } + g_on_end_frame_method = env->GetMethodID(g_flutter_jni_class->obj(), "onEndFrame", "()V"); @@ -1616,6 +1625,20 @@ void PlatformViewAndroidJNIImpl::HardwareBufferClose( FML_CHECK(fml::jni::CheckException(env)); } +bool PlatformViewAndroidJNIImpl::IsAttachedToView() { + JNIEnv* env = fml::jni::AttachCurrentThread(); + + auto java_object = java_object_.get(env); + if (java_object.is_null()) { + return false; + } + + jboolean result = env->CallBooleanMethod(java_object.obj(), + g_is_flutter_view_attached_method); + FML_CHECK(fml::jni::CheckException(env)); + return result; +} + void PlatformViewAndroidJNIImpl::FlutterViewOnDisplayPlatformView( int view_id, int x, diff --git a/shell/platform/android/platform_view_android_jni_impl.h b/shell/platform/android/platform_view_android_jni_impl.h index 73cb6a43cffdb..d6ac550db9d41 100644 --- a/shell/platform/android/platform_view_android_jni_impl.h +++ b/shell/platform/android/platform_view_android_jni_impl.h @@ -81,6 +81,8 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void FlutterViewEndFrame() override; + bool IsAttachedToView() override; + std::unique_ptr FlutterViewCreateOverlaySurface() override; diff --git a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java index 17fa0c07d843c..72df6e3f1bc13 100644 --- a/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java +++ b/testing/scenario_app/android/app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java @@ -28,7 +28,7 @@ public class MemoryLeakTests { public void platformViewHybridComposition_launchActivityFinishAndLaunchAgain() throws Exception { Intent intent = new Intent(Intent.ACTION_MAIN); intent.putExtra("scenario_name", "platform_view"); - // intent.putExtra("use_android_view", true); + intent.putExtra("use_android_view", true); intent.putExtra("view_type", PlatformViewsActivity.TEXT_VIEW_PV); activityRule.launchActivity(intent); From 6498528d68124bc4f2cd5d29bda710fb3ff10ea6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 9 Aug 2024 19:45:47 -0700 Subject: [PATCH 20/21] lets give this a go --- .../android/android_shell_holder_unittests.cc | 2 +- .../external_view_embedder.cc | 34 ++++++++++++------- .../external_view_embedder/surface_pool.cc | 16 +++------ .../external_view_embedder/surface_pool.h | 11 +++--- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/shell/platform/android/android_shell_holder_unittests.cc b/shell/platform/android/android_shell_holder_unittests.cc index 17bd459e92d25..c740be3d6d10d 100644 --- a/shell/platform/android/android_shell_holder_unittests.cc +++ b/shell/platform/android/android_shell_holder_unittests.cc @@ -81,7 +81,7 @@ TEST(AndroidShellHolder, CreateWithMergedPlatformAndUIThread) { TEST(AndroidShellHolder, CreateWithUnMergedPlatformAndUIThread) { Settings settings; settings.merged_platform_ui_thread = false; - auto jni = std::make_shared(); + auto jni = std::make_shared(); auto holder = std::make_unique(settings, jni); auto window = fml::MakeRefCounted( nullptr, /*is_fake_window=*/true); 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 9b12e6685336c..29d2a0b15a263 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -116,23 +116,32 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( // must be posted to the platform thread to construct them. bool destroy_all_layers = surface_pool_->CheckLayerProperties(context, frame_size_); + std::vector> + overlays; if (destroy_all_layers || surface_pool_->size() < overlay_layers.size()) { auto latch = std::make_shared(1u); fml::TaskRunner::RunNowOrPostTask( task_runners_.GetPlatformTaskRunner(), [&]() { if (destroy_all_layers) { - surface_pool_->DestroyLayers(jni_facade_); + jni_facade_->FlutterViewDestroyOverlaySurfaces(); } for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { - surface_pool_->CreateLayer(context, // - android_context_, // - jni_facade_, // - surface_factory_); // + overlays.push_back(jni_facade_->FlutterViewCreateOverlaySurface()); } }); if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { latch->Wait(); } + if (destroy_all_layers) { + surface_pool_->DestroyLayers(); + } + for (auto& overlay : overlays) { + surface_pool_->CreateLayer(context, // + android_context_, // + std::move(overlay), // + surface_factory_ // + ); + } } std::unordered_map> layers; @@ -280,12 +289,6 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::Teardown() { - // Post a platform task to ensure that the task runner has flushed all - // platform view composition. - auto latch = std::make_shared(1u); - fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(), - [&latch]() { latch->CountDown(); }); - latch->Wait(); DestroySurfaces(); } @@ -294,7 +297,14 @@ void AndroidExternalViewEmbedder::DestroySurfaces() { if (surface_pool_->size() == 0) { return; } - surface_pool_->DestroyLayers(jni_facade_); + surface_pool_->DestroyLayers(); + auto latch = std::make_shared(1u); + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetPlatformTaskRunner(), [&]() { + jni_facade_->FlutterViewDestroyOverlaySurfaces(); + latch->CountDown(); + }); + latch->Wait(); } } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 45a4ca7bd222d..0e358325e5e54 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -42,7 +42,7 @@ std::shared_ptr SurfacePool::GetNextLayer() { void SurfacePool::CreateLayer( GrDirectContext* gr_context, const AndroidContext& android_context, - const std::shared_ptr& jni_facade, + std::unique_ptr overlay_metadata, const std::shared_ptr& surface_factory) { std::unique_ptr android_surface = surface_factory->CreateSurface(); @@ -50,18 +50,14 @@ void SurfacePool::CreateLayer( FML_CHECK(android_surface && android_surface->IsValid()) << "Could not create an OpenGL, Vulkan or Software surface to set up " "rendering."; - - std::unique_ptr java_metadata = - jni_facade->FlutterViewCreateOverlaySurface(); - - FML_CHECK(java_metadata->window); - android_surface->SetNativeWindow(java_metadata->window); + FML_CHECK(overlay_metadata->window); + android_surface->SetNativeWindow(overlay_metadata->window); std::unique_ptr surface = android_surface->CreateGPUSurface(gr_context); std::shared_ptr layer = - std::make_shared(java_metadata->id, // + std::make_shared(overlay_metadata->id, // std::move(android_surface), // std::move(surface) // ); @@ -72,12 +68,10 @@ void SurfacePool::RecycleLayers() { available_layer_index_ = 0; } -void SurfacePool::DestroyLayers( - const std::shared_ptr& jni_facade) { +void SurfacePool::DestroyLayers() { if (layers_.empty()) { return; } - jni_facade->FlutterViewDestroyOverlaySurfaces(); layers_.clear(); available_layer_index_ = 0; } diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index 468e0283a45c0..c9e3bc99331a7 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -53,11 +53,11 @@ class SurfacePool { /// @brief Create a new overlay layer. /// - /// This method can only be called on the Platform thread. + /// This method can only be called on the Raster thread. void CreateLayer( GrDirectContext* gr_context, const AndroidContext& android_context, - const std::shared_ptr& jni_facade, + std::unique_ptr overlay_metadata, const std::shared_ptr& surface_factory); /// @brief Removes unused layers from the pool. Returns the unused layers. @@ -69,8 +69,11 @@ class SurfacePool { /// @brief The count of layers currently in the pool. size_t size() const; - /// @brief Destroys all the layers in the pool. - void DestroyLayers(const std::shared_ptr& jni_facade); + /// @brief Clears the state of the surface pool. + /// + /// Requires that the JNI method FlutterViewDestroyOverlaySurfaces is called + /// separately + void DestroyLayers(); private: // The index of the entry in the layers_ vector that determines the beginning From 95afe1174c00b03e60f6ccde26795c514683b9b3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 9 Aug 2024 20:24:19 -0700 Subject: [PATCH 21/21] oops countdown. --- .../android/external_view_embedder/external_view_embedder.cc | 1 + 1 file changed, 1 insertion(+) 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 29d2a0b15a263..010f3bc9b80ae 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -128,6 +128,7 @@ void AndroidExternalViewEmbedder::SubmitFlutterView( for (auto i = surface_pool_->size(); i < overlay_layers.size(); i++) { overlays.push_back(jni_facade_->FlutterViewCreateOverlaySurface()); } + latch->CountDown(); }); if (!task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()) { latch->Wait();