Fix Google Maps rendering issues in TLHC mode when using LATEST renderer

packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md

@@ -1,5 +1,7 @@
-## NEXT
+## 2.6.0
 
+* Fixes missing updates in TLHC mode.
+* Switched default display mode to TLHC mode.
 * Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
 
 ## 2.5.3

packages/google_maps_flutter/google_maps_flutter_android/README.md

@@ -30,26 +30,26 @@ void main() {
   final GoogleMapsFlutterPlatform mapsImplementation =
       GoogleMapsFlutterPlatform.instance;
   if (mapsImplementation is GoogleMapsFlutterAndroid) {
+    // Force Hybrid Composition mode.
     mapsImplementation.useAndroidViewSurface = true;
   }
   // ···
 }
 ```
 
-### Hybrid Composition
+### Texture Layer Hybrid Composition
 
-This is the current default mode, and corresponds to
-`useAndroidViewSurface = true`. It ensures that the map display will work as
-expected, at the cost of some performance.
+This is the the current default mode and corresponds to `useAndroidViewSurface = false`.
+This mode is more performant than Hybrid Composition and we recommend that you use this mode.
 
-### Texture Layer Hybrid Composition
+### Hybrid Composition
 
-This is a new display mode used by most plugins starting with Flutter 3.0, and
-corresponds to `useAndroidViewSurface = false`. This is more performant than
-Hybrid Composition, but currently [misses certain map updates][4].
+This mode is available for backwards compatability and corresponds to `useAndroidViewSurface = true`.
+We do not recommend its use as it is less performant than Texture Layer Hybrid Composition and
+certain flutter rendering effects are not supported. 
 
-This mode will likely become the default in future versions if/when the
-missed updates issue can be resolved.
+If you require this mode for correctness, please file a bug so we can investigate and fix
+the issue in the TLHC mode.
 
 ## Map renderer
 
@@ -70,8 +70,13 @@ AndroidMapRenderer mapRenderer = AndroidMapRenderer.platformDefault;
   }
 ```
 
-Available values are `AndroidMapRenderer.latest`, `AndroidMapRenderer.legacy`, `AndroidMapRenderer.platformDefault`.
-Note that getting the requested renderer as a response is not guaranteed.
+`AndroidMapRenderer.platformDefault` corresponds to `AndroidMapRenderer.latest`.
+
+You are not guaranteed to get the requested renderer. For example, on emulators without
+Google Play the latest renderer will not be available and the legacy renderer will always be used.
+
+WARNING: `AndroidMapRenderer.legacy` is known to crash apps and is no longer supported by the Google Maps team
+and therefore cannot be supported by the Flutter team.
 
 [1]: https://pub.dev/packages/google_maps_flutter
 [2]: https://flutter.dev/docs/development/packages-and-plugins/developing-packages#endorsed-federated-plugin

packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java

@@ -10,10 +10,13 @@
 import android.content.pm.PackageManager;
 import android.graphics.Bitmap;
 import android.graphics.Point;
+import android.graphics.SurfaceTexture;
 import android.os.Bundle;
 import android.util.Log;
-import android.view.Choreographer;
+import android.view.TextureView;
+import android.view.TextureView.SurfaceTextureListener;
 import android.view.View;
+import android.view.ViewGroup;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
@@ -135,61 +138,13 @@ private CameraPosition getCameraPosition() {
     return trackCameraPosition ? googleMap.getCameraPosition() : null;
   }
 
