Remove glitch when displaying platform views (#30724)

Author: Emmanuel Garcia

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

@@ -37,6 +37,7 @@ public class FlutterSurfaceView extends SurfaceView implements RenderSurface {
 
   private final boolean renderTransparently;
   private boolean isSurfaceAvailableForRendering = false;
+  private boolean isPaused = false;
   private boolean isAttachedToFlutterRenderer = false;
   @Nullable private FlutterRenderer flutterRenderer;
 
@@ -200,6 +201,7 @@ public void attachToRenderer(@NonNull FlutterRenderer flutterRenderer) {
           "Surface is available for rendering. Connecting FlutterRenderer to Android surface.");
       connectSurfaceToRenderer();
     }
+    isPaused = false;
   }
 
   /**
@@ -241,6 +243,7 @@ public void pause() {
       // Don't remove the `flutterUiDisplayListener` as `onFlutterUiDisplayed()` will make
       // the `FlutterSurfaceView` visible.
       flutterRenderer = null;
+      isPaused = true;
       isAttachedToFlutterRenderer = false;
     } else {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
@@ -253,8 +256,13 @@ private void connectSurfaceToRenderer() {
       throw new IllegalStateException(
           "connectSurfaceToRenderer() should only be called when flutterRenderer and getHolder() are non-null.");
     }
-
-    flutterRenderer.startRenderingToSurface(getHolder().getSurface());
+    // When connecting the surface to the renderer, it's possible that the surface is currently
+    // paused. For instance, when a platform view is displayed, the current FlutterSurfaceView
+    // is paused, and rendering continues in a FlutterImageView buffer while the platform view
+    // is displayed.
+    //
+    // startRenderingToSurface stops rendering to an active surface if it isn't paused.
+    flutterRenderer.startRenderingToSurface(getHolder().getSurface(), isPaused);
   }
 
   // FlutterRenderer must be non-null.

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

@@ -36,6 +36,7 @@ public class FlutterTextureView extends TextureView implements RenderSurface {
 
   private boolean isSurfaceAvailableForRendering = false;
   private boolean isAttachedToFlutterRenderer = false;
+  private boolean isPaused = false;
   @Nullable private FlutterRenderer flutterRenderer;
   @Nullable private Surface renderSurface;
 
@@ -187,6 +188,7 @@ public void detachFromRenderer() {
   public void pause() {
     if (flutterRenderer != null) {
       flutterRenderer = null;
+      isPaused = true;
       isAttachedToFlutterRenderer = false;
     } else {
       Log.w(TAG, "pause() invoked when no FlutterRenderer was attached.");
@@ -217,7 +219,8 @@ private void connectSurfaceToRenderer() {
     }
 
     renderSurface = new Surface(getSurfaceTexture());
-    flutterRenderer.startRenderingToSurface(renderSurface);
+    flutterRenderer.startRenderingToSurface(renderSurface, isPaused);
+    isPaused = false;
   }
 
   // FlutterRenderer must be non-null.

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

@@ -134,7 +134,7 @@ public void onEngineWillDestroy() {
    *
    * <p>A new {@code FlutterEngine} will not display any UI until a {@link RenderSurface} is
    * registered. See {@link #getRenderer()} and {@link
-   * FlutterRenderer#startRenderingToSurface(Surface)}.
+   * FlutterRenderer#startRenderingToSurface(Surface, boolean)}.
    *
    * <p>A new {@code FlutterEngine} automatically attaches all plugins. See {@link #getPlugins()}.
    *

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

@@ -221,11 +221,24 @@ public void run() {
    * Notifies Flutter that the given {@code surface} was created and is available for Flutter
    * rendering.
    *
+   * <p>If called more than once, the current native resources are released. This can be undesired
+   * if the Engine expects to reuse this surface later. For example, this is true when platform
+   * views are displayed in a frame, and then removed in the next frame.
+   *
+   * <p>To avoid releasing the current surface resources, set {@code keepCurrentSurface} to true.
+   *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}
+   *
+   * @param surface The render surface.
+   * @param keepCurrentSurface True if the current active surface should not be released.
    */
