From 6640eed95d7e75cfb2099eade32e099d125da4ac Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 30 May 2025 12:39:33 -0700 Subject: [PATCH 01/10] add invalidateSurface + debug logs and logic --- .../engine/renderer/FlutterRenderer.java | 34 ++++++++++++++----- .../SurfaceTextureSurfaceProducer.java | 11 +++++- .../io/flutter/view/TextureRegistry.java | 2 ++ 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 31866d764db73..956ed4f15cebf 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -577,6 +577,8 @@ void pruneImageReaderQueue() { // No more ImageReaders can be pruned this round. break; } + Log.e("CAMILLE", "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Image reader queue getting pruned!"); + Log.e("CAMILLE prune hashcode:", Integer.toString(r.reader.getSurface().hashCode())); imageReaderQueue.removeFirst(); perImageReaders.remove(r.reader); r.close(); @@ -829,6 +831,11 @@ public Surface getSurface() { return pir.reader.getSurface(); } + @Override + public void invalidateSurface() { + createNewReader = true; + } + @Override public void scheduleFrame() { if (VERBOSE_LOGS) { @@ -855,17 +862,26 @@ public Image acquireLatestImage() { private PerImageReader getActiveReader() { synchronized (lock) { - if (createNewReader) { - createNewReader = false; - // Create a new ImageReader and add it to the queue. - ImageReader reader = createImageReader(); - if (VERBOSE_LOGS) { - Log.i( - TAG, reader.hashCode() + " created w=" + requestedWidth + " h=" + requestedHeight); + if (!createNewReader) { + // Verify we don't need a new ImageReader anyway because its Surface has been invalidated. + PerImageReader lastPerImageReader = imageReaderQueue.peekLast(); + Surface lastImageReaderSurface = lastPerImageReader.reader.getSurface(); + boolean lastImageReaderHasValidSurface = true; //lastImageReaderSurface.isValid(); + if (lastImageReaderHasValidSurface) { + Log.e("CAMILLE", "1: last image reader has valid surface!"); + return lastPerImageReader; } - return getOrCreatePerImageReader(reader); } - return imageReaderQueue.peekLast(); + + createNewReader = false; + // Create a new ImageReader and add it to the queue. + ImageReader reader = createImageReader(); + if (VERBOSE_LOGS) { + Log.i( + TAG, reader.hashCode() + " created w=" + requestedWidth + " h=" + requestedHeight); + } + Log.e("CAMILLE", "2: created new image reader!"); + return getOrCreatePerImageReader(reader); } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index ed18f80476848..db36fc0163df0 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -8,6 +8,8 @@ import io.flutter.embedding.engine.FlutterJNI; import io.flutter.view.TextureRegistry; +import io.flutter.Log; + /** Uses a {@link android.graphics.SurfaceTexture} to populate the texture registry. */ final class SurfaceTextureSurfaceProducer implements TextureRegistry.SurfaceProducer, TextureRegistry.GLTextureConsumer { @@ -92,12 +94,19 @@ public int getHeight() { @Override public Surface getSurface() { - if (surface == null) { + if (surface == null || !surface.isValid()) { + Log.e("CAMILLE", "1: getting new surface!"); surface = new Surface(texture.surfaceTexture()); } + Log.e("CAMILLE", "2: potentially returning old surface!"); return surface; } + @Override + public void invalidateSurface() { + surface = null; + } + @Override public void scheduleFrame() { flutterJNI.markTextureFrameAvailable(id); diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index 5d979049549fa..0428212290982 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -138,6 +138,8 @@ interface SurfaceProducer extends TextureEntry { */ Surface getSurface(); + void invalidateSurface(); + /** * Sets a callback that is notified when a previously created {@link Surface} returned by {@link * SurfaceProducer#getSurface()} is no longer valid due to being destroyed, or a new surface is From 234a553969e6ce6233f609d9be80699d2a3dcd80 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 2 Jun 2025 15:33:46 -0700 Subject: [PATCH 02/10] get rid of logs, add comments for fixes --- .../embedding/engine/renderer/FlutterRenderer.java | 9 ++++----- .../engine/renderer/SurfaceTextureSurfaceProducer.java | 6 ++++-- .../android/io/flutter/view/TextureRegistry.java | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 956ed4f15cebf..e4168997a7b86 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -577,8 +577,6 @@ void pruneImageReaderQueue() { // No more ImageReaders can be pruned this round. break; } - Log.e("CAMILLE", "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Image reader queue getting pruned!"); - Log.e("CAMILLE prune hashcode:", Integer.toString(r.reader.getSurface().hashCode())); imageReaderQueue.removeFirst(); perImageReaders.remove(r.reader); r.close(); @@ -831,6 +829,8 @@ public Surface getSurface() { return pir.reader.getSurface(); } + // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface + // by calling `SurfaceProducer.getSurface`. @Override public void invalidateSurface() { createNewReader = true; @@ -862,13 +862,13 @@ public Image acquireLatestImage() { private PerImageReader getActiveReader() { synchronized (lock) { + // POTENTIAL FIX #2 (does not fix camera issue): We should never return an invalid Surface. if (!createNewReader) { // Verify we don't need a new ImageReader anyway because its Surface has been invalidated. PerImageReader lastPerImageReader = imageReaderQueue.peekLast(); Surface lastImageReaderSurface = lastPerImageReader.reader.getSurface(); - boolean lastImageReaderHasValidSurface = true; //lastImageReaderSurface.isValid(); + boolean lastImageReaderHasValidSurface = lastImageReaderSurface.isValid(); if (lastImageReaderHasValidSurface) { - Log.e("CAMILLE", "1: last image reader has valid surface!"); return lastPerImageReader; } } @@ -880,7 +880,6 @@ private PerImageReader getActiveReader() { Log.i( TAG, reader.hashCode() + " created w=" + requestedWidth + " h=" + requestedHeight); } - Log.e("CAMILLE", "2: created new image reader!"); return getOrCreatePerImageReader(reader); } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index db36fc0163df0..5c52c91b90896 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -94,14 +94,16 @@ public int getHeight() { @Override public Surface getSurface() { + // POTENTIAL FIX #2 (does not fix camera issue): We should never return an invalid Surface. if (surface == null || !surface.isValid()) { - Log.e("CAMILLE", "1: getting new surface!"); surface = new Surface(texture.surfaceTexture()); } - Log.e("CAMILLE", "2: potentially returning old surface!"); return surface; } + + // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface + // by calling `SurfaceProducer.getSurface`. @Override public void invalidateSurface() { surface = null; diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index 0428212290982..e256371de462b 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -138,6 +138,10 @@ interface SurfaceProducer extends TextureEntry { */ Surface getSurface(); + /** + * POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface + * by calling `SurfaceProducer.getSurface`. + */ void invalidateSurface(); /** From 594611c8c4093641267f2e02991944ebe37d03d0 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 2 Jun 2025 15:37:39 -0700 Subject: [PATCH 03/10] format --- .../embedding/engine/renderer/FlutterRenderer.java | 6 +++--- .../engine/renderer/SurfaceTextureSurfaceProducer.java | 10 ++++------ .../android/io/flutter/view/TextureRegistry.java | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index e4168997a7b86..faa16e6842589 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -829,7 +829,8 @@ public Surface getSurface() { return pir.reader.getSurface(); } - // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface + // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously + // unretrieved Surface // by calling `SurfaceProducer.getSurface`. @Override public void invalidateSurface() { @@ -877,8 +878,7 @@ private PerImageReader getActiveReader() { // Create a new ImageReader and add it to the queue. ImageReader reader = createImageReader(); if (VERBOSE_LOGS) { - Log.i( - TAG, reader.hashCode() + " created w=" + requestedWidth + " h=" + requestedHeight); + Log.i(TAG, reader.hashCode() + " created w=" + requestedWidth + " h=" + requestedHeight); } return getOrCreatePerImageReader(reader); } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index 5c52c91b90896..34b750e8be1d5 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -8,8 +8,6 @@ import io.flutter.embedding.engine.FlutterJNI; import io.flutter.view.TextureRegistry; -import io.flutter.Log; - /** Uses a {@link android.graphics.SurfaceTexture} to populate the texture registry. */ final class SurfaceTextureSurfaceProducer implements TextureRegistry.SurfaceProducer, TextureRegistry.GLTextureConsumer { @@ -101,12 +99,12 @@ public Surface getSurface() { return surface; } - - // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface + // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously + // unretrieved Surface // by calling `SurfaceProducer.getSurface`. @Override - public void invalidateSurface() { - surface = null; + public void invalidateSurface() { + surface = null; } @Override diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index e256371de462b..6d9049d9c4b5b 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -139,8 +139,8 @@ interface SurfaceProducer extends TextureEntry { Surface getSurface(); /** - * POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously unretrieved Surface - * by calling `SurfaceProducer.getSurface`. + * POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously + * unretrieved Surface by calling `SurfaceProducer.getSurface`. */ void invalidateSurface(); From f79c6795f3825e7227323e0236adaf53219c5952 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 13 Jun 2025 11:16:12 -0700 Subject: [PATCH 04/10] format + base tests --- .../engine/renderer/FlutterRenderer.java | 15 +++++-- .../SurfaceTextureSurfaceProducer.java | 8 +++- .../engine/renderer/FlutterRendererTest.java | 40 +++++++++++++++++ .../SurfaceTextureSurfaceProducerTest.java | 43 +++++++++++++++++++ .../platform/PlatformViewsControllerTest.java | 3 ++ 5 files changed, 104 insertions(+), 5 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index faa16e6842589..a345614e59d69 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -480,7 +480,8 @@ public PerImage(Image image, long queuedTime) { } /** Internal class: state held per ImageReader. */ - private class PerImageReader { + @VisibleForTesting + public class PerImageReader { public final ImageReader reader; private final ArrayDeque imageQueue = new ArrayDeque<>(); private boolean closed = false; @@ -555,10 +556,10 @@ void close() { return (double) deltaNanos / 1000000.0; } - PerImageReader getOrCreatePerImageReader(ImageReader reader) { + private PerImageReader getOrCreatePerImageReader(ImageReader reader) { PerImageReader r = perImageReaders.get(reader); if (r == null) { - r = new PerImageReader(reader); + r = createPerImageReader(reader); perImageReaders.put(reader, r); imageReaderQueue.add(r); if (VERBOSE_LOGS) { @@ -568,6 +569,11 @@ PerImageReader getOrCreatePerImageReader(ImageReader reader) { return r; } + @VisibleForTesting + public PerImageReader createPerImageReader(ImageReader reader) { + return new PerImageReader(reader); + } + void pruneImageReaderQueue() { boolean change = false; // Prune nodes from the head of the ImageReader queue. @@ -924,7 +930,8 @@ private ImageReader createImageReader29() { HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); } - private ImageReader createImageReader() { + @VisibleForTesting + public ImageReader createImageReader() { if (Build.VERSION.SDK_INT >= API_LEVELS.API_33) { return createImageReader33(); } else if (Build.VERSION.SDK_INT >= API_LEVELS.API_29) { diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index 34b750e8be1d5..c0598ac1b56e2 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -5,6 +5,7 @@ import android.view.Surface; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import io.flutter.embedding.engine.FlutterJNI; import io.flutter.view.TextureRegistry; @@ -94,11 +95,16 @@ public int getHeight() { public Surface getSurface() { // POTENTIAL FIX #2 (does not fix camera issue): We should never return an invalid Surface. if (surface == null || !surface.isValid()) { - surface = new Surface(texture.surfaceTexture()); + surface = createSurface(texture.surfaceTexture()); } return surface; } + @VisibleForTesting + public Surface createSurface(SurfaceTexture surfaceTexture) { + return new Surface(surfaceTexture); + } + // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously // unretrieved Surface // by calling `SurfaceProducer.getSurface`. diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 8a1212fcb4c2a..04442530e2293 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -9,6 +9,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -21,12 +22,14 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; import android.graphics.Canvas; import android.graphics.Rect; import android.graphics.SurfaceTexture; import android.media.Image; +import android.media.ImageReader; import android.os.Looper; import android.view.Surface; import androidx.lifecycle.Lifecycle; @@ -36,6 +39,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.renderer.FlutterRenderer.ImageReaderSurfaceProducer.PerImageReader; import io.flutter.view.TextureRegistry; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; @@ -991,4 +995,40 @@ public void ImageReaderSurfaceProducerSchedulesFrameIfQueueNotEmpty() throws Exc // is now empty. verify(flutterRenderer, times(3)).scheduleEngineFrame(); } + + @Test + public void getSurface_doesNotReturnInvalidSurface() { + FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); + TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); + FlutterRenderer.ImageReaderSurfaceProducer spyImageReaderSurfaceProducer = + spy((FlutterRenderer.ImageReaderSurfaceProducer) producer); + ImageReader mockImageReader = mock(ImageReader.class); + PerImageReader fakePerImageReader = + spyImageReaderSurfaceProducer.new PerImageReader(mockImageReader); + Surface mockSurface = mock(Surface.class); + Surface mockSurface2 = mock(Surface.class); + + when(mockSurface.isValid()).thenReturn(false); + when(mockImageReader.getSurface()).thenReturn(mockSurface).thenReturn(mockSurface2); + when(spyImageReaderSurfaceProducer.createImageReader()).thenReturn(mockImageReader); + when(spyImageReaderSurfaceProducer.createPerImageReader(mockImageReader)) + .thenReturn(fakePerImageReader); + + Surface firstSurface = spyImageReaderSurfaceProducer.getSurface(); + Surface secondSurface = spyImageReaderSurfaceProducer.getSurface(); + + assertNotEquals(firstSurface, secondSurface); + } + + @Test + public void invalidateSurface_forcesGetSurfaceToReturnNewSurface() { + FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); + TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); + + Surface firstSurface = producer.getSurface(); + producer.invalidateSurface(); + Surface secondSurface = producer.getSurface(); + + assertNotEquals(firstSurface, secondSurface); + } } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java index e451ca51b4968..469e579219cb9 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java @@ -7,8 +7,11 @@ import static io.flutter.Build.API_LEVELS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; import android.annotation.TargetApi; @@ -19,6 +22,7 @@ import android.view.Surface; import androidx.test.ext.junit.runners.AndroidJUnit4; import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.view.TextureRegistry; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Test; import org.junit.runner.RunWith; @@ -84,4 +88,43 @@ public void releaseWillReleaseSurface() { producer.release(); assertFalse(surface.isValid()); } + + @Test + public void getSurface_doesNotReturnInvalidSurface() { + final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); + final Handler handler = new Handler(Looper.getMainLooper()); + final SurfaceTexture mockSurfaceTexture = mock(SurfaceTexture.class); + final TextureRegistry.SurfaceTextureEntry spyTexture = + spy(flutterRenderer.registerSurfaceTexture(mockSurfaceTexture)); + final SurfaceTextureSurfaceProducer producerSpy = + spy(new SurfaceTextureSurfaceProducer(0, handler, fakeJNI, spyTexture)); + final Surface mockSurface = mock(Surface.class); + final Surface mockSurface2 = mock(Surface.class); + + when(spyTexture.surfaceTexture()).thenReturn(mockSurfaceTexture); + when(mockSurface.isValid()).thenReturn(false); + when(producerSpy.createSurface(mockSurfaceTexture)) + .thenReturn(mockSurface) + .thenReturn(mockSurface2); + + final Surface surface1 = producerSpy.getSurface(); + final Surface surface2 = producerSpy.getSurface(); + + assertNotEquals(surface1, surface2); + } + + @Test + public void invalidateSurface_forcesGetSurfaceToReturnNewSurface() { + final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); + final Handler handler = new Handler(Looper.getMainLooper()); + final SurfaceTextureSurfaceProducer producer = + new SurfaceTextureSurfaceProducer( + 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); + + final Surface surface1 = producer.getSurface(); + producer.invalidateSurface(); + final Surface surface2 = producer.getSurface(); + + assertNotEquals(surface1, surface2); + } } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index cca4183676ea6..fe76955944d03 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -1668,6 +1668,9 @@ public long id() { return 0; } + @Override + public void invalidateSurface() {} + @Override public void release() {} From 478dd5199272587b3b832eb8ef9822b4ee9e02e6 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 13 Jun 2025 11:30:18 -0700 Subject: [PATCH 05/10] remove my wip comments --- .../io/flutter/embedding/engine/renderer/FlutterRenderer.java | 4 ---- .../engine/renderer/SurfaceTextureSurfaceProducer.java | 4 ---- 2 files changed, 8 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index a345614e59d69..b13feb352b570 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -835,9 +835,6 @@ public Surface getSurface() { return pir.reader.getSurface(); } - // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously - // unretrieved Surface - // by calling `SurfaceProducer.getSurface`. @Override public void invalidateSurface() { createNewReader = true; @@ -869,7 +866,6 @@ public Image acquireLatestImage() { private PerImageReader getActiveReader() { synchronized (lock) { - // POTENTIAL FIX #2 (does not fix camera issue): We should never return an invalid Surface. if (!createNewReader) { // Verify we don't need a new ImageReader anyway because its Surface has been invalidated. PerImageReader lastPerImageReader = imageReaderQueue.peekLast(); diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index c0598ac1b56e2..4e3c0502ccc38 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -93,7 +93,6 @@ public int getHeight() { @Override public Surface getSurface() { - // POTENTIAL FIX #2 (does not fix camera issue): We should never return an invalid Surface. if (surface == null || !surface.isValid()) { surface = createSurface(texture.surfaceTexture()); } @@ -105,9 +104,6 @@ public Surface createSurface(SurfaceTexture surfaceTexture) { return new Surface(surfaceTexture); } - // POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously - // unretrieved Surface - // by calling `SurfaceProducer.getSurface`. @Override public void invalidateSurface() { surface = null; From bf1358deecc18cfd5db25280ccd30f71d738e7c0 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 13 Jun 2025 13:00:16 -0700 Subject: [PATCH 06/10] change to getSurface(boolean) --- .../engine/renderer/FlutterRenderer.java | 13 ++++++++----- .../SurfaceTextureSurfaceProducer.java | 13 ++++++++----- .../io/flutter/view/TextureRegistry.java | 19 ++++++++++++++++--- .../engine/renderer/FlutterRendererTest.java | 5 ++--- .../SurfaceTextureSurfaceProducerTest.java | 5 ++--- .../platform/PlatformViewsControllerTest.java | 8 +++++--- 6 files changed, 41 insertions(+), 22 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index b13feb352b570..a6e7a5eadec1e 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -828,6 +828,14 @@ public int getHeight() { @Override public Surface getSurface() { + return getSurface(false); + } + + @Override + public Surface getSurface(boolean forceNewSurface) { + if (forceNewSurface) { + createNewReader = true; + } PerImageReader pir = getActiveReader(); if (VERBOSE_LOGS) { Log.i(TAG, pir.reader.hashCode() + " returning surface to render a new frame."); @@ -835,11 +843,6 @@ public Surface getSurface() { return pir.reader.getSurface(); } - @Override - public void invalidateSurface() { - createNewReader = true; - } - @Override public void scheduleFrame() { if (VERBOSE_LOGS) { diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index 4e3c0502ccc38..9d7842bde3cb0 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -93,6 +93,14 @@ public int getHeight() { @Override public Surface getSurface() { + return getSurface(false); + } + + @Override + public Surface getSurface(boolean forceNewSurface) { + if (forceNewSurface) { + surface = null; + } if (surface == null || !surface.isValid()) { surface = createSurface(texture.surfaceTexture()); } @@ -104,11 +112,6 @@ public Surface createSurface(SurfaceTexture surfaceTexture) { return new Surface(surfaceTexture); } - @Override - public void invalidateSurface() { - surface = null; - } - @Override public void scheduleFrame() { flutterJNI.markTextureFrameAvailable(id); diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index 6d9049d9c4b5b..33a73708a0204 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -134,15 +134,28 @@ interface SurfaceProducer extends TextureEntry { * time you need to draw. The surface may change when the texture is resized or has its format * changed. * + *

Calling this method is the same as calling {@link #getSurface(boolean)} with false. + * * @return a Surface to use for a drawing target for various APIs. */ Surface getSurface(); /** - * POTENTIAL FIX #1 (fixes camera issue): Provide a way to force retrieving a previously - * unretrieved Surface by calling `SurfaceProducer.getSurface`. + * Direct access to a surface object that is different from any previous calls to {@link + * #getSurface()} or {@link #getSurface(boolean)}. + * + *

When using this API, you will usually need to implement {@link SurfaceProducer.Callback} + * and provide it to {@link #setCallback(Callback)} in order to be notified when an existing + * surface has been destroyed (such as when the application goes to the background) or a new + * surface has been created (such as when the application is resumed back to the foreground). + * + *

NOTE: You should not cache the returned surface but instead invoke {@code getSurface} each + * time you need to draw. The surface may change when the texture is resized or has its format + * changed. + * + * @return a Surface to use for a drawing target for various APIs. */ - void invalidateSurface(); + Surface getSurface(boolean forceNewSurface); /** * Sets a callback that is notified when a previously created {@link Surface} returned by {@link diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 04442530e2293..b433869076076 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -1021,13 +1021,12 @@ public void getSurface_doesNotReturnInvalidSurface() { } @Test - public void invalidateSurface_forcesGetSurfaceToReturnNewSurface() { + public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); Surface firstSurface = producer.getSurface(); - producer.invalidateSurface(); - Surface secondSurface = producer.getSurface(); + Surface secondSurface = producer.getSurface(true); assertNotEquals(firstSurface, secondSurface); } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java index 469e579219cb9..4e90c9980afcf 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java @@ -114,7 +114,7 @@ public void getSurface_doesNotReturnInvalidSurface() { } @Test - public void invalidateSurface_forcesGetSurfaceToReturnNewSurface() { + public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); final Handler handler = new Handler(Looper.getMainLooper()); final SurfaceTextureSurfaceProducer producer = @@ -122,8 +122,7 @@ public void invalidateSurface_forcesGetSurfaceToReturnNewSurface() { 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); final Surface surface1 = producer.getSurface(); - producer.invalidateSurface(); - final Surface surface2 = producer.getSurface(); + final Surface surface2 = producer.getSurface(true); assertNotEquals(surface1, surface2); } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index fe76955944d03..58b1980582aee 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -1668,9 +1668,6 @@ public long id() { return 0; } - @Override - public void invalidateSurface() {} - @Override public void release() {} @@ -1692,6 +1689,11 @@ public Surface getSurface() { return null; } + @Override + public Surface getSurface(boolean forceNewSurface) { + return null; + } + @Override public boolean handlesCropAndRotation() { return false; From b3f9e1fe9768ddb8e694860798b37309a6cfdb74 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Tue, 17 Jun 2025 15:06:05 -0700 Subject: [PATCH 07/10] fix docs, add more tests --- .../io/flutter/view/TextureRegistry.java | 5 +- .../engine/renderer/FlutterRendererTest.java | 42 ++++++++++++++- .../SurfaceTextureSurfaceProducerTest.java | 51 ++++++++++++++++--- 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index 33a73708a0204..684b03b031e58 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -141,8 +141,9 @@ interface SurfaceProducer extends TextureEntry { Surface getSurface(); /** - * Direct access to a surface object that is different from any previous calls to {@link - * #getSurface()} or {@link #getSurface(boolean)}. + * Direct access to a surface, which will be newly created (and thus, different from any surface + * objects returned from previous calls to {@link #getSurface()} or {@link #getSurface(boolean)} + * if {@code forceNewSurface} is true. * *

When using this API, you will usually need to implement {@link SurfaceProducer.Callback} * and provide it to {@link #setCallback(Callback)} in order to be notified when an existing diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index b433869076076..0583b180553ba 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -1009,7 +1009,10 @@ public void getSurface_doesNotReturnInvalidSurface() { Surface mockSurface2 = mock(Surface.class); when(mockSurface.isValid()).thenReturn(false); - when(mockImageReader.getSurface()).thenReturn(mockSurface).thenReturn(mockSurface2); + when(mockImageReader.getSurface()) + .thenReturn(mockSurface) + .thenReturn(mockSurface) + .thenReturn(mockSurface2); when(spyImageReaderSurfaceProducer.createImageReader()).thenReturn(mockImageReader); when(spyImageReaderSurfaceProducer.createPerImageReader(mockImageReader)) .thenReturn(fakePerImageReader); @@ -1021,7 +1024,31 @@ public void getSurface_doesNotReturnInvalidSurface() { } @Test - public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { + public void getSurface_consecutiveCallsReturnSameSurfaceIfStillValid() { + FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); + TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); + FlutterRenderer.ImageReaderSurfaceProducer spyImageReaderSurfaceProducer = + spy((FlutterRenderer.ImageReaderSurfaceProducer) producer); + ImageReader mockImageReader = mock(ImageReader.class); + PerImageReader fakePerImageReader = + spyImageReaderSurfaceProducer.new PerImageReader(mockImageReader); + Surface mockSurface = mock(Surface.class); + Surface mockSurface2 = mock(Surface.class); + + when(mockSurface.isValid()).thenReturn(true); + when(mockImageReader.getSurface()).thenReturn(mockSurface); + when(spyImageReaderSurfaceProducer.createImageReader()).thenReturn(mockImageReader); + when(spyImageReaderSurfaceProducer.createPerImageReader(mockImageReader)) + .thenReturn(fakePerImageReader); + + Surface firstSurface = spyImageReaderSurfaceProducer.getSurface(); + Surface secondSurface = spyImageReaderSurfaceProducer.getSurface(); + + assertEquals(firstSurface, secondSurface); + } + + @Test + public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurfaceAsExpected() { FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); @@ -1030,4 +1057,15 @@ public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { assertNotEquals(firstSurface, secondSurface); } + + @Test + public void getSurfaceWithForceNewSurface_doesNotForceGetSurfaceToReturnNewSurfaceAsExpected() { + FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); + TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); + + Surface firstSurface = producer.getSurface(); + Surface secondSurface = producer.getSurface(false); + + assertEquals(firstSurface, secondSurface); + } } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java index 4e90c9980afcf..b13015e8689c5 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java @@ -99,18 +99,39 @@ public void getSurface_doesNotReturnInvalidSurface() { final SurfaceTextureSurfaceProducer producerSpy = spy(new SurfaceTextureSurfaceProducer(0, handler, fakeJNI, spyTexture)); final Surface mockSurface = mock(Surface.class); - final Surface mockSurface2 = mock(Surface.class); + final Surface mockSecondSurface = mock(Surface.class); when(spyTexture.surfaceTexture()).thenReturn(mockSurfaceTexture); when(mockSurface.isValid()).thenReturn(false); when(producerSpy.createSurface(mockSurfaceTexture)) .thenReturn(mockSurface) - .thenReturn(mockSurface2); + .thenReturn(mockSecondSurface); - final Surface surface1 = producerSpy.getSurface(); - final Surface surface2 = producerSpy.getSurface(); + final Surface firstSurface = producerSpy.getSurface(); + final Surface secondSurface = producerSpy.getSurface(); - assertNotEquals(surface1, surface2); + assertNotEquals(firstSurface, secondSurface); + } + + @Test + public void getSurface_consecutiveCallsReturnSameSurfaceIfStillValid() { + final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); + final Handler handler = new Handler(Looper.getMainLooper()); + final SurfaceTexture mockSurfaceTexture = mock(SurfaceTexture.class); + final TextureRegistry.SurfaceTextureEntry spyTexture = + spy(flutterRenderer.registerSurfaceTexture(mockSurfaceTexture)); + final SurfaceTextureSurfaceProducer producerSpy = + spy(new SurfaceTextureSurfaceProducer(0, handler, fakeJNI, spyTexture)); + final Surface mockSurface = mock(Surface.class); + + when(spyTexture.surfaceTexture()).thenReturn(mockSurfaceTexture); + when(mockSurface.isValid()).thenReturn(true); + when(producerSpy.createSurface(mockSurfaceTexture)).thenReturn(mockSurface); + + final Surface firstSurface = producerSpy.getSurface(); + final Surface secondSurface = producerSpy.getSurface(); + + assertEquals(firstSurface, secondSurface); } @Test @@ -121,9 +142,23 @@ public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { new SurfaceTextureSurfaceProducer( 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); - final Surface surface1 = producer.getSurface(); - final Surface surface2 = producer.getSurface(true); + final Surface firstSurface = producer.getSurface(); + final Surface secondSurface = producer.getSurface(true); + + assertNotEquals(firstSurface, secondSurface); + } + + @Test + public void getSurfaceWithForceNewSurface_doesNotForceGetSurfaceToReturnNewSurfaceAsExpected() { + final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); + final Handler handler = new Handler(Looper.getMainLooper()); + final SurfaceTextureSurfaceProducer producer = + new SurfaceTextureSurfaceProducer( + 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); + + Surface firstSurface = producer.getSurface(); + Surface secondSurface = producer.getSurface(false); - assertNotEquals(surface1, surface2); + assertEquals(firstSurface, secondSurface); } } From 008337cfca857178e59d84b24c5c6c99977a83c9 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Tue, 17 Jun 2025 15:28:35 -0700 Subject: [PATCH 08/10] self review --- .../engine/renderer/FlutterRenderer.java | 3 +- .../engine/renderer/FlutterRendererTest.java | 30 ++++++++----------- .../SurfaceTextureSurfaceProducerTest.java | 10 +++---- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index a6e7a5eadec1e..845d2b296e551 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -480,8 +480,7 @@ public PerImage(Image image, long queuedTime) { } /** Internal class: state held per ImageReader. */ - @VisibleForTesting - public class PerImageReader { + private class PerImageReader { public final ImageReader reader; private final ArrayDeque imageQueue = new ArrayDeque<>(); private boolean closed = false; diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 0583b180553ba..58321200a794a 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -39,7 +39,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import io.flutter.embedding.android.FlutterActivity; import io.flutter.embedding.engine.FlutterJNI; -import io.flutter.embedding.engine.renderer.FlutterRenderer.ImageReaderSurfaceProducer.PerImageReader; import io.flutter.view.TextureRegistry; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; @@ -1003,24 +1002,23 @@ public void getSurface_doesNotReturnInvalidSurface() { FlutterRenderer.ImageReaderSurfaceProducer spyImageReaderSurfaceProducer = spy((FlutterRenderer.ImageReaderSurfaceProducer) producer); ImageReader mockImageReader = mock(ImageReader.class); - PerImageReader fakePerImageReader = - spyImageReaderSurfaceProducer.new PerImageReader(mockImageReader); - Surface mockSurface = mock(Surface.class); - Surface mockSurface2 = mock(Surface.class); + ImageReader mockSecondImageReader = mock(ImageReader.class); + Surface firstMockSurface = mock(Surface.class); + Surface secondMockSurface = mock(Surface.class); - when(mockSurface.isValid()).thenReturn(false); - when(mockImageReader.getSurface()) - .thenReturn(mockSurface) - .thenReturn(mockSurface) - .thenReturn(mockSurface2); - when(spyImageReaderSurfaceProducer.createImageReader()).thenReturn(mockImageReader); - when(spyImageReaderSurfaceProducer.createPerImageReader(mockImageReader)) - .thenReturn(fakePerImageReader); + when(mockImageReader.getSurface()).thenReturn(firstMockSurface); + when(mockSecondImageReader.getSurface()).thenReturn(secondMockSurface); + when(firstMockSurface.isValid()).thenReturn(false); + when(spyImageReaderSurfaceProducer.createImageReader()) + .thenReturn(mockImageReader) + .thenReturn(mockSecondImageReader); Surface firstSurface = spyImageReaderSurfaceProducer.getSurface(); Surface secondSurface = spyImageReaderSurfaceProducer.getSurface(); assertNotEquals(firstSurface, secondSurface); + assertEquals(firstSurface, firstMockSurface); + assertEquals(secondSurface, secondMockSurface); } @Test @@ -1030,21 +1028,17 @@ public void getSurface_consecutiveCallsReturnSameSurfaceIfStillValid() { FlutterRenderer.ImageReaderSurfaceProducer spyImageReaderSurfaceProducer = spy((FlutterRenderer.ImageReaderSurfaceProducer) producer); ImageReader mockImageReader = mock(ImageReader.class); - PerImageReader fakePerImageReader = - spyImageReaderSurfaceProducer.new PerImageReader(mockImageReader); Surface mockSurface = mock(Surface.class); - Surface mockSurface2 = mock(Surface.class); when(mockSurface.isValid()).thenReturn(true); when(mockImageReader.getSurface()).thenReturn(mockSurface); when(spyImageReaderSurfaceProducer.createImageReader()).thenReturn(mockImageReader); - when(spyImageReaderSurfaceProducer.createPerImageReader(mockImageReader)) - .thenReturn(fakePerImageReader); Surface firstSurface = spyImageReaderSurfaceProducer.getSurface(); Surface secondSurface = spyImageReaderSurfaceProducer.getSurface(); assertEquals(firstSurface, secondSurface); + assertEquals(firstSurface, mockSurface); } @Test diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java index b13015e8689c5..444c0a66ed133 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java @@ -98,14 +98,14 @@ public void getSurface_doesNotReturnInvalidSurface() { spy(flutterRenderer.registerSurfaceTexture(mockSurfaceTexture)); final SurfaceTextureSurfaceProducer producerSpy = spy(new SurfaceTextureSurfaceProducer(0, handler, fakeJNI, spyTexture)); - final Surface mockSurface = mock(Surface.class); - final Surface mockSecondSurface = mock(Surface.class); + final Surface firstMockSurface = mock(Surface.class); + final Surface secondMockSurface = mock(Surface.class); when(spyTexture.surfaceTexture()).thenReturn(mockSurfaceTexture); - when(mockSurface.isValid()).thenReturn(false); + when(firstMockSurface.isValid()).thenReturn(false); when(producerSpy.createSurface(mockSurfaceTexture)) - .thenReturn(mockSurface) - .thenReturn(mockSecondSurface); + .thenReturn(firstMockSurface) + .thenReturn(secondMockSurface); final Surface firstSurface = producerSpy.getSurface(); final Surface secondSurface = producerSpy.getSurface(); From 10f76d34b01b851ca7cd6e3e7a868398a64f1f07 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Wed, 18 Jun 2025 08:34:23 -0700 Subject: [PATCH 09/10] rearrange javadoc --- .../platform/android/io/flutter/view/TextureRegistry.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index 684b03b031e58..f561439abc492 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -130,12 +130,12 @@ interface SurfaceProducer extends TextureEntry { * surface has been destroyed (such as when the application goes to the background) or a new * surface has been created (such as when the application is resumed back to the foreground). * + *

Calling this method is the same as calling {@link #getSurface(boolean)} with false. + * *

NOTE: You should not cache the returned surface but instead invoke {@code getSurface} each * time you need to draw. The surface may change when the texture is resized or has its format * changed. * - *

Calling this method is the same as calling {@link #getSurface(boolean)} with false. - * * @return a Surface to use for a drawing target for various APIs. */ Surface getSurface(); From 58187ad5a4bccadf2c3ba5131c4fbe9f9b1af8a9 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Wed, 18 Jun 2025 13:48:33 -0700 Subject: [PATCH 10/10] rename getSurface(boolean) to getForcedNewSurface() --- .../embedding/engine/renderer/FlutterRenderer.java | 14 ++++++-------- .../renderer/SurfaceTextureSurfaceProducer.java | 14 ++++++-------- .../android/io/flutter/view/TextureRegistry.java | 8 +++----- .../engine/renderer/FlutterRendererTest.java | 8 ++++---- .../SurfaceTextureSurfaceProducerTest.java | 8 ++++---- .../platform/PlatformViewsControllerTest.java | 2 +- 6 files changed, 24 insertions(+), 30 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 845d2b296e551..082633c29b864 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -827,14 +827,6 @@ public int getHeight() { @Override public Surface getSurface() { - return getSurface(false); - } - - @Override - public Surface getSurface(boolean forceNewSurface) { - if (forceNewSurface) { - createNewReader = true; - } PerImageReader pir = getActiveReader(); if (VERBOSE_LOGS) { Log.i(TAG, pir.reader.hashCode() + " returning surface to render a new frame."); @@ -842,6 +834,12 @@ public Surface getSurface(boolean forceNewSurface) { return pir.reader.getSurface(); } + @Override + public Surface getForcedNewSurface() { + createNewReader = true; + return getSurface(); + } + @Override public void scheduleFrame() { if (VERBOSE_LOGS) { diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java index 9d7842bde3cb0..771fec4766744 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducer.java @@ -93,20 +93,18 @@ public int getHeight() { @Override public Surface getSurface() { - return getSurface(false); - } - - @Override - public Surface getSurface(boolean forceNewSurface) { - if (forceNewSurface) { - surface = null; - } if (surface == null || !surface.isValid()) { surface = createSurface(texture.surfaceTexture()); } return surface; } + @Override + public Surface getForcedNewSurface() { + surface = null; + return getSurface(); + } + @VisibleForTesting public Surface createSurface(SurfaceTexture surfaceTexture) { return new Surface(surfaceTexture); diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index f561439abc492..2ba9e6ff947a3 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -130,8 +130,6 @@ interface SurfaceProducer extends TextureEntry { * surface has been destroyed (such as when the application goes to the background) or a new * surface has been created (such as when the application is resumed back to the foreground). * - *

Calling this method is the same as calling {@link #getSurface(boolean)} with false. - * *

NOTE: You should not cache the returned surface but instead invoke {@code getSurface} each * time you need to draw. The surface may change when the texture is resized or has its format * changed. @@ -142,8 +140,8 @@ interface SurfaceProducer extends TextureEntry { /** * Direct access to a surface, which will be newly created (and thus, different from any surface - * objects returned from previous calls to {@link #getSurface()} or {@link #getSurface(boolean)} - * if {@code forceNewSurface} is true. + * objects returned from previous calls to {@link #getSurface()} or {@link + * #getForcedNewSurface()}. * *

When using this API, you will usually need to implement {@link SurfaceProducer.Callback} * and provide it to {@link #setCallback(Callback)} in order to be notified when an existing @@ -156,7 +154,7 @@ interface SurfaceProducer extends TextureEntry { * * @return a Surface to use for a drawing target for various APIs. */ - Surface getSurface(boolean forceNewSurface); + Surface getForcedNewSurface(); /** * Sets a callback that is notified when a previously created {@link Surface} returned by {@link diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 58321200a794a..d08a31be6fcf3 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -1042,23 +1042,23 @@ public void getSurface_consecutiveCallsReturnSameSurfaceIfStillValid() { } @Test - public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurfaceAsExpected() { + public void getForcedNewSurface_returnsNewSurface() { FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); Surface firstSurface = producer.getSurface(); - Surface secondSurface = producer.getSurface(true); + Surface secondSurface = producer.getForcedNewSurface(); assertNotEquals(firstSurface, secondSurface); } @Test - public void getSurfaceWithForceNewSurface_doesNotForceGetSurfaceToReturnNewSurfaceAsExpected() { + public void getSurface_doesNotReturnNewSurface() { FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); Surface firstSurface = producer.getSurface(); - Surface secondSurface = producer.getSurface(false); + Surface secondSurface = producer.getSurface(); assertEquals(firstSurface, secondSurface); } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java index 444c0a66ed133..6dc4e997c5538 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/SurfaceTextureSurfaceProducerTest.java @@ -135,7 +135,7 @@ public void getSurface_consecutiveCallsReturnSameSurfaceIfStillValid() { } @Test - public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { + public void getForcedNewSurface_returnsNewSurface() { final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); final Handler handler = new Handler(Looper.getMainLooper()); final SurfaceTextureSurfaceProducer producer = @@ -143,13 +143,13 @@ public void getSurfaceWithForceNewSurface_forcesGetSurfaceToReturnNewSurface() { 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); final Surface firstSurface = producer.getSurface(); - final Surface secondSurface = producer.getSurface(true); + final Surface secondSurface = producer.getForcedNewSurface(); assertNotEquals(firstSurface, secondSurface); } @Test - public void getSurfaceWithForceNewSurface_doesNotForceGetSurfaceToReturnNewSurfaceAsExpected() { + public void getSurface_doesNotReturnNewSurface() { final FlutterRenderer flutterRenderer = new FlutterRenderer(fakeJNI); final Handler handler = new Handler(Looper.getMainLooper()); final SurfaceTextureSurfaceProducer producer = @@ -157,7 +157,7 @@ public void getSurfaceWithForceNewSurface_doesNotForceGetSurfaceToReturnNewSurfa 0, handler, fakeJNI, flutterRenderer.registerSurfaceTexture(new SurfaceTexture(0))); Surface firstSurface = producer.getSurface(); - Surface secondSurface = producer.getSurface(false); + Surface secondSurface = producer.getSurface(); assertEquals(firstSurface, secondSurface); } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 58b1980582aee..6ab5e05631079 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -1690,7 +1690,7 @@ public Surface getSurface() { } @Override - public Surface getSurface(boolean forceNewSurface) { + public Surface getForcedNewSurface() { return null; }