Use new SurfaceProducer external texture class for rendering platform…

… views (#49201)

- 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

Related: [#139230](flutter/flutter#139230)
Related: [#139702](flutter/flutter#139702)

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

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",

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<ImageReader> 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();

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);

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();
+  }
+}

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());
+  }
 }