From c46862404a9e9488805e82326f4b3f795ef07c80 Mon Sep 17 00:00:00 2001 From: John McCutchan Date: Mon, 18 Dec 2023 13:00:43 -0800 Subject: [PATCH] Use new SurfaceProducer external texture class for rendering platform views - Fix lots of bugs in the implementation of ImageReaderSurfaceProducer - Add test that we drop frames produced from the wrong size - Hookup platform views to use new external texture class --- ci/licenses_golden/licenses_flutter | 1 + shell/platform/android/BUILD.gn | 1 + .../engine/renderer/FlutterRenderer.java | 153 +++++++++++------- .../platform/PlatformViewsController.java | 11 +- ...rfaceProducerPlatformViewRenderTarget.java | 67 ++++++++ .../engine/renderer/FlutterRendererTest.java | 32 +++- 6 files changed, 202 insertions(+), 63 deletions(-) create mode 100644 shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 268a248e7585b..9a1de1da99d92 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -9271,6 +9271,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/Platfor FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsAccessibilityDelegate.java FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceTexturePlatformViewRenderTarget.java FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/VirtualDisplayController.java FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/text/ProcessTextPlugin.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index d0b0c48c39b83..e0ce4c8b4ee39 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -323,6 +323,7 @@ android_java_sources = [ "io/flutter/plugin/platform/PlatformViewsAccessibilityDelegate.java", "io/flutter/plugin/platform/PlatformViewsController.java", "io/flutter/plugin/platform/SingleViewPresentation.java", + "io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java", "io/flutter/plugin/platform/SurfaceTexturePlatformViewRenderTarget.java", "io/flutter/plugin/platform/VirtualDisplayController.java", "io/flutter/plugin/text/ProcessTextPlugin.java", 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 aed82bbede90a..34ea34086e8af 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -166,7 +166,7 @@ public SurfaceProducer createSurfaceProducer() { final ImageReaderSurfaceProducer entry = new ImageReaderSurfaceProducer(nextTextureId.getAndIncrement()); Log.v(TAG, "New SurfaceProducer ID: " + entry.id()); - registerImageTexture(isRenderingToImageViewCount, (TextureRegistry.ImageConsumer) entry); + registerImageTexture(entry.id(), (TextureRegistry.ImageConsumer) entry); return entry; } @@ -371,9 +371,33 @@ final class ImageReaderSurfaceProducer private int requestedWidth = 0; private int requestedHeight = 0; + /** Internal class: state held per image produced by image readers. */ + private class PerImage { + public final ImageReader reader; + public final Image image; + + public PerImage(ImageReader reader, Image image) { + this.reader = reader; + this.image = image; + } + + /** Call close when you are done with an the image. */ + public void close() { + this.image.close(); + maybeCloseReader(reader); + } + } + + // Active image reader. private ImageReader activeReader; - private ImageReader toBeClosedReader; - private Image latestImage; + // Set of image readers that should be closed. + private final Set readersToClose = new HashSet(); + // Last image produced. We keep this around until a new image is produced or the + // consumer consumes this image. + private PerImage lastProducedImage; + // Last image consumed. We only close this at the next image consumption point to avoid + // a race condition with the raster thread accessing an image we closed. + private PerImage lastConsumedImage; private final Handler onImageAvailableHandler = new Handler(); private final ImageReader.OnImageAvailableListener onImageAvailableListener = @@ -389,8 +413,7 @@ public void onImageAvailable(ImageReader reader) { if (image == null) { return; } - onImage(image); - maybeCloseReader(); + onImage(new PerImage(reader, image)); } }; @@ -403,24 +426,32 @@ public long id() { return id; } - @Override - public void release() { - if (released) { - return; - } + private void releaseInternal() { released = true; - if (this.latestImage != null) { - this.latestImage.close(); - this.latestImage = null; + if (this.lastProducedImage != null) { + this.lastProducedImage.close(); + this.lastProducedImage = null; + } + if (this.lastConsumedImage != null) { + this.lastConsumedImage.close(); + this.lastConsumedImage = null; } - if (this.toBeClosedReader != null) { - this.toBeClosedReader.close(); - this.toBeClosedReader = null; + for (ImageReader reader : readersToClose) { + reader.close(); } + readersToClose.clear(); if (this.activeReader != null) { this.activeReader.close(); this.activeReader = null; } + } + + @Override + public void release() { + if (released) { + return; + } + releaseInternal(); unregisterTexture(id); } @@ -432,10 +463,13 @@ public void setSize(int width, int height) { } this.requestedHeight = height; this.requestedWidth = width; - // Because the size was changed we will need to close the currently active reader. - // Instead of closing it eagerly we wait until the a frame is produced at the new - // size, ensuring that we don't render a blank frame in the app. - maybeMarkReaderForClose(); + synchronized (this) { + if (this.activeReader != null) { + // Schedule the old activeReader to be closed. + readersToClose.add(this.activeReader); + this.activeReader = null; + } + } } @Override @@ -457,56 +491,66 @@ public Surface getSurface() { @Override @TargetApi(29) public Image acquireLatestImage() { - Image r; + PerImage r; + PerImage toClose; synchronized (this) { - r = this.latestImage; - this.latestImage = null; + r = this.lastProducedImage; + this.lastProducedImage = null; + toClose = this.lastConsumedImage; + this.lastConsumedImage = r; } - maybeWaitOnFence(r); - return r; + if (toClose != null) { + toClose.close(); + } + if (r == null) { + return null; + } + maybeWaitOnFence(r.image); + return r.image; } - private void maybeMarkReaderForClose() { + private void maybeCloseReader(ImageReader reader) { synchronized (this) { - if (this.toBeClosedReader != null) { - // We only ever have two readers: - // 1) The reader to be closed after the next image is produced. - // 2) The reader being used to produce images. + if (this.lastConsumedImage != null && this.lastConsumedImage.reader == reader) { + // There is still a consumed image in flight for this reader. Don't close. return; } - this.toBeClosedReader = this.activeReader; - this.activeReader = null; - } - } - - private void maybeCloseReader() { - if (this.toBeClosedReader == null) { - return; + if (!readersToClose.contains(reader)) { + return; + } + readersToClose.remove(reader); } - this.toBeClosedReader.close(); - this.toBeClosedReader = null; + // Close the reader. + reader.close(); } private void maybeCreateReader() { - if (this.activeReader != null) { - return; + synchronized (this) { + if (this.activeReader != null) { + return; + } + this.activeReader = createImageReader(); } - this.activeReader = createImageReader(); } /** Invoked for each method that is available. */ - private void onImage(Image image) { + private void onImage(PerImage image) { if (released) { return; } - Image toClose; + PerImage toClose; synchronized (this) { - toClose = this.latestImage; - this.latestImage = image; + if (this.readersToClose.contains(image.reader)) { + Log.i(TAG, "Skipped frame because resize is in flight."); + image.close(); + return; + } + toClose = this.lastProducedImage; + this.lastProducedImage = image; } // Close the previously pushed buffer. if (toClose != null) { - Log.e(TAG, "RawSurfaceTexture frame was not acquired in a timely manner."); + Log.i(TAG, "Dropped frame."); toClose.close(); } if (image != null) { @@ -554,18 +598,7 @@ protected void finalize() throws Throwable { if (released) { return; } - if (latestImage != null) { - // Be sure to finalize any cached image. - latestImage.close(); - latestImage = null; - } - if (this.toBeClosedReader != null) { - this.toBeClosedReader.close(); - } - if (this.activeReader != null) { - this.activeReader.close(); - } - released = true; + releaseInternal(); handler.post(new TextureFinalizerRunnable(id, flutterJNI)); } finally { super.finalize(); diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index f567e32de0fee..581a8b229aa0b 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -149,7 +149,9 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega // Whether software rendering is used. private boolean usesSoftwareRendering = false; - private static boolean enableHardwareBufferRenderingTarget = true; + private static boolean enableImageRenderTarget = true; + + private static boolean enableSurfaceProducerRenderTarget = true; private final PlatformViewsChannel.PlatformViewsHandler channelHandler = new PlatformViewsChannel.PlatformViewsHandler() { @@ -975,7 +977,12 @@ private void unlockInputConnection(@NonNull VirtualDisplayController controller) private static PlatformViewRenderTarget makePlatformViewRenderTarget( TextureRegistry textureRegistry) { - if (enableHardwareBufferRenderingTarget && Build.VERSION.SDK_INT >= 33) { + if (enableSurfaceProducerRenderTarget && Build.VERSION.SDK_INT >= 33) { + final TextureRegistry.SurfaceProducer textureEntry = textureRegistry.createSurfaceProducer(); + Log.i(TAG, "PlatformView is using SurfaceProducer backend"); + return new SurfaceProducerPlatformViewRenderTarget(textureEntry); + } + if (enableImageRenderTarget && Build.VERSION.SDK_INT >= 33) { final TextureRegistry.ImageTextureEntry textureEntry = textureRegistry.createImageTexture(); Log.i(TAG, "PlatformView is using ImageReader backend"); return new ImageReaderPlatformViewRenderTarget(textureEntry); diff --git a/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java b/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java new file mode 100644 index 0000000000000..daf125f64ac82 --- /dev/null +++ b/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java @@ -0,0 +1,67 @@ +package io.flutter.plugin.platform; + +import android.annotation.TargetApi; +import android.graphics.Canvas; +import android.view.Surface; +import io.flutter.view.TextureRegistry.SurfaceProducer; + +@TargetApi(29) +public class SurfaceProducerPlatformViewRenderTarget implements PlatformViewRenderTarget { + private static final String TAG = "SurfaceProducerRenderTarget"; + private SurfaceProducer producer; + + public SurfaceProducerPlatformViewRenderTarget(SurfaceProducer producer) { + this.producer = producer; + } + + // Called when the render target should be resized. + public void resize(int width, int height) { + this.producer.setSize(width, height); + } + + // Returns the currently specified width. + public int getWidth() { + return this.producer.getWidth(); + } + + // Returns the currently specified height. + public int getHeight() { + return this.producer.getHeight(); + } + + // Forwards call to Surface returned by getSurface. + // NOTE: If this returns null the RenderTarget is "full" and has no room for a + // new frame. + public Canvas lockHardwareCanvas() { + Surface surface = this.producer.getSurface(); + return surface.lockHardwareCanvas(); + } + + // Forwards call to Surface returned by getSurface. + // NOTE: Must be called if lockHardwareCanvas returns a non-null Canvas. + public void unlockCanvasAndPost(Canvas canvas) { + Surface surface = this.producer.getSurface(); + surface.unlockCanvasAndPost(canvas); + } + + // The id of this render target. + public long getId() { + return this.producer.id(); + } + + // Releases backing resources. + public void release() { + this.producer.release(); + this.producer = null; + } + + // Returns true in the case that backing resource have been released. + public boolean isReleased() { + return this.producer == null; + } + + // Returns the Surface to be rendered on to. + public Surface getSurface() { + return this.producer.getSurface(); + } +} diff --git a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index ada752e4a2a4a..05ab3777de2aa 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -451,7 +451,7 @@ public void itDoesNotInvokeCreatesSurfaceWhenResumingRendering() { } @Test - public void ImageRawSurfaceTextureProducesImageOfCorrectSize() { + public void ImageReaderSurfaceProducerProducesImageOfCorrectSize() { FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); FlutterRenderer.ImageReaderSurfaceProducer texture = flutterRenderer.new ImageReaderSurfaceProducer(0); @@ -502,4 +502,34 @@ public void ImageRawSurfaceTextureProducesImageOfCorrectSize() { texture.release(); } + + @Test + public void ImageReaderSurfaceProducerSkipsFramesWhenResizeInflight() { + FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); + FlutterRenderer.ImageReaderSurfaceProducer texture = + flutterRenderer.new ImageReaderSurfaceProducer(0); + texture.disableFenceForTest(); + + // Returns a null image when one hasn't been produced. + assertNull(texture.acquireLatestImage()); + + // Give the texture an initial size. + texture.setSize(1, 1); + + // Render a frame. + Surface surface = texture.getSurface(); + assertNotNull(surface); + Canvas canvas = surface.lockHardwareCanvas(); + canvas.drawARGB(255, 255, 0, 0); + surface.unlockCanvasAndPost(canvas); + + // Resize. + texture.setSize(4, 4); + + // Let callbacks run. + shadowOf(Looper.getMainLooper()).idle(); + + // We should not get a new frame because the produced frame was for the previous size. + assertNull(texture.acquireLatestImage()); + } }