-  public void startRenderingToSurface(@NonNull Surface surface) {
-    if (this.surface != null) {
+  public void startRenderingToSurface(@NonNull Surface surface, boolean keepCurrentSurface) {
+    // Don't stop rendering the surface if it's currently paused.
+    // Stop rendering to the surface releases the associated native resources, which
+    // causes a glitch when showing platform views.
+    // For more, https://github.com/flutter/flutter/issues/95343
+    if (this.surface != null && !keepCurrentSurface) {
       stopRenderingToSurface();
     }
 
@@ -248,8 +261,8 @@ public void swapSurface(@NonNull Surface surface) {
 
   /**
    * Notifies Flutter that a {@code surface} previously registered with {@link
-   * #startRenderingToSurface(Surface)} has changed size to the given {@code width} and {@code
-   * height}.
+   * #startRenderingToSurface(Surface, boolean)} has changed size to the given {@code width} and
+   * {@code height}.
    *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}
@@ -260,8 +273,8 @@ public void surfaceChanged(int width, int height) {
 
   /**
    * Notifies Flutter that a {@code surface} previously registered with {@link
-   * #startRenderingToSurface(Surface)} has been destroyed and needs to be released and cleaned up
-   * on the Flutter side.
+   * #startRenderingToSurface(Surface, boolean)} has been destroyed and needs to be released and
+   * cleaned up on the Flutter side.
    *
    * <p>See {@link android.view.SurfaceHolder.Callback} and {@link
    * android.view.TextureView.SurfaceTextureListener}

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

@@ -37,7 +37,7 @@ public interface RenderSurface {
    * FlutterRenderer} at the appropriate times:
    *
    * <ol>
-   *   <li>{@link FlutterRenderer#startRenderingToSurface(Surface)}
+   *   <li>{@link FlutterRenderer#startRenderingToSurface(Surface, boolean)}
    *   <li>{@link FlutterRenderer#surfaceChanged(int, int)}}
    *   <li>{@link FlutterRenderer#stopRenderingToSurface()}
    * </ol>

shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java

@@ -6,6 +6,7 @@
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -45,7 +46,7 @@ public void itForwardsSurfaceCreationNotificationToFlutterJNI() {
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
     // Execute the behavior under test.
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Verify the behavior under test.
     verify(fakeFlutterJNI, times(1)).onSurfaceCreated(eq(fakeSurface));
@@ -57,7 +58,7 @@ public void itForwardsSurfaceChangeNotificationToFlutterJNI() {
     Surface fakeSurface = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.surfaceChanged(100, 50);
@@ -72,7 +73,7 @@ public void itForwardsSurfaceDestructionNotificationToFlutterJNI() {
     Surface fakeSurface = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -87,10 +88,10 @@ public void itStopsRenderingToOneSurfaceBeforeRenderingToANewSurface() {
     Surface fakeSurface2 = mock(Surface.class);
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute behavior under test.
-    flutterRenderer.startRenderingToSurface(fakeSurface2);
+    flutterRenderer.startRenderingToSurface(fakeSurface2, /*keepCurrentSurface=*/ false);
 
     // Verify behavior under test.
     verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed(); // notification of 1st surface's removal.
@@ -101,7 +102,7 @@ public void itStopsRenderingToSurfaceWhenRequested() {
     // Setup the test.
     FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -110,6 +111,32 @@ public void itStopsRenderingToSurfaceWhenRequested() {
     verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed();
   }
 
+  @Test
+  public void iStopsRenderingToSurfaceWhenSurfaceAlreadySet() {
+    // Setup the test.
+    FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    // Verify behavior under test.
+    verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed();
+  }
+
+  @Test
+  public void itNeverStopsRenderingToSurfaceWhenRequested() {
+    // Setup the test.
+    FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
+
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ true);
+
+    // Verify behavior under test.
+    verify(fakeFlutterJNI, never()).onSurfaceDestroyed();
+  }
+
   @Test
   public void itStopsSurfaceTextureCallbackWhenDetached() {
     // Setup the test.
@@ -120,7 +147,7 @@ public void itStopsSurfaceTextureCallbackWhenDetached() {
     FlutterRenderer.SurfaceTextureRegistryEntry entry =
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     flutterRenderer.stopRenderingToSurface();
@@ -143,7 +170,7 @@ public void itRegistersExistingSurfaceTexture() {
         (FlutterRenderer.SurfaceTextureRegistryEntry)
             flutterRenderer.registerSurfaceTexture(surfaceTexture);
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Verify behavior under test.
     assertEquals(surfaceTexture, entry.surfaceTexture());
@@ -164,7 +191,7 @@ public void itUnregistersTextureWhenSurfaceTextureFinalized() {
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
     long id = entry.id();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     // Execute the behavior under test.
     runFinalization(entry);
@@ -190,7 +217,7 @@ public void itStopsUnregisteringTextureWhenDetached() {
         (FlutterRenderer.SurfaceTextureRegistryEntry) flutterRenderer.createSurfaceTexture();
     long id = entry.id();
 
-    flutterRenderer.startRenderingToSurface(fakeSurface);
+    flutterRenderer.startRenderingToSurface(fakeSurface, /*keepCurrentSurface=*/ false);
 
     flutterRenderer.stopRenderingToSurface();
 

shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java

@@ -1,9 +1,11 @@
 package io.flutter.plugin.platform;
 
+import static android.os.Looper.getMainLooper;
 import static io.flutter.embedding.engine.systemchannels.PlatformViewsChannel.PlatformViewTouch;
 import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
+import static org.robolectric.Shadows.shadowOf;
 
 import android.content.Context;
 import android.content.res.AssetManager;
@@ -486,10 +488,7 @@ public void onEndFrame__destroysOverlaySurfaceAfterFrameOnFlutterSurfaceView() {
     platformViewsController.onBeginFrame();
     platformViewsController.onEndFrame();
 
-    verify(overlayImageView, never()).detachFromRenderer();
-
-    // Simulate first frame from the framework.
-    jni.onFirstFrame();
+    shadowOf(getMainLooper()).idle();
     verify(overlayImageView, times(1)).detachFromRenderer();
   }