diff --git a/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc b/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc index 2efa6660cd8..2a7216a258a 100644 --- a/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc +++ b/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc @@ -35,6 +35,10 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { SurfaceTextureAttachToGLContext, (JavaLocalRef surface_texture, int textureId), (override)); + MOCK_METHOD(bool, + SurfaceTextureShouldUpdate, + (JavaLocalRef surface_texture), + (override)); MOCK_METHOD(void, SurfaceTextureUpdateTexImage, (JavaLocalRef surface_texture), diff --git a/engine/src/flutter/shell/platform/android/image_external_texture.cc b/engine/src/flutter/shell/platform/android/image_external_texture.cc index d045ca91ffd..87a8704c111 100644 --- a/engine/src/flutter/shell/platform/android/image_external_texture.cc +++ b/engine/src/flutter/shell/platform/android/image_external_texture.cc @@ -26,11 +26,9 @@ void ImageExternalTexture::Paint(PaintContext& context, return; } Attach(context); - const bool should_process_frame = - (!freeze && new_frame_ready_) || dl_image_ == nullptr; + const bool should_process_frame = !freeze; if (should_process_frame) { ProcessFrame(context, bounds); - new_frame_ready_ = false; } if (dl_image_) { context.canvas->DrawImageRect( @@ -48,7 +46,7 @@ void ImageExternalTexture::Paint(PaintContext& context, // Implementing flutter::Texture. void ImageExternalTexture::MarkNewFrameAvailable() { - new_frame_ready_ = true; + // NOOP. } // Implementing flutter::Texture. diff --git a/engine/src/flutter/shell/platform/android/image_external_texture.h b/engine/src/flutter/shell/platform/android/image_external_texture.h index 3de077706ff..251310cf537 100644 --- a/engine/src/flutter/shell/platform/android/image_external_texture.h +++ b/engine/src/flutter/shell/platform/android/image_external_texture.h @@ -61,7 +61,6 @@ class ImageExternalTexture : public flutter::Texture { enum class AttachmentState { kUninitialized, kAttached, kDetached }; AttachmentState state_ = AttachmentState::kUninitialized; - bool new_frame_ready_ = false; sk_sp dl_image_; diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 42eb25848e0..756298a777e 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -951,6 +951,16 @@ public class FlutterJNI { private native void nativeMarkTextureFrameAvailable(long nativeShellHolderId, long textureId); + /** Schedule the engine to draw a frame but does not invalidate the layout tree. */ + @UiThread + public void scheduleFrame() { + ensureRunningOnMainThread(); + ensureAttachedToNative(); + nativeScheduleFrame(nativeShellHolderId); + } + + private native void nativeScheduleFrame(long nativeShellHolderId); + /** * Unregisters a texture that was registered with {@link #registerTexture(long, * SurfaceTextureWrapper)}. 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 e6dacdf6fa1..9527bef01bb 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 @@ -27,8 +27,10 @@ import java.io.IOException; import java.lang.ref.WeakReference; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; @@ -172,21 +174,29 @@ public class FlutterRenderer implements TextureRegistry { @NonNull @Override public SurfaceProducer createSurfaceProducer() { - // Prior to Impeller, Flutter on Android *only* ran on OpenGLES (via Skia). That meant that + // Prior to Impeller, Flutter on Android *only* ran on OpenGLES (via Skia). That + // meant that // plugins (i.e. end-users) either explicitly created a SurfaceTexture (via // createX/registerX) or an ImageTexture (via createX/registerX). // - // In an Impeller world, which for the first time uses (if available) a Vulkan rendering - // backend, it is no longer possible (at least not trivially) to render an OpenGLES-provided + // In an Impeller world, which for the first time uses (if available) a Vulkan + // rendering + // backend, it is no longer possible (at least not trivially) to render an + // OpenGLES-provided // texture (SurfaceTexture) in a Vulkan context. // - // This function picks the "best" rendering surface based on the Android runtime, and - // provides a consumer-agnostic SurfaceProducer (which in turn vends a Surface), and has - // plugins (i.e. end-users) use the Surface instead, letting us "hide" the consumer-side + // This function picks the "best" rendering surface based on the Android + // runtime, and + // provides a consumer-agnostic SurfaceProducer (which in turn vends a Surface), + // and has + // plugins (i.e. end-users) use the Surface instead, letting us "hide" the + // consumer-side // of the implementation. // - // tl;dr: If ImageTexture is available, we use it, otherwise we use a SurfaceTexture. - // Coincidentally, if ImageTexture is available, we are also on an Android version that is + // tl;dr: If ImageTexture is available, we use it, otherwise we use a + // SurfaceTexture. + // Coincidentally, if ImageTexture is available, we are also on an Android + // version that is // running Vulkan, so we don't have to worry about it not being supported. final long id = nextTextureId.getAndIncrement(); final SurfaceProducer entry; @@ -302,7 +312,8 @@ public class FlutterRenderer implements TextureRegistry { // mNativeView==null. return; } - markTextureFrameAvailable(id); + textureWrapper.markDirty(); + scheduleEngineFrame(); }; if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { // The callback relies on being executed on the UI thread (unsynchronised read @@ -404,6 +415,10 @@ public class FlutterRenderer implements TextureRegistry { } } + // Keep a queue of ImageReaders. + // Each ImageReader holds acquired Images. + // When we acquire the next image, close any ImageReaders that don't have any + // more pending images. @Keep @TargetApi(29) final class ImageReaderSurfaceProducer @@ -411,200 +426,204 @@ public class FlutterRenderer implements TextureRegistry { private static final String TAG = "ImageReaderSurfaceProducer"; private static final int MAX_IMAGES = 4; + // Flip when debugging to see verbose logs. + private static final boolean VERBOSE_LOGS = false; + private final long id; private boolean released; + // Will be true in tests and on Android API < 33. private boolean ignoringFence = false; + // The requested width and height are updated by setSize. private int requestedWidth = 0; private int requestedHeight = 0; + // Whenever the requested width and height change we set this to be true so we + // create a new ImageReader (inside getSurface) with the correct width and height. + // We use this flag so that we lazily create the ImageReader only when a frame + // will be produced at that size. + private boolean createNewReader = true; - /** Internal class: state held per image produced by image readers. */ + // State held to track latency of various stages. + private long lastDequeueTime = 0; + private long lastQueueTime = 0; + private long lastScheduleTime = 0; + + private Object lock = new Object(); + // REQUIRED: The following fields must only be accessed when lock is held. + private final LinkedList imageReaderQueue = new LinkedList(); + private final HashMap perImageReaders = + new HashMap(); + + /** Internal class: state held per Image produced by ImageReaders. */ private class PerImage { - public final ImageReader reader; public final Image image; + public final long queuedTime; - public PerImage(ImageReader reader, Image image) { - this.reader = reader; + public PerImage(Image image, long queuedTime) { this.image = image; - } - - /** Call close when you are done with an the image. */ - public void close() { - this.image.close(); - maybeCloseReader(reader); + this.queuedTime = queuedTime; } } - // Active image reader. - private ImageReader activeReader; - // 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; + /** Internal class: state held per ImageReader. */ + private class PerImageReader { + public final ImageReader reader; + private final LinkedList imageQueue = new LinkedList(); - private final Handler onImageAvailableHandler = new Handler(); - private final ImageReader.OnImageAvailableListener onImageAvailableListener = - reader -> { - Image image = null; - try { - image = reader.acquireLatestImage(); - } catch (IllegalStateException e) { - Log.e(TAG, "onImageAvailable acquireLatestImage failed: " + e); - } - if (image == null) { - return; - } - onImage(new PerImage(reader, image)); - }; + private final ImageReader.OnImageAvailableListener onImageAvailableListener = + reader -> { + Image image = null; + try { + image = reader.acquireLatestImage(); + } catch (IllegalStateException e) { + Log.e(TAG, "onImageAvailable acquireLatestImage failed: " + e); + } + if (released) { + return; + } + if (image == null) { + return; + } + onImage(reader, image); + }; - ImageReaderSurfaceProducer(long id) { - this.id = id; + public PerImageReader(ImageReader reader) { + this.reader = reader; + reader.setOnImageAvailableListener(onImageAvailableListener, new Handler()); + } + + PerImage queueImage(Image image) { + PerImage perImage = new PerImage(image, System.nanoTime()); + imageQueue.add(perImage); + // If we fall too far behind we will skip some frames. + while (imageQueue.size() > 2) { + PerImage r = imageQueue.removeFirst(); + r.image.close(); + } + return perImage; + } + + PerImage dequeueImage() { + if (imageQueue.size() == 0) { + return null; + } + PerImage r = imageQueue.removeFirst(); + return r; + } + + /** returns true if we can prune this reader */ + boolean canPrune() { + return imageQueue.size() == 0; + } + + void close() { + reader.close(); + imageQueue.clear(); + } } - @Override - public long id() { - return id; + double deltaMillis(long deltaNanos) { + double ms = (double) deltaNanos / (double) 1000000.0; + return ms; + } + + PerImageReader getOrCreatePerImageReader(ImageReader reader) { + PerImageReader r = perImageReaders.get(reader); + if (r == null) { + r = new PerImageReader(reader); + perImageReaders.put(reader, r); + imageReaderQueue.add(r); + } + return r; + } + + void pruneImageReaderQueue() { + // Prune nodes from the head of the ImageReader queue. + while (imageReaderQueue.size() > 1) { + PerImageReader r = imageReaderQueue.peekFirst(); + if (!r.canPrune()) { + // No more ImageReaders can be pruned this round. + break; + } + imageReaderQueue.removeFirst(); + perImageReaders.remove(r.reader); + r.close(); + } + } + + void onImage(ImageReader reader, Image image) { + PerImage queuedImage = null; + synchronized (lock) { + PerImageReader perReader = getOrCreatePerImageReader(reader); + queuedImage = perReader.queueImage(image); + } + if (queuedImage == null) { + return; + } + if (VERBOSE_LOGS) { + if (lastQueueTime != 0) { + long now = System.nanoTime(); + long queueDelta = now - lastQueueTime; + Log.i( + TAG, + "enqueued image=" + + queuedImage.image.hashCode() + + " queueDelta=" + + deltaMillis(queueDelta)); + lastQueueTime = now; + } else { + lastQueueTime = System.nanoTime(); + } + } + scheduleEngineFrame(); + } + + PerImage dequeueImage() { + PerImage r = null; + synchronized (lock) { + for (PerImageReader reader : imageReaderQueue) { + r = reader.dequeueImage(); + if (r == null) { + // This reader is probably about to get pruned. + continue; + } + if (VERBOSE_LOGS) { + if (lastDequeueTime != 0) { + long now = System.nanoTime(); + long dequeueDelta = now - lastDequeueTime; + long queuedFor = now - r.queuedTime; + long scheduleDelay = now - lastScheduleTime; + Log.i( + TAG, + "dequeued image=" + + r.image.hashCode() + + " queuedFor= " + + deltaMillis(queuedFor) + + " dequeueDelta=" + + deltaMillis(dequeueDelta) + + " scheduleDelay=" + + deltaMillis(scheduleDelay)); + lastDequeueTime = now; + } else { + lastDequeueTime = System.nanoTime(); + } + } + // We have dequeued the first image. + break; + } + pruneImageReaderQueue(); + } + return r; } private void releaseInternal() { released = true; - if (this.lastProducedImage != null) { - this.lastProducedImage.close(); - this.lastProducedImage = null; + for (PerImageReader pir : perImageReaders.values()) { + pir.close(); } - if (this.lastConsumedImage != null) { - this.lastConsumedImage.close(); - this.lastConsumedImage = 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); - } - - @Override - public void setSize(int width, int height) { - if (requestedWidth == width && requestedHeight == height) { - // No size change. - return; - } - this.requestedHeight = height; - this.requestedWidth = width; - synchronized (this) { - if (this.activeReader != null) { - // Schedule the old activeReader to be closed. - readersToClose.add(this.activeReader); - this.activeReader = null; - } - } - } - - @Override - public int getWidth() { - return this.requestedWidth; - } - - @Override - public int getHeight() { - return this.requestedHeight; - } - - @Override - public Surface getSurface() { - maybeCreateReader(); - return activeReader.getSurface(); - } - - @Override - @TargetApi(29) - public Image acquireLatestImage() { - PerImage r; - PerImage toClose; - synchronized (this) { - r = this.lastProducedImage; - this.lastProducedImage = null; - toClose = this.lastConsumedImage; - this.lastConsumedImage = r; - } - if (toClose != null) { - toClose.close(); - } - if (r == null) { - return null; - } - maybeWaitOnFence(r.image); - return r.image; - } - - private void maybeCloseReader(ImageReader reader) { - synchronized (this) { - if (!readersToClose.contains(reader)) { - return; - } - if (this.lastConsumedImage != null && this.lastConsumedImage.reader == reader) { - // There is still a consumed image in flight for this reader. Don't close. - return; - } - if (this.lastProducedImage != null && this.lastProducedImage.reader == reader) { - // There is still a pending image for this reader. Don't close. - return; - } - readersToClose.remove(reader); - } - // Close the reader. - reader.close(); - } - - private void maybeCreateReader() { - synchronized (this) { - if (this.activeReader != null) { - return; - } - this.activeReader = createImageReader(); - } - } - - /** Invoked for each method that is available. */ - private void onImage(PerImage image) { - if (released) { - return; - } - PerImage toClose; - synchronized (this) { - 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.i(TAG, "Dropping rendered frame that was not acquired in time."); - toClose.close(); - } - // Mark that we have a new frame available. Eventually the raster thread will - // call acquireLatestImage. - markTextureFrameAvailable(id); + perImageReaders.clear(); + imageReaderQueue.clear(); } @TargetApi(33) @@ -637,6 +656,86 @@ public class FlutterRenderer implements TextureRegistry { Log.w(TAG, "ImageTextureEntry can't wait on the fence on Android < 33"); } + ImageReaderSurfaceProducer(long id) { + this.id = id; + } + + @Override + public long id() { + return id; + } + + @Override + public void release() { + if (released) { + return; + } + releaseInternal(); + unregisterTexture(id); + } + + @Override + public void setSize(int width, int height) { + if (requestedWidth == width && requestedHeight == height) { + // No size change. + return; + } + this.createNewReader = true; + this.requestedHeight = height; + this.requestedWidth = width; + } + + @Override + public int getWidth() { + return this.requestedWidth; + } + + @Override + public int getHeight() { + return this.requestedHeight; + } + + @Override + public Surface getSurface() { + PerImageReader pir = getActiveReader(); + return pir.reader.getSurface(); + } + + @Override + public void scheduleFrame() { + if (VERBOSE_LOGS) { + long now = System.nanoTime(); + if (lastScheduleTime != 0) { + long delta = now - lastScheduleTime; + Log.v(TAG, "scheduleFrame delta=" + deltaMillis(delta)); + } + lastScheduleTime = now; + } + scheduleEngineFrame(); + } + + @Override + @TargetApi(29) + public Image acquireLatestImage() { + PerImage r = dequeueImage(); + if (r == null) { + return null; + } + maybeWaitOnFence(r.image); + return r.image; + } + + private PerImageReader getActiveReader() { + synchronized (lock) { + if (createNewReader) { + createNewReader = false; + // Create a new ImageReader and add it to the queue. + return getOrCreatePerImageReader(createImageReader()); + } + return imageReaderQueue.peekLast(); + } + } + @Override protected void finalize() throws Throwable { try { @@ -669,7 +768,6 @@ public class FlutterRenderer implements TextureRegistry { // Hint that consumed images will only be read by GPU. builder.setUsage(HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); final ImageReader reader = builder.build(); - reader.setOnImageAvailableListener(this.onImageAvailableListener, onImageAvailableHandler); return reader; } @@ -682,7 +780,6 @@ public class FlutterRenderer implements TextureRegistry { ImageFormat.PRIVATE, MAX_IMAGES, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); - reader.setOnImageAvailableListener(this.onImageAvailableListener, onImageAvailableHandler); return reader; } @@ -703,8 +800,21 @@ public class FlutterRenderer implements TextureRegistry { } @VisibleForTesting - public int readersToCloseSize() { - return readersToClose.size(); + public int numImageReaders() { + synchronized (lock) { + return imageReaderQueue.size(); + } + } + + @VisibleForTesting + public int numImages() { + int r = 0; + synchronized (lock) { + for (PerImageReader reader : imageReaderQueue) { + r += reader.imageQueue.size(); + } + } + return r; } } @@ -757,8 +867,7 @@ public class FlutterRenderer implements TextureRegistry { toClose.close(); } if (image != null) { - // Mark that we have a new frame available. - markTextureFrameAvailable(id); + scheduleEngineFrame(); } } @@ -1023,6 +1132,10 @@ public class FlutterRenderer implements TextureRegistry { flutterJNI.registerImageTexture(textureId, imageTexture); } + private void scheduleEngineFrame() { + flutterJNI.scheduleFrame(); + } + // TODO(mattcarroll): describe the native behavior that this invokes private void markTextureFrameAvailable(long textureId) { flutterJNI.markTextureFrameAvailable(textureId); 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 50205f51d54..8153fc55826 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 @@ -81,4 +81,9 @@ final class SurfaceTextureSurfaceProducer } return surface; } + + @Override + public void scheduleFrame() { + flutterJNI.markTextureFrameAvailable(id); + } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureWrapper.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureWrapper.java index fcd6c1694c6..a4468a58d31 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureWrapper.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/SurfaceTextureWrapper.java @@ -22,6 +22,7 @@ public class SurfaceTextureWrapper { private boolean released; private boolean attached; private Runnable onFrameConsumed; + private boolean newFrameAvailable = false; public SurfaceTextureWrapper(@NonNull SurfaceTexture surfaceTexture) { this(surfaceTexture, null); @@ -47,10 +48,23 @@ public class SurfaceTextureWrapper { return surfaceTexture; } + public void markDirty() { + synchronized (this) { + newFrameAvailable = true; + } + } + + public boolean shouldUpdate() { + synchronized (this) { + return newFrameAvailable; + } + } + // Called by native. @SuppressWarnings("unused") public void updateTexImage() { synchronized (this) { + newFrameAvailable = false; if (!released) { surfaceTexture.updateTexImage(); if (onFrameConsumed != null) { diff --git a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRenderTarget.java b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRenderTarget.java index d04240374c3..7cb6c7a7840 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRenderTarget.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRenderTarget.java @@ -32,4 +32,7 @@ public interface PlatformViewRenderTarget { // Returns the Surface to be rendered on to. public Surface getSurface(); + + // Schedules a frame to be drawn. + public default void scheduleFrame() {} } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewWrapper.java b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewWrapper.java index 9ffb59877eb..9fac8a553eb 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewWrapper.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewWrapper.java @@ -167,6 +167,7 @@ public class PlatformViewWrapper extends FrameLayout { // Override the canvas that this subtree of views will use to draw. super.draw(targetCanvas); } finally { + renderTarget.scheduleFrame(); targetSurface.unlockCanvasAndPost(targetCanvas); } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java index 430bad3c89d..b37b53f1561 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/SurfaceProducerPlatformViewRenderTarget.java @@ -48,4 +48,8 @@ public class SurfaceProducerPlatformViewRenderTarget implements PlatformViewRend public Surface getSurface() { return this.producer.getSurface(); } + + public void scheduleFrame() { + this.producer.scheduleFrame(); + } } 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 7387e39a379..add6cebaeac 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 @@ -94,6 +94,8 @@ public interface TextureRegistry { * @return a Surface to use for a drawing target for various APIs. */ Surface getSurface(); + + void scheduleFrame(); }; /** A registry entry for a managed SurfaceTexture. */ diff --git a/engine/src/flutter/shell/platform/android/jni/jni_mock.h b/engine/src/flutter/shell/platform/android/jni/jni_mock.h index 20c64317dc5..dd6c9737eae 100644 --- a/engine/src/flutter/shell/platform/android/jni/jni_mock.h +++ b/engine/src/flutter/shell/platform/android/jni/jni_mock.h @@ -49,6 +49,11 @@ class JNIMock final : public PlatformViewAndroidJNI { (JavaLocalRef surface_texture, int textureId), (override)); + MOCK_METHOD(bool, + SurfaceTextureShouldUpdate, + (JavaLocalRef surface_texture), + (override)); + MOCK_METHOD(void, SurfaceTextureUpdateTexImage, (JavaLocalRef surface_texture), diff --git a/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h b/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h index 54aaa814fea..dcce4fd569e 100644 --- a/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h +++ b/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h @@ -91,6 +91,11 @@ class PlatformViewAndroidJNI { virtual void SurfaceTextureAttachToGLContext(JavaLocalRef surface_texture, int textureId) = 0; + //---------------------------------------------------------------------------- + /// @brief Returns true if surface_texture should be updated. + /// + virtual bool SurfaceTextureShouldUpdate(JavaLocalRef surface_texture) = 0; + //---------------------------------------------------------------------------- /// @brief Updates the texture image to the most recent frame from the /// image stream. diff --git a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc index 4fe0a585290..a928fb08cc2 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc +++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc @@ -111,6 +111,8 @@ static jmethodID g_java_weak_reference_get_method = nullptr; static jmethodID g_attach_to_gl_context_method = nullptr; +static jmethodID g_surface_texture_wrapper_should_update = nullptr; + static jmethodID g_update_tex_image_method = nullptr; static jmethodID g_get_transform_matrix_method = nullptr; @@ -521,6 +523,10 @@ static void MarkTextureFrameAvailable(JNIEnv* env, static_cast(texture_id)); } +static void ScheduleFrame(JNIEnv* env, jobject jcaller, jlong shell_holder) { + ANDROID_SHELL_HOLDER->GetPlatformView()->ScheduleFrame(); +} + static void InvokePlatformMessageResponseCallback(JNIEnv* env, jobject jcaller, jlong shell_holder, @@ -795,12 +801,16 @@ bool RegisterApi(JNIEnv* env) { .signature = "(JJ)V", .fnPtr = reinterpret_cast(&MarkTextureFrameAvailable), }, + { + .name = "nativeScheduleFrame", + .signature = "(J)V", + .fnPtr = reinterpret_cast(&ScheduleFrame), + }, { .name = "nativeUnregisterTexture", .signature = "(JJ)V", .fnPtr = reinterpret_cast(&UnregisterTexture), }, - // Methods for Dart callback functionality. { .name = "nativeLookupCallbackInformation", @@ -1159,6 +1169,15 @@ bool PlatformViewAndroid::Register(JNIEnv* env) { return false; } + g_surface_texture_wrapper_should_update = + env->GetMethodID(g_texture_wrapper_class->obj(), "shouldUpdate", "()Z"); + + if (g_surface_texture_wrapper_should_update == nullptr) { + FML_LOG(ERROR) + << "Could not locate SurfaceTextureWrapper.shouldUpdate method"; + return false; + } + g_update_tex_image_method = env->GetMethodID(g_texture_wrapper_class->obj(), "updateTexImage", "()V"); @@ -1453,6 +1472,29 @@ void PlatformViewAndroidJNIImpl::SurfaceTextureAttachToGLContext( FML_CHECK(fml::jni::CheckException(env)); } +bool PlatformViewAndroidJNIImpl::SurfaceTextureShouldUpdate( + JavaLocalRef surface_texture) { + JNIEnv* env = fml::jni::AttachCurrentThread(); + + if (surface_texture.is_null()) { + return false; + } + + fml::jni::ScopedJavaLocalRef surface_texture_local_ref( + env, env->CallObjectMethod(surface_texture.obj(), + g_java_weak_reference_get_method)); + if (surface_texture_local_ref.is_null()) { + return false; + } + + jboolean shouldUpdate = env->CallBooleanMethod( + surface_texture_local_ref.obj(), g_surface_texture_wrapper_should_update); + + FML_CHECK(fml::jni::CheckException(env)); + + return shouldUpdate; +} + void PlatformViewAndroidJNIImpl::SurfaceTextureUpdateTexImage( JavaLocalRef surface_texture) { JNIEnv* env = fml::jni::AttachCurrentThread(); diff --git a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h index 0ca073dab33..5cf432559f9 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h +++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h @@ -45,6 +45,8 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { void SurfaceTextureAttachToGLContext(JavaLocalRef surface_texture, int textureId) override; + bool SurfaceTextureShouldUpdate(JavaLocalRef surface_texture) override; + void SurfaceTextureUpdateTexImage(JavaLocalRef surface_texture) override; void SurfaceTextureGetTransformMatrix(JavaLocalRef surface_texture, diff --git a/engine/src/flutter/shell/platform/android/surface_texture_external_texture.cc b/engine/src/flutter/shell/platform/android/surface_texture_external_texture.cc index d4013a0a64e..8f86300c033 100644 --- a/engine/src/flutter/shell/platform/android/surface_texture_external_texture.cc +++ b/engine/src/flutter/shell/platform/android/surface_texture_external_texture.cc @@ -37,7 +37,7 @@ void SurfaceTextureExternalTexture::OnGrContextCreated() { } void SurfaceTextureExternalTexture::MarkNewFrameAvailable() { - new_frame_ready_ = true; + // NOOP. } void SurfaceTextureExternalTexture::Paint(PaintContext& context, @@ -47,11 +47,9 @@ void SurfaceTextureExternalTexture::Paint(PaintContext& context, if (state_ == AttachmentState::kDetached) { return; } - const bool should_process_frame = - (!freeze && new_frame_ready_) || dl_image_ == nullptr; - if (should_process_frame) { + const bool should_process_frame = !freeze; + if (should_process_frame && ShouldUpdate()) { ProcessFrame(context, bounds); - new_frame_ready_ = false; } FML_CHECK(state_ == AttachmentState::kAttached); @@ -111,6 +109,11 @@ void SurfaceTextureExternalTexture::Attach(int gl_tex_id) { state_ = AttachmentState::kAttached; } +bool SurfaceTextureExternalTexture::ShouldUpdate() { + return jni_facade_->SurfaceTextureShouldUpdate( + fml::jni::ScopedJavaLocalRef(surface_texture_)); +} + void SurfaceTextureExternalTexture::Update() { jni_facade_->SurfaceTextureUpdateTexImage( fml::jni::ScopedJavaLocalRef(surface_texture_)); diff --git a/engine/src/flutter/shell/platform/android/surface_texture_external_texture.h b/engine/src/flutter/shell/platform/android/surface_texture_external_texture.h index de4210dd4e9..e3400a1a649 100644 --- a/engine/src/flutter/shell/platform/android/surface_texture_external_texture.h +++ b/engine/src/flutter/shell/platform/android/surface_texture_external_texture.h @@ -40,6 +40,7 @@ class SurfaceTextureExternalTexture : public flutter::Texture { virtual void Detach(); void Attach(int gl_tex_id); + bool ShouldUpdate(); void Update(); enum class AttachmentState { kUninitialized, kAttached, kDetached }; @@ -48,7 +49,6 @@ class SurfaceTextureExternalTexture : public flutter::Texture { fml::jni::ScopedJavaGlobalRef surface_texture_; AttachmentState state_ = AttachmentState::kUninitialized; SkMatrix transform_; - bool new_frame_ready_ = false; sk_sp dl_image_; FML_DISALLOW_COPY_AND_ASSIGN(SurfaceTextureExternalTexture); 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 b2f2043e210..948f18a82f5 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 @@ -511,7 +511,7 @@ public class FlutterRendererTest { } @Test - public void ImageReaderSurfaceProducerSkipsFramesWhenResizeInflight() { + public void ImageReaderSurfaceProducerDoesNotDropFramesWhenResizeInflight() { FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); FlutterRenderer.ImageReaderSurfaceProducer texture = flutterRenderer.new ImageReaderSurfaceProducer(0); @@ -533,15 +533,15 @@ public class FlutterRendererTest { // Resize. texture.setSize(4, 4); - // Let callbacks run. + // Let callbacks run. The rendered frame will manifest here. shadowOf(Looper.getMainLooper()).idle(); - // We should not get a new frame because the produced frame was for the previous size. - assertNull(texture.acquireLatestImage()); + // We acquired the frame produced above. + assertNotNull(texture.acquireLatestImage()); } @Test - public void ImageReaderSurfaceProducerHandlesLateFrameWhenResizeInflight() { + public void ImageReaderSurfaceProducerImageReadersAndImagesCount() { FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); FlutterRenderer.ImageReaderSurfaceProducer texture = flutterRenderer.new ImageReaderSurfaceProducer(0); @@ -563,6 +563,9 @@ public class FlutterRendererTest { // Let callbacks run, this will produce a single frame. shadowOf(Looper.getMainLooper()).idle(); + assertEquals(1, texture.numImageReaders()); + assertEquals(1, texture.numImages()); + // Resize. texture.setSize(4, 4); @@ -574,15 +577,8 @@ public class FlutterRendererTest { // Let callbacks run. shadowOf(Looper.getMainLooper()).idle(); - // We will acquire a frame that is the old size. - Image produced = texture.acquireLatestImage(); - assertNotNull(produced); - assertEquals(produced.getWidth(), 1); - assertEquals(produced.getHeight(), 1); - - // We will still have one pending reader to be closed. - // If we didn't we would have closed the reader that owned the image we just acquired. - assertEquals(1, texture.readersToCloseSize()); + assertEquals(1, texture.numImageReaders()); + assertEquals(2, texture.numImages()); // Render a new frame with the current size. surface = texture.getSurface(); @@ -594,11 +590,35 @@ public class FlutterRendererTest { // Let callbacks run. shadowOf(Looper.getMainLooper()).idle(); - // Acquire the new image. - assertNotNull(texture.acquireLatestImage()); + assertEquals(2, texture.numImageReaders()); + assertEquals(3, texture.numImages()); - // We will have no pending readers to close. - assertEquals(0, texture.readersToCloseSize()); + // Acquire first frame. + Image produced = texture.acquireLatestImage(); + assertNotNull(produced); + assertEquals(1, produced.getWidth()); + assertEquals(1, produced.getHeight()); + assertEquals(2, texture.numImageReaders()); + assertEquals(2, texture.numImages()); + // Acquire second frame. This should result in closing the first ImageReader. + produced = texture.acquireLatestImage(); + assertNotNull(produced); + assertEquals(1, produced.getWidth()); + assertEquals(1, produced.getHeight()); + assertEquals(1, texture.numImageReaders()); + assertEquals(1, texture.numImages()); + // Acquire third frame. We should not close the active ImageReader. + produced = texture.acquireLatestImage(); + assertNotNull(produced); + assertEquals(4, produced.getWidth()); + assertEquals(4, produced.getHeight()); + assertEquals(1, texture.numImageReaders()); + assertEquals(0, texture.numImages()); + + // Returns null image when no more images are queued. + assertNull(texture.acquireLatestImage()); + assertEquals(1, texture.numImageReaders()); + assertEquals(0, texture.numImages()); } @Test 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 81e264232b3..c5871db27bf 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 @@ -1614,6 +1614,8 @@ public class PlatformViewsControllerTest { public Surface getSurface() { return null; } + + public void scheduleFrame() {} }; } };