From 1a7eee1587e92818fd870e49ec60f245a1f39de1 Mon Sep 17 00:00:00 2001 From: John McCutchan Date: Wed, 16 Aug 2023 17:30:48 -0700 Subject: [PATCH] Reenable HardwareBuffer backed Android Platform Views on SDK >= 29 - Fix a bug in the SDK < 33 ImageReader construction code path. - Fix a bug that resulted in references to Images produced by a closed ImageReader. - Fix an order of operations bug in ImageReaderPlatformViewRenderTarget release/finalizer code path. - Enable HardwareBuffer backed Android Platform Views on SDK >= 29 --- .../hardware_buffer_external_texture.cc | 3 +- .../engine/renderer/FlutterRenderer.java | 7 ++- .../ImageReaderPlatformViewRenderTarget.java | 51 +++++++++---------- .../platform/PlatformViewsController.java | 3 +- .../surface_texture_external_texture.cc | 3 +- 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/shell/platform/android/hardware_buffer_external_texture.cc b/shell/platform/android/hardware_buffer_external_texture.cc index 0b1319d7458de..13d76e8d871ea 100644 --- a/shell/platform/android/hardware_buffer_external_texture.cc +++ b/shell/platform/android/hardware_buffer_external_texture.cc @@ -39,7 +39,8 @@ void HardwareBufferExternalTexture::Paint(PaintContext& context, flutter::DlCanvas::SrcRectConstraint::kStrict // enforce edges ); } else { - FML_LOG(WARNING) << "No DlImage available."; + FML_LOG(WARNING) + << "No DlImage available for HardwareBufferExternalTexture to paint."; } } 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 270e51a28a1ad..f9ea32852171c 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -322,6 +322,7 @@ public void run() { @Keep final class ImageTextureRegistryEntry implements TextureRegistry.ImageTextureEntry { + private static final String TAG = "ImageTextureRegistryEntry"; private final long id; private boolean released; private Image image; @@ -356,8 +357,10 @@ public void pushImage(Image image) { if (toClose != null) { toClose.close(); } - // Mark that we have a new frame available. - markTextureFrameAvailable(id); + if (image != null) { + // Mark that we have a new frame available. + markTextureFrameAvailable(id); + } } @Override diff --git a/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java b/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java index 9dc93b0176d68..655f7ee3bcc49 100644 --- a/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java +++ b/shell/platform/android/io/flutter/plugin/platform/ImageReaderPlatformViewRenderTarget.java @@ -11,10 +11,9 @@ import android.view.Surface; import io.flutter.view.TextureRegistry.ImageTextureEntry; -@TargetApi(26) +@TargetApi(29) public class ImageReaderPlatformViewRenderTarget implements PlatformViewRenderTarget { private ImageTextureEntry textureEntry; - private boolean mustRecreateImageReader = false; private ImageReader reader; private int bufferWidth = 0; private int bufferHeight = 0; @@ -22,20 +21,14 @@ public class ImageReaderPlatformViewRenderTarget implements PlatformViewRenderTa private void closeReader() { if (this.reader != null) { + // Push a null image, forcing the texture entry to close any cached images. + textureEntry.pushImage(null); + // Close the reader, which also closes any produced images. this.reader.close(); this.reader = null; } } - private void recreateImageReaderIfNeeded() { - if (!mustRecreateImageReader) { - return; - } - mustRecreateImageReader = false; - closeReader(); - this.reader = createImageReader(); - } - private final Handler onImageAvailableHandler = new Handler(); private final ImageReader.OnImageAvailableListener onImageAvailableListener = new ImageReader.OnImageAvailableListener() { @@ -55,9 +48,12 @@ protected ImageReader createImageReader33() { // Allow for double buffering. builder.setMaxImages(3); // Use PRIVATE image format so that we can support video decoding. - // TODO(johnmccutchan): Should we always use PRIVATE here? It may impact our ability - // to read back texture data. If we don't always want to use it, how do we decide when - // to use it or not? Perhaps PlatformViews can indicate if they may contain DRM'd content. + // TODO(johnmccutchan): Should we always use PRIVATE here? It may impact our + // ability to read back texture data. If we don't always want to use it, how do we + // decide when to use it or not? Perhaps PlatformViews can indicate if they may contain + // DRM'd content. + // I need to investigate how PRIVATE impacts our ability to take screenshots or capture + // the output of Flutter application. builder.setImageFormat(ImageFormat.PRIVATE); // Hint that consumed images will only be read by GPU. builder.setUsage(HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); @@ -68,13 +64,15 @@ protected ImageReader createImageReader33() { @TargetApi(29) protected ImageReader createImageReader29() { - return ImageReader.newInstance( - bufferWidth, bufferHeight, ImageFormat.PRIVATE, 2, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); - } - - @TargetApi(26) - protected ImageReader createImageReader26() { - return ImageReader.newInstance(bufferWidth, bufferHeight, ImageFormat.PRIVATE, 2); + final ImageReader reader = + ImageReader.newInstance( + bufferWidth, + bufferHeight, + ImageFormat.PRIVATE, + 2, + HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); + reader.setOnImageAvailableListener(this.onImageAvailableListener, onImageAvailableHandler); + return reader; } protected ImageReader createImageReader() { @@ -82,15 +80,15 @@ protected ImageReader createImageReader() { return createImageReader33(); } else if (Build.VERSION.SDK_INT >= 29) { return createImageReader29(); - } else { - return createImageReader26(); } + throw new UnsupportedOperationException( + "ImageReaderPlatformViewRenderTarget requires API version 29+"); } public ImageReaderPlatformViewRenderTarget(ImageTextureEntry textureEntry) { - if (Build.VERSION.SDK_INT < 26) { + if (Build.VERSION.SDK_INT < 29) { throw new UnsupportedOperationException( - "ImageReaderPlatformViewRenderTarget requires API version 26+"); + "ImageReaderPlatformViewRenderTarget requires API version 29+"); } this.textureEntry = textureEntry; } @@ -127,9 +125,9 @@ public long getId() { } public void release() { + closeReader(); // textureEntry has a finalizer attached. textureEntry = null; - closeReader(); } public boolean isReleased() { @@ -137,7 +135,6 @@ public boolean isReleased() { } public Surface getSurface() { - recreateImageReaderIfNeeded(); return this.reader.getSurface(); } } diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index f9500cdf00c20..b783128900e2c 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -968,8 +968,7 @@ private void unlockInputConnection(@NonNull VirtualDisplayController controller) private static PlatformViewRenderTarget makePlatformViewRenderTarget( TextureRegistry textureRegistry) { - // TODO(johnmccutchan): Enable ImageReaderPlatformViewRenderTarget for public use. - if (false) { + if (Build.VERSION.SDK_INT >= 29) { final TextureRegistry.ImageTextureEntry textureEntry = textureRegistry.createImageTexture(); return new ImageReaderPlatformViewRenderTarget(textureEntry); } diff --git a/shell/platform/android/surface_texture_external_texture.cc b/shell/platform/android/surface_texture_external_texture.cc index 39d5b034d3390..a6758a8d0328c 100644 --- a/shell/platform/android/surface_texture_external_texture.cc +++ b/shell/platform/android/surface_texture_external_texture.cc @@ -65,7 +65,8 @@ void SurfaceTextureExternalTexture::Paint(PaintContext& context, flutter::DlCanvas::SrcRectConstraint::kStrict // enforce edges ); } else { - FML_LOG(WARNING) << "No DlImage available."; + FML_LOG(WARNING) + << "No DlImage available for SurfaceTextureExternalTexture to paint."; } }