Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure all images are closed in FlutterImageView #20842

Merged
merged 6 commits into from Sep 1, 2020
Merged

Ensure all images are closed in FlutterImageView #20842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b2e67e
Ensure all images are closed in FlutterImageView
Aug 28, 2020
74bddff
Merge remote-tracking branch 'upstream/master' into fix_image_limit
Sep 1, 2020
fc81d61
Lint
Sep 1, 2020
43b9aee
Feedback
Sep 1, 2020
a43b79a
Fixes
Sep 1, 2020
f05d4f9
Use View#onDraw and suppress lint
Sep 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 39 additions & 33 deletions shell/platform/android/io/flutter/embedding/android/FlutterImageView.java
View file
Open in desktop
Expand Up @@ -4,7 +4,6 @@

package io.flutter.embedding.android;

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Context;
import android.graphics.Bitmap;
Expand All @@ -15,13 +14,16 @@
import android.media.Image;
import android.media.Image.Plane;
import android.media.ImageReader;
import android.util.AttributeSet;
import android.view.Surface;
import android.view.View;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import io.flutter.embedding.engine.renderer.FlutterRenderer;
import io.flutter.embedding.engine.renderer.RenderSurface;
import java.util.LinkedList;
import java.util.Queue;

/**
* Paints a Flutter UI provided by an {@link android.media.ImageReader} onto a {@link
Expand All @@ -35,11 +37,10 @@
* an {@link android.media.Image} and renders it to the {@link android.graphics.Canvas} in {@code
* onDraw}.
*/
@SuppressLint("ViewConstructor")
@TargetApi(19)
public class FlutterImageView extends View implements RenderSurface {
@NonNull private ImageReader imageReader;
@Nullable private Image nextImage;
@Nullable private Queue<Image> imageQueue;
@Nullable private Image currentImage;
@Nullable private Bitmap currentBitmap;
@Nullable private FlutterRenderer flutterRenderer;
Expand Down Expand Up @@ -70,17 +71,24 @@ public enum SurfaceKind {
* the Flutter UI.
*/
public FlutterImageView(@NonNull Context context, int width, int height, SurfaceKind kind) {
super(context, null);
this.imageReader = createImageReader(width, height);
this.kind = kind;
init();
this(context, createImageReader(width, height), kind);
}

public FlutterImageView(@NonNull Context context) {
this(context, 1, 1, SurfaceKind.background);
}

public FlutterImageView(@NonNull Context context, @NonNull AttributeSet attrs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the import for AttributeSet

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️

this(context, 1, 1, SurfaceKind.background);
}

@VisibleForTesting
FlutterImageView(@NonNull Context context, @NonNull ImageReader imageReader, SurfaceKind kind) {
/*package*/ FlutterImageView(
@NonNull Context context, @NonNull ImageReader imageReader, SurfaceKind kind) {
super(context, null);
this.imageReader = imageReader;
this.kind = kind;
this.imageQueue = new LinkedList<>();
init();
}

Expand Down Expand Up @@ -150,12 +158,14 @@ public void detachFromRenderer() {
// attached to the renderer again.
acquireLatestImage();
// Clear drawings.
pendingImages = 0;
currentBitmap = null;
if (nextImage != null) {
nextImage.close();
nextImage = null;

// Close the images in the queue and clear the queue.
for (final Image image : imageQueue) {
image.close();
}
imageQueue.clear();
// Close and clear the current image if any.
if (currentImage != null) {
currentImage.close();
currentImage = null;
Expand All @@ -168,7 +178,10 @@ public void pause() {
// Not supported.
}

/** Acquires the next image to be drawn to the {@link android.graphics.Canvas}. */
/**
* Acquires the next image to be drawn to the {@link android.graphics.Canvas}. Returns true if
* there's an image available in the queue.
*/
@TargetApi(19)
public boolean acquireLatestImage() {
if (!isAttachedToFlutterRenderer) {
Expand All @@ -182,14 +195,14 @@ public boolean acquireLatestImage() {
// While the engine will also stop producing frames, there is a race condition.
//
// To avoid exceptions, check if a new image can be acquired.
if (pendingImages < imageReader.getMaxImages()) {
nextImage = imageReader.acquireLatestImage();
if (nextImage != null) {
pendingImages++;
if (imageQueue.size() < imageReader.getMaxImages()) {
final Image image = imageReader.acquireLatestImage();
if (image != null) {
imageQueue.add(image);
}
}
invalidate();
return nextImage != null;
return !imageQueue.isEmpty();
}

/** Creates a new image reader with the provided size. */
Expand All @@ -200,15 +213,10 @@ public void resizeIfNeeded(int width, int height) {
if (width == imageReader.getWidth() && height == imageReader.getHeight()) {
return;
}
// Close resources.
if (nextImage != null) {
nextImage.close();
nextImage = null;
}
if (currentImage != null) {
currentImage.close();
currentImage = null;
}
imageQueue.clear();
currentImage = null;
// Close all the resources associated with the image reader,
// including the images.
imageReader.close();
// Image readers cannot be resized once created.
imageReader = createImageReader(width, height);
Expand All @@ -218,16 +226,14 @@ public void resizeIfNeeded(int width, int height) {
@Override
protected void onDraw(Canvas canvas) {
super.onDraw(canvas);
if (nextImage != null) {

if (!imageQueue.isEmpty()) {
if (currentImage != null) {
currentImage.close();
pendingImages--;
}
currentImage = nextImage;
nextImage = null;
currentImage = imageQueue.poll();
updateCurrentBitmap();
}

if (currentBitmap != null) {
canvas.drawBitmap(currentBitmap, 0, 0, null);
}
Expand All @@ -238,6 +244,7 @@ private void updateCurrentBitmap() {
if (android.os.Build.VERSION.SDK_INT >= 29) {
final HardwareBuffer buffer = currentImage.getHardwareBuffer();
currentBitmap = Bitmap.wrapHardwareBuffer(buffer, ColorSpace.get(ColorSpace.Named.SRGB));
buffer.close();
} else {
final Plane[] imagePlanes = currentImage.getPlanes();
if (imagePlanes.length != 1) {
Expand All @@ -255,7 +262,6 @@ private void updateCurrentBitmap() {
Bitmap.createBitmap(
desiredWidth, desiredHeight, android.graphics.Bitmap.Config.ARGB_8888);
}

currentBitmap.copyPixelsFromBuffer(imagePlane.getBuffer());
}
}
Expand Down
74 changes: 74 additions & 0 deletions shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java
View file
Open in desktop
Expand Up @@ -12,12 +12,15 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Context;
import android.content.res.Configuration;
import android.content.res.Resources;
import android.graphics.Canvas;
import android.graphics.Insets;
import android.media.Image;
import android.media.Image.Plane;
import android.media.ImageReader;
import android.view.View;
import android.view.ViewGroup;
Expand Down Expand Up @@ -550,6 +553,77 @@ public void flutterImageView_acquiresMaxImagesAtMost() {
verify(mockReader, times(2)).acquireLatestImage();
}

@Test
public void flutterImageView_detachFromRendererClosesAllImages() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(2);

final Image mockImage = mock(Image.class);
when(mockReader.acquireLatestImage()).thenReturn(mockImage);

final FlutterImageView imageView =
spy(
new FlutterImageView(
RuntimeEnvironment.application,
mockReader,
FlutterImageView.SurfaceKind.background));

final FlutterJNI jni = mock(FlutterJNI.class);
imageView.attachToRenderer(new FlutterRenderer(jni));

doNothing().when(imageView).invalidate();
imageView.acquireLatestImage();
imageView.acquireLatestImage();
imageView.detachFromRenderer();

verify(mockImage, times(2)).close();
}

@Test
@SuppressLint("WrongCall") /*View#onDraw*/
public void flutterImageView_onDrawClosesAllImages() {
final ImageReader mockReader = mock(ImageReader.class);
when(mockReader.getMaxImages()).thenReturn(2);

final Image mockImage = mock(Image.class);
when(mockImage.getPlanes()).thenReturn(new Plane[0]);
when(mockReader.acquireLatestImage()).thenReturn(mockImage);

final FlutterImageView imageView =
spy(
new FlutterImageView(
RuntimeEnvironment.application,
mockReader,
FlutterImageView.SurfaceKind.background));

final FlutterJNI jni = mock(FlutterJNI.class);
imageView.attachToRenderer(new FlutterRenderer(jni));

doNothing().when(imageView).invalidate();
imageView.acquireLatestImage();
imageView.acquireLatestImage();

imageView.onDraw(mock(Canvas.class));
imageView.onDraw(mock(Canvas.class));

// 1 image is closed and 1 is active.
verify(mockImage, times(1)).close();
verify(mockReader, times(2)).acquireLatestImage();

// This call doesn't do anything because there isn't
// an image in the queue.
imageView.onDraw(mock(Canvas.class));
verify(mockImage, times(1)).close();

// Aquire another image and push it to the queue.
imageView.acquireLatestImage();
verify(mockReader, times(3)).acquireLatestImage();

// Then, the second image is closed.
imageView.onDraw(mock(Canvas.class));
verify(mockImage, times(2)).close();
}

/*
* A custom shadow that reports fullscreen flag for system UI visibility
*/
Expand Down