diff --git a/shell/platform/android/image_external_texture.cc b/shell/platform/android/image_external_texture.cc index 855b56d2b77d8..90d78291c988f 100644 --- a/shell/platform/android/image_external_texture.cc +++ b/shell/platform/android/image_external_texture.cc @@ -26,7 +26,6 @@ void ImageExternalTexture::Paint(PaintContext& context, if (state_ == AttachmentState::kDetached) { return; } - latest_bounds_ = bounds; Attach(context); const bool should_process_frame = !freeze; if (should_process_frame) { diff --git a/shell/platform/android/image_external_texture.h b/shell/platform/android/image_external_texture.h index 6193c7b86baea..02ccdd91394db 100644 --- a/shell/platform/android/image_external_texture.h +++ b/shell/platform/android/image_external_texture.h @@ -59,8 +59,6 @@ class ImageExternalTexture : public flutter::Texture { fml::jni::ScopedJavaGlobalRef image_texture_entry_; std::shared_ptr jni_facade_; - SkRect latest_bounds_ = SkRect::MakeEmpty(); - fml::jni::ScopedJavaGlobalRef latest_android_image_; enum class AttachmentState { kUninitialized, kAttached, kDetached }; AttachmentState state_ = AttachmentState::kUninitialized; diff --git a/shell/platform/android/image_external_texture_gl.cc b/shell/platform/android/image_external_texture_gl.cc index 6a676e90cbf22..8ac28e46a7470 100644 --- a/shell/platform/android/image_external_texture_gl.cc +++ b/shell/platform/android/image_external_texture_gl.cc @@ -29,19 +29,19 @@ ImageExternalTextureGL::ImageExternalTextureGL( void ImageExternalTextureGL::Attach(PaintContext& context) { if (state_ == AttachmentState::kUninitialized) { - if (!latest_android_image_.is_null() && !latest_bounds_.isEmpty()) { - // After detach the cache of textures will have been cleared. If - // there is an android image we must populate it now so that the - // first frame isn't blank. - JavaLocalRef hardware_buffer = HardwareBufferFor(latest_android_image_); - UpdateImage(hardware_buffer, context); - CloseHardwareBuffer(hardware_buffer); - } + // TODO(johnmccurtchan): We currently display the first frame after an + // attach-detach cycle as blank. There seems to be an issue on some + // devices where ImageReaders/Images from before the detach aren't + // valid after the attach. According to Android folks this doesn't + // match the spec. Revisit this in the future. + // See https://github.com/flutter/flutter/issues/142978 and + // https://github.com/flutter/flutter/issues/139039. state_ = AttachmentState::kAttached; } } void ImageExternalTextureGL::UpdateImage(JavaLocalRef& hardware_buffer, + const SkRect& bounds, PaintContext& context) { AHardwareBuffer* latest_hardware_buffer = AHardwareBufferFor(hardware_buffer); std::optional key = @@ -57,7 +57,7 @@ void ImageExternalTextureGL::UpdateImage(JavaLocalRef& hardware_buffer, return; } - dl_image_ = CreateDlImage(context, latest_bounds_, key, std::move(egl_image)); + dl_image_ = CreateDlImage(context, bounds, key, std::move(egl_image)); if (key.has_value()) { gl_entries_.erase(image_lru_.AddImage(dl_image_, key.value())); } @@ -70,17 +70,8 @@ void ImageExternalTextureGL::ProcessFrame(PaintContext& context, return; } JavaLocalRef hardware_buffer = HardwareBufferFor(image); - UpdateImage(hardware_buffer, context); + UpdateImage(hardware_buffer, bounds, context); CloseHardwareBuffer(hardware_buffer); - - // NOTE: In the following code it is important that old_android_image is - // not closed until after the update of egl_image_ otherwise the image might - // be closed before the old EGLImage referencing it has been deleted. After - // an image is closed the underlying HardwareBuffer may be recycled and used - // for a future frame. - JavaLocalRef old_android_image(latest_android_image_); - latest_android_image_.Reset(image); - CloseImage(old_android_image); } void ImageExternalTextureGL::Detach() { diff --git a/shell/platform/android/image_external_texture_gl.h b/shell/platform/android/image_external_texture_gl.h index 8aeec744ab0cd..6a132237dbe3d 100644 --- a/shell/platform/android/image_external_texture_gl.h +++ b/shell/platform/android/image_external_texture_gl.h @@ -33,7 +33,9 @@ class ImageExternalTextureGL : public ImageExternalTexture { void Attach(PaintContext& context) override; void Detach() override; void ProcessFrame(PaintContext& context, const SkRect& bounds) override; - void UpdateImage(JavaLocalRef& hardware_buffer, PaintContext& context); + void UpdateImage(JavaLocalRef& hardware_buffer, + const SkRect& bounds, + PaintContext& context); virtual sk_sp CreateDlImage( PaintContext& context, diff --git a/shell/platform/android/image_external_texture_vk.cc b/shell/platform/android/image_external_texture_vk.cc index 9af0683340b1f..b5cf834aa9314 100644 --- a/shell/platform/android/image_external_texture_vk.cc +++ b/shell/platform/android/image_external_texture_vk.cc @@ -38,9 +38,7 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context, if (image.is_null()) { return; } - JavaLocalRef old_android_image(latest_android_image_); - latest_android_image_.Reset(image); - JavaLocalRef hardware_buffer = HardwareBufferFor(latest_android_image_); + JavaLocalRef hardware_buffer = HardwareBufferFor(image); AHardwareBuffer* latest_hardware_buffer = AHardwareBufferFor(hardware_buffer); AHardwareBuffer_Desc hb_desc = {}; @@ -53,9 +51,6 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context, dl_image_ = existing_image; CloseHardwareBuffer(hardware_buffer); - // IMPORTANT: We have just received a new frame to display so close the - // previous Java Image so that it is recycled and used for a future frame. - CloseImage(old_android_image); return; } @@ -105,9 +100,6 @@ void ImageExternalTextureVK::ProcessFrame(PaintContext& context, image_lru_.AddImage(dl_image_, key.value()); } CloseHardwareBuffer(hardware_buffer); - // IMPORTANT: We have just received a new frame to display so close the - // previous Java Image so that it is recycled and used for a future frame. - CloseImage(old_android_image); } } // namespace flutter 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 a5bc96401f7f1..66a47e6c030b1 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -189,6 +189,7 @@ public SurfaceProducer createSurfaceProducer() { if (!debugForceSurfaceProducerGlTextures && Build.VERSION.SDK_INT >= 29) { final ImageReaderSurfaceProducer producer = new ImageReaderSurfaceProducer(id); registerImageTexture(id, producer); + addOnTrimMemoryListener(producer); Log.v(TAG, "New ImageReaderSurfaceProducer ID: " + id); entry = producer; } else { @@ -404,7 +405,9 @@ public void run() { @Keep @TargetApi(29) final class ImageReaderSurfaceProducer - implements TextureRegistry.SurfaceProducer, TextureRegistry.ImageConsumer { + implements TextureRegistry.SurfaceProducer, + TextureRegistry.ImageConsumer, + TextureRegistry.OnTrimMemoryListener { private static final String TAG = "ImageReaderSurfaceProducer"; private static final int MAX_IMAGES = 4; @@ -436,6 +439,7 @@ final class ImageReaderSurfaceProducer private final LinkedList imageReaderQueue = new LinkedList(); private final HashMap perImageReaders = new HashMap(); + private PerImage lastDequeuedImage = null; private PerImageReader lastReaderDequeuedFrom = null; /** Internal class: state held per Image produced by ImageReaders. */ @@ -607,7 +611,15 @@ PerImage dequeueImage() { lastDequeueTime = System.nanoTime(); } } - // We have dequeued the first image from reader. + if (lastDequeuedImage != null) { + // We must keep the last image dequeued open until we are done presenting it. + // We have just dequeued a new image (r). Close the previously dequeued image. + lastDequeuedImage.image.close(); + lastDequeuedImage = null; + } + // Remember the last image and reader dequeued from. We do this because we must + // keep both of these alive until we are done presenting the image. + lastDequeuedImage = r; lastReaderDequeuedFrom = reader; break; } @@ -616,23 +628,40 @@ PerImage dequeueImage() { return r; } + @Override + public void onTrimMemory(int level) { + cleanup(); + createNewReader = true; + } + private void releaseInternal() { + cleanup(); released = true; - for (PerImageReader pir : perImageReaders.values()) { - pir.close(); + } + + private void cleanup() { + synchronized (lock) { + for (PerImageReader pir : perImageReaders.values()) { + pir.close(); + } + perImageReaders.clear(); + if (lastDequeuedImage != null) { + lastDequeuedImage.image.close(); + lastDequeuedImage = null; + } + if (lastReaderDequeuedFrom != null) { + lastReaderDequeuedFrom.close(); + lastReaderDequeuedFrom = null; + } + imageReaderQueue.clear(); } - perImageReaders.clear(); - imageReaderQueue.clear(); } @TargetApi(33) private void waitOnFence(Image image) { try { SyncFence fence = image.getFence(); - boolean signaled = fence.awaitForever(); - if (!signaled) { - Log.e(TAG, "acquireLatestImage image's fence was never signalled."); - } + fence.awaitForever(); } catch (IOException e) { // Drop. } @@ -874,10 +903,7 @@ public void pushImage(Image image) { private void waitOnFence(Image image) { try { SyncFence fence = image.getFence(); - boolean signaled = fence.awaitForever(); - if (!signaled) { - Log.e(TAG, "acquireLatestImage image's fence was never signalled."); - } + fence.awaitForever(); } catch (IOException e) { // Drop. } 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 5aae302666758..146a1a8348667 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 @@ -622,6 +622,40 @@ public void ImageReaderSurfaceProducerImageReadersAndImagesCount() { assertEquals(0, texture.numImages()); } + @Test + public void ImageReaderSurfaceProducerTrimMemoryCallback() { + FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); + FlutterRenderer.ImageReaderSurfaceProducer texture = + flutterRenderer.new ImageReaderSurfaceProducer(0); + texture.disableFenceForTest(); + + // Returns a null image when one hasn't been produced. + assertNull(texture.acquireLatestImage()); + + // Give the texture an initial size. + texture.setSize(1, 1); + + // Grab the surface so we can render a frame at 1x1 after resizing. + Surface surface = texture.getSurface(); + assertNotNull(surface); + Canvas canvas = surface.lockHardwareCanvas(); + canvas.drawARGB(255, 255, 0, 0); + surface.unlockCanvasAndPost(canvas); + + // Let callbacks run, this will produce a single frame. + shadowOf(Looper.getMainLooper()).idle(); + + assertEquals(1, texture.numImageReaders()); + assertEquals(1, texture.numImages()); + + // Invoke the onTrimMemory callback. + texture.onTrimMemory(0); + shadowOf(Looper.getMainLooper()).idle(); + + assertEquals(0, texture.numImageReaders()); + assertEquals(0, texture.numImages()); + } + // A 0x0 ImageReader is a runtime error. @Test public void ImageReaderSurfaceProducerClampsWidthAndHeightTo1() {