From 7b036073faf0e406de78b1663035b672b052ffa9 Mon Sep 17 00:00:00 2001 From: John McCutchan Date: Tue, 10 Oct 2023 11:12:05 -0700 Subject: [PATCH] Fix high FPS screen flicker of Platform Views when using ImageReaderPlatformViewRenderTarget The root bug was that we were holding onto HardwareBuffers after the owning Image was closed. This CL refactors the C++ code to properly hold a reference to the Image until it is safe to dispose of. This CL also refactors the Impeller GL and Skia GL code paths to share more code. --- .../hardware_buffer_external_texture.cc | 76 +++--- .../hardware_buffer_external_texture.h | 16 +- .../hardware_buffer_external_texture_gl.cc | 223 ++++++++++-------- .../hardware_buffer_external_texture_gl.h | 56 +++-- .../hardware_buffer_external_texture_vk.cc | 32 ++- .../hardware_buffer_external_texture_vk.h | 3 + .../engine/renderer/FlutterRenderer.java | 23 +- .../ImageReaderPlatformViewRenderTarget.java | 4 +- .../platform/PlatformViewsController.java | 14 +- .../platform/android/platform_view_android.cc | 4 +- 10 files changed, 273 insertions(+), 178 deletions(-) diff --git a/shell/platform/android/hardware_buffer_external_texture.cc b/shell/platform/android/hardware_buffer_external_texture.cc index 0b5d54a20785a..e26a7e51cd6dd 100644 --- a/shell/platform/android/hardware_buffer_external_texture.cc +++ b/shell/platform/android/hardware_buffer_external_texture.cc @@ -3,6 +3,8 @@ #include #include + +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "flutter/shell/platform/android/ndk_helpers.h" namespace flutter { @@ -23,6 +25,7 @@ void HardwareBufferExternalTexture::Paint(PaintContext& context, if (state_ == AttachmentState::kDetached) { return; } + Attach(context); const bool should_process_frame = (!freeze && new_frame_ready_) || dl_image_ == nullptr; if (should_process_frame) { @@ -39,7 +42,7 @@ void HardwareBufferExternalTexture::Paint(PaintContext& context, flutter::DlCanvas::SrcRectConstraint::kStrict // enforce edges ); } else { - FML_LOG(WARNING) + FML_LOG(ERROR) << "No DlImage available for HardwareBufferExternalTexture to paint."; } } @@ -57,53 +60,56 @@ void HardwareBufferExternalTexture::OnGrContextCreated() { state_ = AttachmentState::kUninitialized; } -AHardwareBuffer* HardwareBufferExternalTexture::GetLatestHardwareBuffer() { +// Implementing flutter::ContextListener. +void HardwareBufferExternalTexture::OnGrContextDestroyed() { + if (state_ == AttachmentState::kAttached) { + dl_image_.reset(); + Detach(); + } + state_ = AttachmentState::kDetached; +} + +JavaLocalRef HardwareBufferExternalTexture::AcquireLatestImage() { JNIEnv* env = fml::jni::AttachCurrentThread(); FML_CHECK(env != nullptr); // ImageTextureEntry.acquireLatestImage. JavaLocalRef image_java = jni_facade_->ImageTextureEntryAcquireLatestImage( JavaLocalRef(image_texture_entry_)); - if (image_java.obj() == nullptr) { - return nullptr; - } + return image_java; +} - // Image.getHardwareBuffer. - JavaLocalRef hardware_buffer_java = - jni_facade_->ImageGetHardwareBuffer(image_java); - if (hardware_buffer_java.obj() == nullptr) { - jni_facade_->ImageClose(image_java); - return nullptr; +void HardwareBufferExternalTexture::CloseImage( + const fml::jni::JavaRef& image) { + if (image.obj() == nullptr) { + return; } + jni_facade_->ImageClose(JavaLocalRef(image)); +} - // Convert into NDK HardwareBuffer. - AHardwareBuffer* latest_hardware_buffer = - NDKHelpers::AHardwareBuffer_fromHardwareBuffer( - env, hardware_buffer_java.obj()); - if (latest_hardware_buffer == nullptr) { - jni_facade_->HardwareBufferClose(hardware_buffer_java); - jni_facade_->ImageClose(image_java); - return nullptr; +void HardwareBufferExternalTexture::CloseHardwareBuffer( + const fml::jni::JavaRef& hardware_buffer) { + if (hardware_buffer.obj() == nullptr) { + return; } - - // Keep hardware buffer alive. - NDKHelpers::AHardwareBuffer_acquire(latest_hardware_buffer); - - // Now that we have referenced the native hardware buffer, close the Java - // Image and HardwareBuffer objects. - jni_facade_->HardwareBufferClose(hardware_buffer_java); - jni_facade_->ImageClose(image_java); - - return latest_hardware_buffer; + jni_facade_->HardwareBufferClose(JavaLocalRef(hardware_buffer)); } -// Implementing flutter::ContextListener. -void HardwareBufferExternalTexture::OnGrContextDestroyed() { - if (state_ == AttachmentState::kAttached) { - dl_image_.reset(); - Detach(); +JavaLocalRef HardwareBufferExternalTexture::HardwareBufferFor( + const fml::jni::JavaRef& image) { + if (image.obj() == nullptr) { + return JavaLocalRef(); } - state_ = AttachmentState::kDetached; + // Image.getHardwareBuffer. + return jni_facade_->ImageGetHardwareBuffer(JavaLocalRef(image)); +} + +AHardwareBuffer* HardwareBufferExternalTexture::AHardwareBufferFor( + const fml::jni::JavaRef& hardware_buffer) { + JNIEnv* env = fml::jni::AttachCurrentThread(); + FML_CHECK(env != nullptr); + return NDKHelpers::AHardwareBuffer_fromHardwareBuffer(env, + hardware_buffer.obj()); } } // namespace flutter diff --git a/shell/platform/android/hardware_buffer_external_texture.h b/shell/platform/android/hardware_buffer_external_texture.h index 5f93923fb9fa1..6a0ec78977e1a 100644 --- a/shell/platform/android/hardware_buffer_external_texture.h +++ b/shell/platform/android/hardware_buffer_external_texture.h @@ -7,6 +7,7 @@ #include "flutter/common/graphics/texture.h" #include "flutter/fml/logging.h" +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "flutter/shell/platform/android/platform_view_android_jni_impl.h" #include @@ -20,10 +21,11 @@ class HardwareBufferExternalTexture : public flutter::Texture { public: explicit HardwareBufferExternalTexture( int64_t id, - const fml::jni::ScopedJavaGlobalRef& - hardware_buffer_texture_entry, + const fml::jni::ScopedJavaGlobalRef& image_texture_entry, const std::shared_ptr& jni_facade); + virtual ~HardwareBufferExternalTexture() = default; + // |flutter::Texture|. void Paint(PaintContext& context, const SkRect& bounds, @@ -43,10 +45,16 @@ class HardwareBufferExternalTexture : public flutter::Texture { void OnGrContextDestroyed() override; protected: - virtual void ProcessFrame(PaintContext& context, const SkRect& bounds) = 0; + virtual void Attach(PaintContext& context) = 0; virtual void Detach() = 0; + virtual void ProcessFrame(PaintContext& context, const SkRect& bounds) = 0; - AHardwareBuffer* GetLatestHardwareBuffer(); + JavaLocalRef AcquireLatestImage(); + void CloseImage(const fml::jni::JavaRef& image); + JavaLocalRef HardwareBufferFor(const fml::jni::JavaRef& image); + void CloseHardwareBuffer(const fml::jni::JavaRef& hardware_buffer); + AHardwareBuffer* AHardwareBufferFor( + const fml::jni::JavaRef& hardware_buffer); fml::jni::ScopedJavaGlobalRef image_texture_entry_; std::shared_ptr jni_facade_; diff --git a/shell/platform/android/hardware_buffer_external_texture_gl.cc b/shell/platform/android/hardware_buffer_external_texture_gl.cc index 2dfe278433d08..3eb0defe19d98 100644 --- a/shell/platform/android/hardware_buffer_external_texture_gl.cc +++ b/shell/platform/android/hardware_buffer_external_texture_gl.cc @@ -6,15 +6,16 @@ #include #include -#include "flutter/common/graphics/texture.h" -#include "flutter/shell/platform/android/ndk_helpers.h" -#include "impeller/core/formats.h" -#include "impeller/display_list/dl_image_impeller.h" -#include "impeller/renderer/backend/gles/texture_gles.h" -#include "impeller/toolkit/egl/image.h" -#include "impeller/toolkit/gles/texture.h" +#include "flutter/common/graphics/texture.h" #include "flutter/display_list/effects/dl_color_source.h" +#include "flutter/flow/layers/layer.h" +#include "flutter/impeller/core/formats.h" +#include "flutter/impeller/display_list/dl_image_impeller.h" +#include "flutter/impeller/renderer/backend/gles/texture_gles.h" +#include "flutter/impeller/toolkit/egl/image.h" +#include "flutter/impeller/toolkit/gles/texture.h" +#include "flutter/shell/platform/android/ndk_helpers.h" #include "third_party/skia/include/core/SkAlphaType.h" #include "third_party/skia/include/core/SkColorSpace.h" #include "third_party/skia/include/core/SkColorType.h" @@ -27,122 +28,164 @@ namespace flutter { +HardwareBufferExternalTextureGL::HardwareBufferExternalTextureGL( + int64_t id, + const fml::jni::ScopedJavaGlobalRef& image_texture_entry, + const std::shared_ptr& jni_facade) + : HardwareBufferExternalTexture(id, image_texture_entry, jni_facade) {} + +void HardwareBufferExternalTextureGL::Attach(PaintContext& context) { + if (state_ == AttachmentState::kUninitialized) { + if (!android_image_.is_null()) { + JavaLocalRef hardware_buffer = HardwareBufferFor(android_image_); + AHardwareBuffer* hardware_buffer_ahw = + AHardwareBufferFor(hardware_buffer); + egl_image_ = CreateEGLImage(hardware_buffer_ahw); + CloseHardwareBuffer(hardware_buffer); + } + state_ = AttachmentState::kAttached; + } +} + void HardwareBufferExternalTextureGL::Detach() { - image_.reset(); - texture_.reset(); + egl_image_.reset(); +} + +bool HardwareBufferExternalTextureGL::MaybeSwapImages() { + JavaLocalRef image = AcquireLatestImage(); + if (image.is_null()) { + return false; + } + // NOTE: In the following code it is important that old_android_image is + // not closed until after the update of egl_image_ otherwise the image might + // be closed before the old EGLImage referencing it has been deleted. After + // an image is closed the underlying HardwareBuffer may be recycled and used + // for a future frame. + JavaLocalRef old_android_image(android_image_); + android_image_.Reset(image); + JavaLocalRef hardware_buffer = HardwareBufferFor(image); + egl_image_ = CreateEGLImage(AHardwareBufferFor(hardware_buffer)); + CloseHardwareBuffer(hardware_buffer); + // IMPORTANT: We only close the old image after egl_image_ stops referencing + // it. + CloseImage(old_android_image); + return true; +} + +impeller::UniqueEGLImageKHR HardwareBufferExternalTextureGL::CreateEGLImage( + AHardwareBuffer* hardware_buffer) { + if (hardware_buffer == nullptr) { + return impeller::UniqueEGLImageKHR(); + } + + EGLDisplay display = eglGetCurrentDisplay(); + FML_CHECK(display != EGL_NO_DISPLAY); + + EGLClientBuffer client_buffer = + NDKHelpers::eglGetNativeClientBufferANDROID(hardware_buffer); + FML_DCHECK(client_buffer != nullptr); + if (client_buffer == nullptr) { + FML_LOG(ERROR) << "eglGetNativeClientBufferAndroid returned null."; + return impeller::UniqueEGLImageKHR(); + } + + impeller::EGLImageKHRWithDisplay maybe_image = + impeller::EGLImageKHRWithDisplay{ + eglCreateImageKHR(display, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, + client_buffer, 0), + display}; + + return impeller::UniqueEGLImageKHR(maybe_image); } -void HardwareBufferExternalTextureGL::ProcessFrame(PaintContext& context, - const SkRect& bounds) { +HardwareBufferExternalTextureGLSkia::HardwareBufferExternalTextureGLSkia( + const std::shared_ptr& context, + int64_t id, + const fml::jni::ScopedJavaGlobalRef& image_texture_entry, + const std::shared_ptr& jni_facade) + : HardwareBufferExternalTextureGL(id, image_texture_entry, jni_facade) {} + +void HardwareBufferExternalTextureGLSkia::Attach(PaintContext& context) { if (state_ == AttachmentState::kUninitialized) { + // After this call state_ will be AttachmentState::kAttached and egl_image_ + // will have been created if we still have an Image associated with us. + HardwareBufferExternalTextureGL::Attach(context); GLuint texture_name; glGenTextures(1, &texture_name); texture_.reset(impeller::GLTexture{texture_name}); - state_ = AttachmentState::kAttached; } - glBindTexture(GL_TEXTURE_EXTERNAL_OES, texture_.get().texture_name); - - EGLDisplay display = eglGetCurrentDisplay(); - FML_CHECK(display != EGL_NO_DISPLAY); +} - image_.reset(); +void HardwareBufferExternalTextureGLSkia::Detach() { + HardwareBufferExternalTextureGL::Detach(); + texture_.reset(); +} - AHardwareBuffer* latest_hardware_buffer = GetLatestHardwareBuffer(); - if (latest_hardware_buffer == nullptr) { - FML_LOG(WARNING) << "GetLatestHardwareBuffer returned null."; +void HardwareBufferExternalTextureGLSkia::ProcessFrame(PaintContext& context, + const SkRect& bounds) { + const bool swapped = MaybeSwapImages(); + if (!swapped && !egl_image_.is_valid()) { + // Nothing to do. return; } + BindImageToTexture(egl_image_, texture_.get().texture_name); + dl_image_ = CreateDlImage(context, bounds); +} - EGLClientBuffer client_buffer = - NDKHelpers::eglGetNativeClientBufferANDROID(latest_hardware_buffer); - if (client_buffer == nullptr) { - FML_LOG(WARNING) << "eglGetNativeClientBufferAndroid returned null."; - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); +void HardwareBufferExternalTextureGLSkia::BindImageToTexture( + const impeller::UniqueEGLImageKHR& image, + GLuint tex) { + if (!image.is_valid() || tex == 0) { return; } - FML_CHECK(client_buffer != nullptr); - image_.reset(impeller::EGLImageKHRWithDisplay{ - eglCreateImageKHR(display, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, - client_buffer, 0), - display}); - FML_CHECK(image_.get().image != EGL_NO_IMAGE_KHR); + glBindTexture(GL_TEXTURE_EXTERNAL_OES, tex); glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES, - (GLeglImageOES)image_.get().image); - - // Drop our temporary reference to the hardware buffer as the call to - // eglCreateImageKHR now has the reference. - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); + (GLeglImageOES)image.get().image); +} +sk_sp HardwareBufferExternalTextureGLSkia::CreateDlImage( + PaintContext& context, + const SkRect& bounds) { GrGLTextureInfo textureInfo = {GL_TEXTURE_EXTERNAL_OES, texture_.get().texture_name, GL_RGBA8_OES}; auto backendTexture = GrBackendTextures::MakeGL(1, 1, skgpu::Mipmapped::kNo, textureInfo); - dl_image_ = DlImage::Make(SkImages::BorrowTextureFrom( + return DlImage::Make(SkImages::BorrowTextureFrom( context.gr_context, backendTexture, kTopLeft_GrSurfaceOrigin, kRGBA_8888_SkColorType, kPremul_SkAlphaType, nullptr)); } -HardwareBufferExternalTextureGL::HardwareBufferExternalTextureGL( - const std::shared_ptr& context, - int64_t id, - const fml::jni::ScopedJavaGlobalRef& image_texture_entry, - const std::shared_ptr& jni_facade) - : HardwareBufferExternalTexture(id, image_texture_entry, jni_facade) {} - -HardwareBufferExternalTextureGL::~HardwareBufferExternalTextureGL() {} - -HardwareBufferExternalTextureImpellerGL:: - HardwareBufferExternalTextureImpellerGL( +HardwareBufferExternalTextureGLImpeller:: + HardwareBufferExternalTextureGLImpeller( const std::shared_ptr& context, int64_t id, - const fml::jni::ScopedJavaGlobalRef& - hardware_buffer_texture_entry, + const fml::jni::ScopedJavaGlobalRef& image_textury_entry, const std::shared_ptr& jni_facade) - : HardwareBufferExternalTexture(id, - hardware_buffer_texture_entry, - jni_facade), + : HardwareBufferExternalTextureGL(id, image_textury_entry, jni_facade), impeller_context_(context) {} -HardwareBufferExternalTextureImpellerGL:: - ~HardwareBufferExternalTextureImpellerGL() {} +void HardwareBufferExternalTextureGLImpeller::Detach() {} -void HardwareBufferExternalTextureImpellerGL::Detach() { - egl_image_.reset(); +void HardwareBufferExternalTextureGLImpeller::Attach(PaintContext& context) { + if (state_ == AttachmentState::kUninitialized) { + HardwareBufferExternalTextureGL::Attach(context); + } } -void HardwareBufferExternalTextureImpellerGL::ProcessFrame( +void HardwareBufferExternalTextureGLImpeller::ProcessFrame( PaintContext& context, const SkRect& bounds) { - EGLDisplay display = eglGetCurrentDisplay(); - FML_CHECK(display != EGL_NO_DISPLAY); - - if (state_ == AttachmentState::kUninitialized) { - // First processed frame we are attached. - state_ = AttachmentState::kAttached; - } - - AHardwareBuffer* latest_hardware_buffer = GetLatestHardwareBuffer(); - if (latest_hardware_buffer == nullptr) { - FML_LOG(ERROR) << "GetLatestHardwareBuffer returned null."; - return; - } - - EGLClientBuffer client_buffer = - NDKHelpers::eglGetNativeClientBufferANDROID(latest_hardware_buffer); - if (client_buffer == nullptr) { - FML_LOG(ERROR) << "eglGetNativeClientBufferAndroid returned null."; - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); + const bool swapped = MaybeSwapImages(); + if (!swapped && !egl_image_.is_valid()) { + // Nothing to do. return; } + dl_image_ = CreateDlImage(context, bounds); +} - FML_CHECK(client_buffer != nullptr); - egl_image_.reset(impeller::EGLImageKHRWithDisplay{ - eglCreateImageKHR(display, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, - client_buffer, 0), - display}); - FML_CHECK(egl_image_.get().image != EGL_NO_IMAGE_KHR); - - // Create the texture. +sk_sp HardwareBufferExternalTextureGLImpeller::CreateDlImage( + PaintContext& context, + const SkRect& bounds) { impeller::TextureDescriptor desc; desc.type = impeller::TextureType::kTextureExternalOES; desc.storage_mode = impeller::StorageMode::kDevicePrivate; @@ -156,18 +199,12 @@ void HardwareBufferExternalTextureImpellerGL::ProcessFrame( texture->SetCoordinateSystem( impeller::TextureCoordinateSystem::kUploadFromHost); if (!texture->Bind()) { - FML_LOG(ERROR) << "Could not bind texture."; - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); - return; + return nullptr; } // Associate the hardware buffer image with the texture. glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES, (GLeglImageOES)egl_image_.get().image); - - dl_image_ = impeller::DlImageImpeller::Make(texture); - - // Release the reference acquired by GetLatestHardwareBuffer. - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); + return impeller::DlImageImpeller::Make(texture); } } // namespace flutter diff --git a/shell/platform/android/hardware_buffer_external_texture_gl.h b/shell/platform/android/hardware_buffer_external_texture_gl.h index c8e05e0c750e7..0a81fde0d1321 100644 --- a/shell/platform/android/hardware_buffer_external_texture_gl.h +++ b/shell/platform/android/hardware_buffer_external_texture_gl.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_HARDWARE_BUFFER_EXTERNAL_TEXTURE_GL_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_HARDWARE_BUFFER_EXTERNAL_TEXTURE_GL_H_ +#include "flutter/fml/platform/android/scoped_java_ref.h" #include "flutter/shell/platform/android/hardware_buffer_external_texture.h" #include "flutter/impeller/renderer/backend/gles/context_gles.h" @@ -15,51 +16,76 @@ #include "flutter/impeller/toolkit/gles/texture.h" #include "flutter/shell/platform/android/android_context_gl_skia.h" +#include "flutter/shell/platform/android/ndk_helpers.h" namespace flutter { class HardwareBufferExternalTextureGL : public HardwareBufferExternalTexture { public: HardwareBufferExternalTextureGL( - const std::shared_ptr& context, int64_t id, - const fml::jni::ScopedJavaGlobalRef& - hardware_buffer_texture_entry, + const fml::jni::ScopedJavaGlobalRef& image_textury_entry, const std::shared_ptr& jni_facade); - ~HardwareBufferExternalTextureGL() override; + protected: + void Attach(PaintContext& context) override; + void Detach() override; + + // Returns true if a new image was acquired and android_image_ and egl_image_ + // were updated. + bool MaybeSwapImages(); + impeller::UniqueEGLImageKHR CreateEGLImage(AHardwareBuffer* buffer); + + fml::jni::ScopedJavaGlobalRef android_image_; + impeller::UniqueEGLImageKHR egl_image_; + + FML_DISALLOW_COPY_AND_ASSIGN(HardwareBufferExternalTextureGL); +}; + +class HardwareBufferExternalTextureGLSkia + : public HardwareBufferExternalTextureGL { + public: + HardwareBufferExternalTextureGLSkia( + const std::shared_ptr& context, + int64_t id, + const fml::jni::ScopedJavaGlobalRef& image_textury_entry, + const std::shared_ptr& jni_facade); private: - void ProcessFrame(PaintContext& context, const SkRect& bounds) override; + void Attach(PaintContext& context) override; void Detach() override; + void ProcessFrame(PaintContext& context, const SkRect& bounds) override; + + void BindImageToTexture(const impeller::UniqueEGLImageKHR& image, GLuint tex); + sk_sp CreateDlImage(PaintContext& context, + const SkRect& bounds); - impeller::UniqueEGLImageKHR image_; impeller::UniqueGLTexture texture_; - FML_DISALLOW_COPY_AND_ASSIGN(HardwareBufferExternalTextureGL); + FML_DISALLOW_COPY_AND_ASSIGN(HardwareBufferExternalTextureGLSkia); }; -class HardwareBufferExternalTextureImpellerGL - : public HardwareBufferExternalTexture { +class HardwareBufferExternalTextureGLImpeller + : public HardwareBufferExternalTextureGL { public: - HardwareBufferExternalTextureImpellerGL( + HardwareBufferExternalTextureGLImpeller( const std::shared_ptr& context, int64_t id, const fml::jni::ScopedJavaGlobalRef& hardware_buffer_texture_entry, const std::shared_ptr& jni_facade); - ~HardwareBufferExternalTextureImpellerGL() override; - private: + void Attach(PaintContext& context) override; void ProcessFrame(PaintContext& context, const SkRect& bounds) override; void Detach() override; - const std::shared_ptr impeller_context_; + sk_sp CreateDlImage(PaintContext& context, + const SkRect& bounds); - impeller::UniqueEGLImageKHR egl_image_; + const std::shared_ptr impeller_context_; - FML_DISALLOW_COPY_AND_ASSIGN(HardwareBufferExternalTextureImpellerGL); + FML_DISALLOW_COPY_AND_ASSIGN(HardwareBufferExternalTextureGLImpeller); }; } // namespace flutter diff --git a/shell/platform/android/hardware_buffer_external_texture_vk.cc b/shell/platform/android/hardware_buffer_external_texture_vk.cc index 9ba52e43e7547..d02a4f545af97 100644 --- a/shell/platform/android/hardware_buffer_external_texture_vk.cc +++ b/shell/platform/android/hardware_buffer_external_texture_vk.cc @@ -1,12 +1,12 @@ #include "flutter/shell/platform/android/hardware_buffer_external_texture_vk.h" +#include "flutter/impeller/core/formats.h" +#include "flutter/impeller/core/texture_descriptor.h" +#include "flutter/impeller/display_list/dl_image_impeller.h" #include "flutter/impeller/renderer/backend/vulkan/android_hardware_buffer_texture_source_vk.h" #include "flutter/impeller/renderer/backend/vulkan/texture_vk.h" #include "flutter/shell/platform/android/ndk_helpers.h" -#include "impeller/core/formats.h" -#include "impeller/core/texture_descriptor.h" -#include "impeller/display_list/dl_image_impeller.h" namespace flutter { @@ -20,18 +20,25 @@ HardwareBufferExternalTextureVK::HardwareBufferExternalTextureVK( HardwareBufferExternalTextureVK::~HardwareBufferExternalTextureVK() {} -void HardwareBufferExternalTextureVK::ProcessFrame(PaintContext& context, - const SkRect& bounds) { +void HardwareBufferExternalTextureVK::Attach(PaintContext& context) { if (state_ == AttachmentState::kUninitialized) { // First processed frame we are attached. state_ = AttachmentState::kAttached; } +} + +void HardwareBufferExternalTextureVK::Detach() {} - AHardwareBuffer* latest_hardware_buffer = GetLatestHardwareBuffer(); - if (latest_hardware_buffer == nullptr) { - FML_LOG(WARNING) << "GetLatestHardwareBuffer returned null."; +void HardwareBufferExternalTextureVK::ProcessFrame(PaintContext& context, + const SkRect& bounds) { + JavaLocalRef image = AcquireLatestImage(); + if (image.is_null()) { return; } + JavaLocalRef old_android_image(android_image_); + android_image_.Reset(image); + JavaLocalRef hardware_buffer = HardwareBufferFor(android_image_); + AHardwareBuffer* latest_hardware_buffer = AHardwareBufferFor(hardware_buffer); AHardwareBuffer_Desc hb_desc = {}; flutter::NDKHelpers::AHardwareBuffer_describe(latest_hardware_buffer, @@ -54,11 +61,10 @@ void HardwareBufferExternalTextureVK::ProcessFrame(PaintContext& context, std::make_shared(impeller_context_, texture_source); dl_image_ = impeller::DlImageImpeller::Make(texture); - - // GetLatestHardwareBuffer keeps a reference on the hardware buffer, drop it. - NDKHelpers::AHardwareBuffer_release(latest_hardware_buffer); + CloseHardwareBuffer(hardware_buffer); + // IMPORTANT: We only close the old image after texture stops referencing + // it. + CloseImage(old_android_image); } -void HardwareBufferExternalTextureVK::Detach() {} - } // namespace flutter diff --git a/shell/platform/android/hardware_buffer_external_texture_vk.h b/shell/platform/android/hardware_buffer_external_texture_vk.h index 76c2779eff28d..f4cd12a1ffb00 100644 --- a/shell/platform/android/hardware_buffer_external_texture_vk.h +++ b/shell/platform/android/hardware_buffer_external_texture_vk.h @@ -26,10 +26,13 @@ class HardwareBufferExternalTextureVK : public HardwareBufferExternalTexture { ~HardwareBufferExternalTextureVK() override; private: + void Attach(PaintContext& context) override; void ProcessFrame(PaintContext& context, const SkRect& bounds) override; void Detach() override; const std::shared_ptr impeller_context_; + + fml::jni::ScopedJavaGlobalRef android_image_; }; } // namespace flutter diff --git a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 1ab9699c3fe2d..e71e36488169d 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -19,7 +19,6 @@ import androidx.annotation.VisibleForTesting; import io.flutter.Log; import io.flutter.embedding.engine.FlutterJNI; -import io.flutter.embedding.engine.renderer.FlutterRenderer.ImageTextureRegistryEntry; import io.flutter.view.TextureRegistry; import io.flutter.view.TextureRegistry.ImageTextureEntry; import java.io.IOException; @@ -372,19 +371,23 @@ public void release() { @Override @TargetApi(19) public void pushImage(Image image) { + if (released) { + return; + } Image toClose; synchronized (this) { toClose = this.image; this.image = image; - if (image != null) { - // Mark that we have a new frame available. - markTextureFrameAvailable(id); - } } // Close the previously pushed buffer. if (toClose != null) { + Log.e(TAG, "Dropping PlatformView Frame"); toClose.close(); } + if (image != null) { + // Mark that we have a new frame available. + markTextureFrameAvailable(id); + } } @Override @@ -398,12 +401,14 @@ public Image acquireLatestImage() { if (r != null) { try { SyncFence fence = r.getFence(); - boolean signaled = fence.awaitForever(); - if (!signaled) { - Log.e(TAG, "acquireLatestImage failed to wait on image fence."); + if (fence.getSignalTime() == SyncFence.SIGNAL_TIME_PENDING) { + boolean signaled = fence.awaitForever(); + if (!signaled) { + Log.e(TAG, "acquireLatestImage image's fence was never signalled."); + } } } catch (IOException e) { - Log.e(TAG, "acquireLatestImage failed calling Image.getFence: " + e); + // Drop. } } return r; diff --git a/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java b/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java index 60b2d73e70bbd..79690179a8189 100644 --- a/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java +++ b/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java @@ -19,7 +19,7 @@ public class ImageReaderPlatformViewRenderTarget implements PlatformViewRenderTa private int bufferWidth = 0; private int bufferHeight = 0; private static final String TAG = "ImageReaderPlatformViewRenderTarget"; - private static final int MAX_IMAGES = 3; + private static final int MAX_IMAGES = 4; private void closeReader() { if (this.reader != null) { @@ -40,7 +40,7 @@ public void onImageAvailable(ImageReader reader) { try { image = reader.acquireLatestImage(); } catch (IllegalStateException e) { - Log.e(TAG, "New image available but it could not be acquired: " + e.toString()); + Log.e(TAG, "onImageAvailable acquireLatestImage failed: " + e.toString()); } if (image == null) { return; diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index d5a1621d1e21f..820a8629b4350 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -147,7 +147,7 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega // Whether software rendering is used. private boolean usesSoftwareRendering = false; - private static boolean enableHardwareBufferRenderingTarget = false; + private static boolean enableHardwareBufferRenderingTarget = true; private final PlatformViewsChannel.PlatformViewsHandler channelHandler = new PlatformViewsChannel.PlatformViewsHandler() { @@ -181,12 +181,14 @@ public long createForTextureLayer( } if (textureRegistry == null) { throw new IllegalStateException( - "Texture registry is null. This means that platform views controller was detached, view id: " + "Texture registry is null. This means that platform views controller was detached," + + " view id: " + viewId); } if (flutterView == null) { throw new IllegalStateException( - "Flutter view is null. This means the platform views controller doesn't have an attached view, view id: " + "Flutter view is null. This means the platform views controller doesn't have an" + + " attached view, view id: " + viewId); } @@ -195,7 +197,8 @@ public long createForTextureLayer( final View embeddedView = platformView.getView(); if (embeddedView.getParent() != null) { throw new IllegalStateException( - "The Android view returned from PlatformView#getView() was already added to a parent view."); + "The Android view returned from PlatformView#getView() was already added to a" + + " parent view."); } // The newer Texture Layer Hybrid Composition mode isn't suppported if any of the @@ -1098,7 +1101,8 @@ void initializePlatformViewIfNeeded(int viewId) { } if (embeddedView.getParent() != null) { throw new IllegalStateException( - "The Android view returned from PlatformView#getView() was already added to a parent view."); + "The Android view returned from PlatformView#getView() was already added to a parent" + + " view."); } final FlutterMutatorView parentView = new FlutterMutatorView( diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index ac6111c343ada..434c44db634a1 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -324,13 +324,13 @@ void PlatformViewAndroid::RegisterImageTexture( if (android_context_->RenderingApi() == AndroidRenderingAPI::kOpenGLES) { if (android_context_->GetImpellerContext()) { // Impeller GLES. - RegisterTexture(std::make_shared( + RegisterTexture(std::make_shared( std::static_pointer_cast( android_context_->GetImpellerContext()), texture_id, image_texture_entry, jni_facade_)); } else { // Legacy GL. - RegisterTexture(std::make_shared( + RegisterTexture(std::make_shared( std::static_pointer_cast(android_context_), texture_id, image_texture_entry, jni_facade_)); }