From 359594fcc5539a1e8f115a238716c058c40638be Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 17 Oct 2024 19:10:06 -0700 Subject: [PATCH] Request another frame in ImageReaderSurfaceProducer.dequeueImage if more images are pending in the queue ImageReaderSurfaceProducer will request a frame when an image is enqueued. But there is no guarantee that each request will produce an additional frame. Multiple requests happening within one vsync interval could be merged into one frame. If no other frame is scheduled, then some images will remain in the queue and the image shown on screen will not be the latest image. With this change, ImageReaderSurfaceProducer will continue requesting frames after consuming an image if the queue is not empty. Fixes https://github.com/flutter/flutter/issues/156903 Fixes https://github.com/flutter/flutter/issues/155787 --- .../engine/renderer/FlutterRenderer.java | 23 ++++++++++- .../engine/renderer/FlutterRendererTest.java | 39 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) 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 f66ea69fc94e7..54434c27d0594 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -553,6 +553,10 @@ boolean canPrune() { return imageQueue.isEmpty() && lastReaderDequeuedFrom != this; } + boolean imageQueueIsEmpty() { + return imageQueue.isEmpty(); + } + void close() { closed = true; if (VERBOSE_LOGS) { @@ -630,6 +634,7 @@ void onImage(ImageReader reader, Image image) { PerImage dequeueImage() { PerImage r = null; + boolean hasPendingImages = false; synchronized (lock) { for (PerImageReader reader : imageReaderQueue) { r = reader.dequeueImage(); @@ -679,6 +684,21 @@ PerImage dequeueImage() { break; } pruneImageReaderQueue(); + for (PerImageReader reader : imageReaderQueue) { + if (!reader.imageQueueIsEmpty()) { + hasPendingImages = true; + break; + } + } + } + if (hasPendingImages) { + // Request another frame to ensure that images are consumed until the queue is empty. + handler.post( + () -> { + if (!released) { + scheduleEngineFrame(); + } + }); } return r; } @@ -1245,7 +1265,8 @@ private void registerImageTexture( flutterJNI.registerImageTexture(textureId, imageTexture); } - private void scheduleEngineFrame() { + @VisibleForTesting + /* package */ void scheduleEngineFrame() { flutterJNI.scheduleFrame(); } 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 953a3e490b680..eaada1ef26fee 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 @@ -826,4 +826,43 @@ public void onSurfaceDestroyed() {} // Verify. latch.await(); } + + @Test + public void ImageReaderSurfaceProducerSchedulesFrameIfQueueNotEmpty() throws Exception { + FlutterRenderer flutterRenderer = spy(engineRule.getFlutterEngine().getRenderer()); + TextureRegistry.SurfaceProducer producer = flutterRenderer.createSurfaceProducer(); + FlutterRenderer.ImageReaderSurfaceProducer texture = + (FlutterRenderer.ImageReaderSurfaceProducer) producer; + texture.disableFenceForTest(); + texture.setSize(1, 1); + + // Render two frames. + for (int i = 0; i < 2; i++) { + Surface surface = texture.getSurface(); + assertNotNull(surface); + Canvas canvas = surface.lockHardwareCanvas(); + canvas.drawARGB(255, 255, 0, 0); + surface.unlockCanvasAndPost(canvas); + shadowOf(Looper.getMainLooper()).idle(); + } + + // Each enqueue of an image should result in a call to scheduleEngineFrame. + verify(flutterRenderer, times(2)).scheduleEngineFrame(); + + // Consume the first image. + Image image = texture.acquireLatestImage(); + shadowOf(Looper.getMainLooper()).idle(); + + // The dequeue should call scheduleEngineFrame because another image + // remains in the queue. + verify(flutterRenderer, times(3)).scheduleEngineFrame(); + + // Consume the second image. + image = texture.acquireLatestImage(); + shadowOf(Looper.getMainLooper()).idle(); + + // The dequeue should not call scheduleEngineFrame because the queue + // is now empty. + verify(flutterRenderer, times(3)).scheduleEngineFrame(); + } }