Skip to content

Commit c332675

Browse files
author
Emmanuel Garcia
authored
Fix hybrid composition bugs (flutter#19325)
1 parent fc0e272 commit c332675

13 files changed

+216
-33
lines changed

shell/platform/android/external_view_embedder/external_view_embedder.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView(
2626
"AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView");
2727

2828
auto rtree_factory = RTreeFactory();
29-
view_rtrees_.insert({view_id, rtree_factory.getInstance()});
29+
view_rtrees_.insert_or_assign(view_id, rtree_factory.getInstance());
3030

3131
auto picture_recorder = std::make_unique<SkPictureRecorder>();
3232
picture_recorder->beginRecording(SkRect::Make(frame_size_), &rtree_factory);
3333

34-
picture_recorders_.insert({view_id, std::move(picture_recorder)});
34+
picture_recorders_.insert_or_assign(view_id, std::move(picture_recorder));
3535
composition_order_.push_back(view_id);
3636
// Update params only if they changed.
3737
if (view_params_.count(view_id) == 1 &&
@@ -96,6 +96,8 @@ bool AndroidExternalViewEmbedder::SubmitFrame(
9696
picture_recorders_.at(view_id)->finishRecordingAsPicture();
9797
FML_CHECK(picture);
9898
pictures.insert({view_id, picture});
99+
overlay_layers.insert({view_id, {}});
100+
99101
sk_sp<RTree> rtree = view_rtrees_.at(view_id);
100102
// Determinate if Flutter UI intersects with any of the previous
101103
// platform views stacked by z position.
@@ -124,20 +126,17 @@ bool AndroidExternalViewEmbedder::SubmitFrame(
124126
intersection_rects.push_back(joined_rect);
125127
}
126128
for (SkRect& intersection_rect : intersection_rects) {
127-
// Get the intersection rect between the current rect
128-
// and the platform view rect.
129-
// joined_rect.intersect(platform_view_rect);
130129
// Subpixels in the platform may not align with the canvas subpixels.
131130
//
132131
// To workaround it, round the floating point bounds and make the rect
133132
// slighly larger. For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4,
134133
// 5}.
135134
intersection_rect.set(intersection_rect.roundOut());
135+
overlay_layers.at(view_id).push_back(intersection_rect);
136136
// Clip the background canvas, so it doesn't contain any of the pixels
137137
// drawn on the overlay layer.
138138
background_canvas->clipRect(intersection_rect, SkClipOp::kDifference);
139139
}
140-
overlay_layers.insert({current_view_id, intersection_rects});
141140
}
142141
background_canvas->drawPicture(pictures.at(view_id));
143142
}

shell/platform/android/io/flutter/embedding/android/FlutterImageView.java

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,33 @@ public class FlutterImageView extends View implements RenderSurface {
4141
@Nullable private Bitmap currentBitmap;
4242
@Nullable private FlutterRenderer flutterRenderer;
4343

44+
public enum SurfaceKind {
45+
/** Displays the background canvas. */
46+
background,
47+
48+
/** Displays the overlay surface canvas. */
49+
overlay,
50+
}
51+
52+
/** The kind of surface. */
53+
private SurfaceKind kind;
54+
55+
/**
56+
* The number of images acquired from the current {@link android.media.ImageReader} that are
57+
* waiting to be painted. This counter is decreased after calling {@link
58+
* android.media.Image#close()}.
59+
*/
60+
private int pendingImages = 0;
61+
4462
/**
4563
* Constructs a {@code FlutterImageView} with an {@link android.media.ImageReader} that provides
4664
* the Flutter UI.
4765
*/
48-
public FlutterImageView(@NonNull Context context, @NonNull ImageReader imageReader) {
66+
public FlutterImageView(
67+
@NonNull Context context, @NonNull ImageReader imageReader, SurfaceKind kind) {
4968
super(context, null);
5069
this.imageReader = imageReader;
70+
this.kind = kind;
5171
}
5272

5373
@Nullable
@@ -62,29 +82,56 @@ public FlutterRenderer getAttachedRenderer() {
6282
*/
6383
@Override
6484
public void attachToRenderer(@NonNull FlutterRenderer flutterRenderer) {
65-
if (this.flutterRenderer != null) {
66-
this.flutterRenderer.stopRenderingToSurface();
67-
}
68-
6985
this.flutterRenderer = flutterRenderer;
70-
flutterRenderer.startRenderingToSurface(imageReader.getSurface());
86+
switch (kind) {
87+
case background:
88+
flutterRenderer.swapSurface(imageReader.getSurface());
89+
break;
90+
case overlay:
91+
// Don't do anything as this is done by the handler of
92+
// `FlutterJNI#createOverlaySurface()` in the native side.
93+
break;
94+
}
7195
}
7296

7397
/**
7498
* Invoked by the owner of this {@code FlutterImageView} when it no longer wants to render a
7599
* Flutter UI to this {@code FlutterImageView}.
76100
*/
77101
public void detachFromRenderer() {
78-
if (flutterRenderer != null) {
79-
flutterRenderer.stopRenderingToSurface();
80-
flutterRenderer = null;
102+
switch (kind) {
103+
case background:
104+
// TODO: Swap the surface back to the original one.
105+
// https://github.com/flutter/flutter/issues/58291
106+
break;
107+
case overlay:
108+
// TODO: Handle this in the native side.
109+
// https://github.com/flutter/flutter/issues/59904
110+
break;
81111
}
82112
}
83113

114+
public void pause() {
115+
// Not supported.
116+
}
117+
84118
/** Acquires the next image to be drawn to the {@link android.graphics.Canvas}. */
85119
@TargetApi(19)
86120
public void acquireLatestImage() {
87-
nextImage = imageReader.acquireLatestImage();
121+
// There's no guarantee that the image will be closed before the next call to
122+
// `acquireLatestImage()`. For example, the device may not produce new frames if
123+
// it's in sleep mode, so the calls to `invalidate()` will be queued up
124+
// until the device produces a new frame.
125+
//
126+
// While the engine will also stop producing frames, there is a race condition.
127+
//
128+
// To avoid exceptions, check if a new image can be acquired.
129+
if (pendingImages < imageReader.getMaxImages()) {
130+
nextImage = imageReader.acquireLatestImage();
131+
if (nextImage != null) {
132+
pendingImages++;
133+
}
134+
}
88135
invalidate();
89136
}
90137

@@ -94,6 +141,7 @@ protected void onDraw(Canvas canvas) {
94141
if (nextImage != null) {
95142
if (currentImage != null) {
96143
currentImage.close();
144+
pendingImages--;
97145
}
98146
currentImage = nextImage;
99147
nextImage = null;

shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ public void detachFromRenderer() {
200200
// Make the SurfaceView invisible to avoid showing a black rectangle.
201201
setAlpha(0.0f);
202202

203-
this.flutterRenderer.removeIsDisplayingFlutterUiListener(flutterUiDisplayListener);
203+
flutterRenderer.removeIsDisplayingFlutterUiListener(flutterUiDisplayListener);
204204

205205
flutterRenderer = null;
206206
isAttachedToFlutterRenderer = false;
@@ -209,6 +209,21 @@ public void detachFromRenderer() {
209209
}
210210
}
211211

212+
/**
213+
* Invoked by the owner of this {@code FlutterSurfaceView} when it should pause rendering Flutter
214+
* UI to this {@code FlutterSurfaceView}.
215+
*/
216+
public void pause() {
217+
if (flutterRenderer != null) {
218+
// Don't remove the `flutterUiDisplayListener` as `onFlutterUiDisplayed()` will make
219+
// the `FlutterSurfaceView` visible.
220+
flutterRenderer = null;
221+
isAttachedToFlutterRenderer = false;
222+
} else {
223+
Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
224+
}
225+
}
226+
212227
// FlutterRenderer and getSurfaceTexture() must both be non-null.
213228
private void connectSurfaceToRenderer() {
214229
if (flutterRenderer == null || getHolder() == null) {

shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,19 @@ public void detachFromRenderer() {
173173
}
174174
}
175175

176+
/**
177+
* Invoked by the owner of this {@code FlutterTextureView} when it should pause rendering Flutter
178+
* UI to this {@code FlutterTextureView}.
179+
*/
180+
public void pause() {
181+
if (flutterRenderer != null) {
182+
flutterRenderer = null;
183+
isAttachedToFlutterRenderer = false;
184+
} else {
185+
Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
186+
}
187+
}
188+
176189
// FlutterRenderer and getSurfaceTexture() must both be non-null.
177190
private void connectSurfaceToRenderer() {
178191
if (flutterRenderer == null || getSurfaceTexture() == null) {

shell/platform/android/io/flutter/embedding/android/FlutterView.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -943,16 +943,15 @@ public void detachFromFlutterEngine() {
943943
}
944944

945945
public void convertToImageView() {
946-
renderSurface.detachFromRenderer();
946+
renderSurface.pause();
947947

948948
ImageReader imageReader = PlatformViewsController.createImageReader(getWidth(), getHeight());
949-
flutterImageView = new FlutterImageView(getContext(), imageReader);
949+
flutterImageView =
950+
new FlutterImageView(getContext(), imageReader, FlutterImageView.SurfaceKind.background);
950951
renderSurface = flutterImageView;
951952
if (flutterEngine != null) {
952953
renderSurface.attachToRenderer(flutterEngine.getRenderer());
953954
}
954-
955-
removeAllViews();
956955
addView(flutterImageView);
957956
}
958957

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,23 @@ public void onSurfaceCreated(@NonNull Surface surface) {
313313

314314
private native void nativeSurfaceCreated(long nativePlatformViewId, @NonNull Surface surface);
315315

316+
/**
317+
* In hybrid composition, call this method when the {@link Surface} has changed.
318+
*
319+
* <p>In hybrid composition, the root surfaces changes from {@link
320+
* android.view.SurfaceHolder#getSurface()} to {@link android.media.ImageReader#getSurface()} when
321+
* a platform view is in the current frame.
322+
*/
323+
@UiThread
324+
public void onSurfaceWindowChanged(@NonNull Surface surface) {
325+
ensureRunningOnMainThread();
326+
ensureAttachedToNative();
327+
nativeSurfaceWindowChanged(nativePlatformViewId, surface);
328+
}
329+
330+
private native void nativeSurfaceWindowChanged(
331+
long nativePlatformViewId, @NonNull Surface surface);
332+
316333
/**
317334
* Call this method when the {@link Surface} changes that was previously registered with {@link
318335
* #onSurfaceCreated(Surface)}.

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,18 @@ public void startRenderingToSurface(@NonNull Surface surface) {
183183
flutterJNI.onSurfaceCreated(surface);
184184
}
185185

186+
/**
187+
* Swaps the {@link Surface} used to render the current frame.
188+
*
189+
* <p>In hybrid composition, the root surfaces changes from {@link
190+
* android.view.SurfaceHolder#getSurface()} to {@link android.media.ImageReader#getSurface()} when
191+
* a platform view is in the current frame.
192+
*/
193+
public void swapSurface(@NonNull Surface surface) {
194+
this.surface = surface;
195+
flutterJNI.onSurfaceWindowChanged(surface);
196+
}
197+
186198
/**
187199
* Notifies Flutter that a {@code surface} previously registered with {@link
188200
* #startRenderingToSurface(Surface)} has changed size to the given {@code width} and {@code

shell/platform/android/io/flutter/embedding/engine/renderer/RenderSurface.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,11 @@ public interface RenderSurface {
5353
* connected {@code FlutterRenderer}.
5454
*/
5555
void detachFromRenderer();
56+
57+
/**
58+
* Instructs this {@code RenderSurface} to stop forwarding {@code Surface} notifications to the
59+
* {@code FlutterRenderer} that was previously connected with {@link
60+
* #attachToRenderer(FlutterRenderer)}.
61+
*/
62+
void pause();
5663
}

shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega
5454
private Context context;
5555

5656
// The View currently rendering the Flutter UI associated with these platform views.
57+
// TODO(egarciad): Investigate if this can be downcasted to `FlutterView`.
5758
private View flutterView;
5859

5960
// The texture registry maintaining the textures into which the embedded views will be rendered.
@@ -557,20 +558,23 @@ private void flushAllViews() {
557558
vdControllers.clear();
558559
}
559560

561+
private void initializeRootImageViewIfNeeded() {
562+
if (!flutterViewConvertedToImageView) {
563+
((FlutterView) flutterView).convertToImageView();
564+
flutterViewConvertedToImageView = true;
565+
}
566+
}
567+
560568
public void onDisplayPlatformView(int viewId, int x, int y, int width, int height) {
569+
initializeRootImageViewIfNeeded();
561570
// TODO: Implement this method. https://github.com/flutter/flutter/issues/58288
562571
}
563572

564573
public void onDisplayOverlaySurface(int id, int x, int y, int width, int height) {
565-
FlutterView flutterView = (FlutterView) this.flutterView;
566-
if (!flutterViewConvertedToImageView) {
567-
flutterView.convertToImageView();
568-
flutterViewConvertedToImageView = true;
569-
}
570-
574+
initializeRootImageViewIfNeeded();
571575
FlutterImageView overlayView = overlayLayerViews.get(id);
572576
if (overlayView.getParent() == null) {
573-
flutterView.addView(overlayView);
577+
((FlutterView) flutterView).addView(overlayView);
574578
}
575579

576580
FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams((int) width, (int) height);
@@ -579,7 +583,6 @@ public void onDisplayOverlaySurface(int id, int x, int y, int width, int height)
579583
overlayView.setLayoutParams(layoutParams);
580584
overlayView.setVisibility(View.VISIBLE);
581585
overlayView.bringToFront();
582-
583586
currentFrameUsedOverlayLayerIds.add(id);
584587
}
585588

@@ -599,8 +602,7 @@ public void onEndFrame() {
599602
}
600603

601604
if (flutterViewConvertedToImageView) {
602-
FlutterView flutterView = (FlutterView) this.flutterView;
603-
flutterView.acquireLatestImageViewFrame();
605+
((FlutterView) flutterView).acquireLatestImageViewFrame();
604606
}
605607
}
606608

@@ -621,7 +623,9 @@ public static ImageReader createImageReader(int width, int height) {
621623
@TargetApi(19)
622624
public FlutterOverlaySurface createOverlaySurface() {
623625
ImageReader imageReader = createImageReader(flutterView.getWidth(), flutterView.getHeight());
624-
FlutterImageView imageView = new FlutterImageView(flutterView.getContext(), imageReader);
626+
FlutterImageView imageView =
627+
new FlutterImageView(
628+
flutterView.getContext(), imageReader, FlutterImageView.SurfaceKind.overlay);
625629
int id = nextOverlayLayerId++;
626630
overlayLayerViews.put(id, imageView);
627631

shell/platform/android/platform_view_android.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,22 @@ void PlatformViewAndroid::NotifyCreated(
104104
PlatformView::NotifyCreated();
105105
}
106106

107+
void PlatformViewAndroid::NotifySurfaceWindowChanged(
108+
fml::RefPtr<AndroidNativeWindow> native_window) {
109+
if (android_surface_) {
110+
fml::AutoResetWaitableEvent latch;
111+
fml::TaskRunner::RunNowOrPostTask(
112+
task_runners_.GetRasterTaskRunner(),
113+
[&latch, surface = android_surface_.get(),
114+
native_window = std::move(native_window)]() {
115+
surface->TeardownOnScreenContext();
116+
surface->SetNativeWindow(native_window);
117+
latch.Signal();
118+
});
119+
latch.Wait();
120+
}
121+
}
122+
107123
void PlatformViewAndroid::NotifyDestroyed() {
108124
PlatformView::NotifyDestroyed();
109125

shell/platform/android/platform_view_android.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ class PlatformViewAndroid final : public PlatformView {
4141

4242
void NotifyCreated(fml::RefPtr<AndroidNativeWindow> native_window);
4343

44+
void NotifySurfaceWindowChanged(
45+
fml::RefPtr<AndroidNativeWindow> native_window);
46+
4447
void NotifyChanged(const SkISize& size);
4548

4649
// |PlatformView|

0 commit comments

Comments
 (0)