-  private boolean loadedCallbackPending = false;
-
-  /**
-   * Invalidates the map view after the map has finished rendering.
-   *
-   * <p>gmscore GL renderer uses a {@link android.view.TextureView}. Android platform views that are
-   * displayed as a texture after Flutter v3.0.0. require that the view hierarchy is notified after
-   * all drawing operations have been flushed.
-   *
-   * <p>Since the GL renderer doesn't use standard Android views, and instead uses GL directly, we
-   * notify the view hierarchy by invalidating the view.
-   *
-   * <p>Unfortunately, when {@link GoogleMap.OnMapLoadedCallback} is fired, the texture may not have
-   * been updated yet.
-   *
-   * <p>To workaround this limitation, wait two frames. This ensures that at least the frame budget
-   * (16.66ms at 60hz) have passed since the drawing operation was issued.
-   */
-  private void invalidateMapIfNeeded() {
-    if (googleMap == null || loadedCallbackPending) {
-      return;
-    }
-    loadedCallbackPending = true;
-    googleMap.setOnMapLoadedCallback(
-        () -> {
-          loadedCallbackPending = false;
-          postFrameCallback(
-              () -> {
-                postFrameCallback(
-                    () -> {
-                      if (mapView != null) {
-                        mapView.invalidate();
-                      }
-                    });
-              });
-        });
-  }
-
-  private static void postFrameCallback(Runnable f) {
-    Choreographer.getInstance()
-        .postFrameCallback(
-            new Choreographer.FrameCallback() {
-              @Override
-              public void doFrame(long frameTimeNanos) {
-                f.run();
-              }
-            });
-  }
-
   @Override
   public void onMapReady(GoogleMap googleMap) {
     this.googleMap = googleMap;
     this.googleMap.setIndoorEnabled(this.indoorEnabled);
     this.googleMap.setTrafficEnabled(this.trafficEnabled);
     this.googleMap.setBuildingsEnabled(this.buildingsEnabled);
+    installInvalidator();
     googleMap.setOnInfoWindowClickListener(this);
     if (mapReadyResult != null) {
       mapReadyResult.success(null);
@@ -216,6 +171,71 @@ public void onMapReady(GoogleMap googleMap) {
     }
   }
 
+  // Returns the first TextureView found in the view hierarchy.
+  private static TextureView findTextureView(ViewGroup group) {
+    final int n = group.getChildCount();
+    for (int i = 0; i < n; i++) {
+      View view = group.getChildAt(i);
+      if (view instanceof TextureView) {
+        return (TextureView) view;
+      }
+      if (view instanceof ViewGroup) {
+        TextureView r = findTextureView((ViewGroup) view);
+        if (r != null) {
+          return r;
+        }
+      }
+    }
+    return null;
+  }
+
+  private void installInvalidator() {
+    if (mapView == null) {
+      // This should only happen in tests.
+      return;
+    }
+    TextureView textureView = findTextureView(mapView);
+    if (textureView == null) {
+      Log.i(TAG, "No TextureView found. Likely using the LEGACY renderer.");
+      return;
+    }
+    Log.i(TAG, "Installing custom TextureView driven invalidator.");
+    SurfaceTextureListener internalListener = textureView.getSurfaceTextureListener();
+    // Override the Maps internal SurfaceTextureListener with our own. Our listener
+    // mostly just invokes the internal listener callbacks but in onSurfaceTextureUpdated
+    // the mapView is invalidated which ensures that all map updates are presented to the
+    // screen.
+    final MapView mapView = this.mapView;
+    textureView.setSurfaceTextureListener(
+        new TextureView.SurfaceTextureListener() {
+          public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int height) {
+            if (internalListener != null) {
+              internalListener.onSurfaceTextureAvailable(surface, width, height);
+            }
+          }
+
+          public boolean onSurfaceTextureDestroyed(SurfaceTexture surface) {
+            if (internalListener != null) {
+              return internalListener.onSurfaceTextureDestroyed(surface);
+            }
+            return true;
+          }
+
+          public void onSurfaceTextureSizeChanged(SurfaceTexture surface, int width, int height) {
+            if (internalListener != null) {
+              internalListener.onSurfaceTextureSizeChanged(surface, width, height);
+            }
+          }
+
+          public void onSurfaceTextureUpdated(SurfaceTexture surface) {
+            if (internalListener != null) {
+              internalListener.onSurfaceTextureUpdated(surface);
+            }
+            mapView.invalidate();
+          }
+        });
+  }
+
   @Override
   public void onMethodCall(MethodCall call, MethodChannel.Result result) {
     switch (call.method) {
@@ -309,7 +329,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "markers#update":
         {
-          invalidateMapIfNeeded();
           List<Object> markersToAdd = call.argument("markersToAdd");
           markersController.addMarkers(markersToAdd);
           List<Object> markersToChange = call.argument("markersToChange");
@@ -339,7 +358,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "polygons#update":
         {
-          invalidateMapIfNeeded();
           List<Object> polygonsToAdd = call.argument("polygonsToAdd");
           polygonsController.addPolygons(polygonsToAdd);
           List<Object> polygonsToChange = call.argument("polygonsToChange");
@@ -351,7 +369,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "polylines#update":
         {
-          invalidateMapIfNeeded();
           List<Object> polylinesToAdd = call.argument("polylinesToAdd");
           polylinesController.addPolylines(polylinesToAdd);
           List<Object> polylinesToChange = call.argument("polylinesToChange");
@@ -363,7 +380,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "circles#update":
         {
-          invalidateMapIfNeeded();
           List<Object> circlesToAdd = call.argument("circlesToAdd");
           circlesController.addCircles(circlesToAdd);
           List<Object> circlesToChange = call.argument("circlesToChange");
@@ -443,7 +459,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "map#setStyle":
         {
-          invalidateMapIfNeeded();
           boolean mapStyleSet;
           if (call.arguments instanceof String) {
             String mapStyle = (String) call.arguments;
@@ -466,7 +481,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "tileOverlays#update":
         {
-          invalidateMapIfNeeded();
           List<Map<String, ?>> tileOverlaysToAdd = call.argument("tileOverlaysToAdd");
           tileOverlaysController.addTileOverlays(tileOverlaysToAdd);
           List<Map<String, ?>> tileOverlaysToChange = call.argument("tileOverlaysToChange");
@@ -478,7 +492,6 @@ public void onSnapshotReady(Bitmap bitmap) {
         }
       case "tileOverlays#clearTileCache":
         {
-          invalidateMapIfNeeded();
           String tileOverlayId = call.argument("tileOverlayId");
           tileOverlaysController.clearTileCache(tileOverlayId);
           result.success(null);

packages/google_maps_flutter/google_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/GoogleMapControllerTest.java

@@ -7,33 +7,24 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-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 android.content.Context;
 import android.os.Build;
-import android.os.Looper;
 import androidx.activity.ComponentActivity;
 import androidx.test.core.app.ApplicationProvider;
 import com.google.android.gms.maps.GoogleMap;
-import com.google.android.gms.maps.MapView;
 import io.flutter.plugin.common.BinaryMessenger;
-import io.flutter.plugin.common.MethodCall;
-import io.flutter.plugin.common.MethodChannel;
-import java.util.HashMap;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
-import org.robolectric.Shadows;
 import org.robolectric.annotation.Config;
 
 @RunWith(RobolectricTestRunner.class)
@@ -86,87 +77,6 @@ public void OnDestroyReleaseTheMap() throws InterruptedException {
     assertNull(googleMapController.getView());
   }
 
-  @Test
-  public void InvalidateMapAfterMethodCalls() throws InterruptedException {
-    String[] methodsThatTriggerInvalidation = {
-      "markers#update",
-      "polygons#update",
-      "polylines#update",
-      "circles#update",
-      "map#setStyle",
-      "tileOverlays#update",
-      "tileOverlays#clearTileCache"
-    };
-
-    for (String methodName : methodsThatTriggerInvalidation) {
-      googleMapController =
-          new GoogleMapController(0, context, mockMessenger, activity::getLifecycle, null);
-      googleMapController.init();
-
-      mockGoogleMap = mock(GoogleMap.class);
-      googleMapController.onMapReady(mockGoogleMap);
-
-      MethodChannel.Result result = mock(MethodChannel.Result.class);
-      System.out.println(methodName);
-      googleMapController.onMethodCall(
-          new MethodCall(methodName, new HashMap<String, Object>()), result);
-
-      ArgumentCaptor<GoogleMap.OnMapLoadedCallback> argument =
-          ArgumentCaptor.forClass(GoogleMap.OnMapLoadedCallback.class);
-      verify(mockGoogleMap).setOnMapLoadedCallback(argument.capture());
-
-      MapView mapView = mock(MapView.class);
-      googleMapController.setView(mapView);
-
-      verify(mapView, never()).invalidate();
-      argument.getValue().onMapLoaded();
-      Shadows.shadowOf(Looper.getMainLooper()).idle();
-      verify(mapView).invalidate();
-    }
-  }
-
-  @Test
-  public void InvalidateMapOnceAfterMethodCall() throws InterruptedException {
-    googleMapController.onMapReady(mockGoogleMap);
-
-    MethodChannel.Result result = mock(MethodChannel.Result.class);
-    googleMapController.onMethodCall(
-        new MethodCall("markers#update", new HashMap<String, Object>()), result);
-    googleMapController.onMethodCall(
-        new MethodCall("polygons#update", new HashMap<String, Object>()), result);
-
-    ArgumentCaptor<GoogleMap.OnMapLoadedCallback> argument =
-        ArgumentCaptor.forClass(GoogleMap.OnMapLoadedCallback.class);
-    verify(mockGoogleMap).setOnMapLoadedCallback(argument.capture());
-
-    MapView mapView = mock(MapView.class);
-    googleMapController.setView(mapView);
-
-    verify(mapView, never()).invalidate();
-    argument.getValue().onMapLoaded();
-    Shadows.shadowOf(Looper.getMainLooper()).idle();
-    verify(mapView).invalidate();
-  }
-
-  @Test
-  public void MethodCalledAfterControllerIsDestroyed() throws InterruptedException {
-    googleMapController.onMapReady(mockGoogleMap);
-    MethodChannel.Result result = mock(MethodChannel.Result.class);
-    googleMapController.onMethodCall(
-        new MethodCall("markers#update", new HashMap<String, Object>()), result);
-
-    ArgumentCaptor<GoogleMap.OnMapLoadedCallback> argument =
-        ArgumentCaptor.forClass(GoogleMap.OnMapLoadedCallback.class);
-    verify(mockGoogleMap).setOnMapLoadedCallback(argument.capture());
-
-    MapView mapView = mock(MapView.class);
-    googleMapController.setView(mapView);
-    googleMapController.onDestroy(activity);
-
-    argument.getValue().onMapLoaded();
-    verify(mapView, never()).invalidate();
-  }
-
   @Test
   public void OnMapReadySetsPaddingIfInitialPaddingIsThere() {
     float padding = 10f;

packages/google_maps_flutter/google_maps_flutter_android/example/lib/readme_excerpts.dart

@@ -14,6 +14,7 @@ void main() {
   final GoogleMapsFlutterPlatform mapsImplementation =
       GoogleMapsFlutterPlatform.instance;
   if (mapsImplementation is GoogleMapsFlutterAndroid) {
+    // Force Hybrid Composition mode.
     mapsImplementation.useAndroidViewSurface = true;
   }
   // #enddocregion DisplayMode