diff --git a/packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md b/packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md index 21b41d813f9b..efba27003f8b 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md +++ b/packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.18.13 + +* Batches clustered marker add/remove operations to avoid redundant re-rendering. + ## 2.18.12 * Bumps com.google.maps.android:android-maps-utils from 3.20.1 to 4.0.0. @@ -23,6 +27,7 @@ * Replaces internal use of deprecated methods. ## 2.18.6 + * Bumps com.android.tools.build:gradle from 8.12.1 to 8.13.1. ## 2.18.5 @@ -48,7 +53,7 @@ ## 2.18.0 -* Adds support for warming up the Google Maps SDK +* Adds support for warming up the Google Maps SDK via `GoogleMapsFlutterAndroid.warmup()`. ## 2.17.0 diff --git a/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/ClusterManagersController.java b/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/ClusterManagersController.java index 60832bf5a1e4..2fc5f1344f58 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/ClusterManagersController.java +++ b/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/ClusterManagersController.java @@ -130,6 +130,15 @@ public void addItem(MarkerBuilder item) { } } + /** Adds multiple items to the ClusterManager with the given ID. */ + public void addItems(String clusterManagerId, List items) { + ClusterManager clusterManager = clusterManagerIdToManager.get(clusterManagerId); + if (clusterManager != null) { + clusterManager.addItems(items); + clusterManager.cluster(); + } + } + /** Removes item from the ClusterManager it belongs to. */ public void removeItem(MarkerBuilder item) { ClusterManager clusterManager = @@ -140,6 +149,15 @@ public void removeItem(MarkerBuilder item) { } } + /** Removes multiple items from the ClusterManager with the given ID. */ + public void removeItems(String clusterManagerId, List items) { + ClusterManager clusterManager = clusterManagerIdToManager.get(clusterManagerId); + if (clusterManager != null) { + clusterManager.removeItems(items); + clusterManager.cluster(); + } + } + /** Called when ClusterRenderer has rendered new visible marker to the map. */ void onClusterItemRendered(@NonNull MarkerBuilder item, @NonNull Marker marker) { // If map is being disposed, clusterItemRenderedListener might have been cleared and diff --git a/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/MarkersController.java b/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/MarkersController.java index 66c06a4ebb5c..0207e015d1d2 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/MarkersController.java +++ b/packages/google_maps_flutter/google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/MarkersController.java @@ -11,8 +11,10 @@ import com.google.android.gms.maps.model.MarkerOptions; import com.google.maps.android.collections.MarkerManager; import io.flutter.plugins.googlemaps.Messages.MapsCallbackApi; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; class MarkersController { @@ -47,20 +49,161 @@ void setCollection(MarkerManager.Collection markerCollection) { } void addMarkers(@NonNull List markersToAdd) { + // Group markers by cluster manager ID for batch operations + Map> markersByCluster = new HashMap<>(); + List nonClusteredMarkers = new ArrayList<>(); + for (Messages.PlatformMarker markerToAdd : markersToAdd) { - addMarker(markerToAdd); + String markerId = markerToAdd.getMarkerId(); + String clusterManagerId = markerToAdd.getClusterManagerId(); + MarkerBuilder markerBuilder = new MarkerBuilder(markerId, clusterManagerId); + Convert.interpretMarkerOptions( + markerToAdd, markerBuilder, assetManager, density, bitmapDescriptorFactoryWrapper); + + // Store marker builder for future marker rebuilds when used under clusters. + markerIdToMarkerBuilder.put(markerId, markerBuilder); + + if (clusterManagerId == null) { + nonClusteredMarkers.add(markerBuilder); + } else { + markersByCluster + .computeIfAbsent(clusterManagerId, k -> new ArrayList<>()) + .add(markerBuilder); + } + } + + // Add non-clustered markers to the collection + for (MarkerBuilder markerBuilder : nonClusteredMarkers) { + addMarkerToCollection(markerBuilder.markerId(), markerBuilder); + } + + // Batch add clustered markers + for (Map.Entry> entry : markersByCluster.entrySet()) { + clusterManagersController.addItems(entry.getKey(), entry.getValue()); } } void changeMarkers(@NonNull List markersToChange) { + // Collect markers that need cluster manager changes for batch processing + Map> markersToAddByCluster = new HashMap<>(); + Map> markersToRemoveByCluster = new HashMap<>(); + for (Messages.PlatformMarker markerToChange : markersToChange) { - changeMarker(markerToChange); + String markerId = markerToChange.getMarkerId(); + MarkerBuilder markerBuilder = markerIdToMarkerBuilder.get(markerId); + if (markerBuilder == null) { + continue; + } + + String clusterManagerId = markerToChange.getClusterManagerId(); + String oldClusterManagerId = markerBuilder.clusterManagerId(); + + // If the cluster ID on the updated marker has changed, collect for batch processing + if (!(Objects.equals(clusterManagerId, oldClusterManagerId))) { + // Remove from old cluster manager + if (oldClusterManagerId != null) { + markersToRemoveByCluster + .computeIfAbsent(oldClusterManagerId, k -> new ArrayList<>()) + .add(markerBuilder); + } + + // Prepare new marker for addition + MarkerBuilder newMarkerBuilder = new MarkerBuilder(markerId, clusterManagerId); + Convert.interpretMarkerOptions( + markerToChange, + newMarkerBuilder, + assetManager, + density, + bitmapDescriptorFactoryWrapper); + markerIdToMarkerBuilder.put(markerId, newMarkerBuilder); + + if (clusterManagerId != null) { + markersToAddByCluster + .computeIfAbsent(clusterManagerId, k -> new ArrayList<>()) + .add(newMarkerBuilder); + } else { + // Add to map immediately if not clustered + addMarkerToCollection(markerId, newMarkerBuilder); + } + + // Clean up old marker controller if it's not clustered + if (oldClusterManagerId == null) { + MarkerController oldController = markerIdToController.remove(markerId); + if (oldController != null && markerCollection != null) { + oldController.removeFromCollection(markerCollection); + googleMapsMarkerIdToDartMarkerId.remove(oldController.getGoogleMapsMarkerId()); + } + } + } else { + // Update existing marker in place + Convert.interpretMarkerOptions( + markerToChange, markerBuilder, assetManager, density, bitmapDescriptorFactoryWrapper); + MarkerController markerController = markerIdToController.get(markerId); + if (markerController != null) { + Convert.interpretMarkerOptions( + markerToChange, + markerController, + assetManager, + density, + bitmapDescriptorFactoryWrapper); + } + } + } + + // Batch remove from cluster managers + for (Map.Entry> entry : markersToRemoveByCluster.entrySet()) { + clusterManagersController.removeItems(entry.getKey(), entry.getValue()); + } + + // Batch add to cluster managers + for (Map.Entry> entry : markersToAddByCluster.entrySet()) { + clusterManagersController.addItems(entry.getKey(), entry.getValue()); } } void removeMarkers(@NonNull List markerIdsToRemove) { + // Group markers by cluster manager ID for batch operations + Map> markersByCluster = new HashMap<>(); + List nonClusteredControllers = new ArrayList<>(); + for (String markerId : markerIdsToRemove) { - removeMarker(markerId); + final MarkerBuilder markerBuilder = markerIdToMarkerBuilder.get(markerId); + if (markerBuilder == null) { + continue; + } + + final String clusterManagerId = markerBuilder.clusterManagerId(); + if (clusterManagerId != null) { + markersByCluster + .computeIfAbsent(clusterManagerId, k -> new ArrayList<>()) + .add(markerBuilder); + } else { + final MarkerController markerController = markerIdToController.get(markerId); + if (markerController != null) { + nonClusteredControllers.add(markerController); + } + } + } + + // Batch remove clustered markers + for (Map.Entry> entry : markersByCluster.entrySet()) { + clusterManagersController.removeItems(entry.getKey(), entry.getValue()); + } + + // Remove non-clustered markers from the collection + for (MarkerController markerController : nonClusteredControllers) { + if (this.markerCollection != null) { + markerController.removeFromCollection(markerCollection); + } + } + + // Clean up all marker references + for (String markerId : markerIdsToRemove) { + markerIdToMarkerBuilder.remove(markerId); + final MarkerController markerController = markerIdToController.remove(markerId); + if (markerController != null) { + googleMapsMarkerIdToDartMarkerId.remove(markerController.getGoogleMapsMarkerId()); + } } } diff --git a/packages/google_maps_flutter/google_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java b/packages/google_maps_flutter/google_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java index 7b5e0dae851a..b169deea14e7 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java +++ b/packages/google_maps_flutter/google_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java @@ -30,7 +30,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -199,15 +198,27 @@ public void controller_AddChangeAndRemoveMarkerWithClusterManagerId() { when(marker.getId()).thenReturn(googleMarkerId); - // Add marker and capture the markerBuilder + // Store reference to verify later, since markerIdToMarkerBuilder is private + final MarkerBuilder[] addedMarkerBuilder = new MarkerBuilder[1]; + + // Add marker and verify addItems is called with correct parameters controller.addMarkers(Collections.singletonList(builder.build())); - ArgumentCaptor captor = ArgumentCaptor.forClass(MarkerBuilder.class); - Mockito.verify(clusterManagersController, times(1)).addItem(captor.capture()); - MarkerBuilder capturedMarkerBuilder = captor.getValue(); - assertEquals(clusterManagerId, capturedMarkerBuilder.clusterManagerId()); + Mockito.verify(clusterManagersController, times(1)) + .addItems( + eq(clusterManagerId), + Mockito.argThat( + markerBuilders -> { + if (markerBuilders.size() == 1 + && markerBuilders.get(0).clusterManagerId().equals(clusterManagerId)) { + // Store reference for later use in onClusterItemRendered + addedMarkerBuilder[0] = markerBuilders.get(0); + return true; + } + return false; + })); // clusterManagersController calls onClusterItemRendered with created marker. - controller.onClusterItemRendered(capturedMarkerBuilder, marker); + controller.onClusterItemRendered(addedMarkerBuilder[0], marker); // Change marker to test that markerController is created and the marker can be updated final LatLng latLng2 = new LatLng(3.3, 4.4); @@ -226,9 +237,12 @@ public void controller_AddChangeAndRemoveMarkerWithClusterManagerId() { controller.removeMarkers(Collections.singletonList(googleMarkerId)); Mockito.verify(clusterManagersController, times(1)) - .removeItem( + .removeItems( + eq(clusterManagerId), Mockito.argThat( - markerBuilder -> markerBuilder.clusterManagerId().equals(clusterManagerId))); + markerBuilders -> + markerBuilders.size() == 1 + && markerBuilders.get(0).clusterManagerId().equals(clusterManagerId))); } @Test @@ -266,4 +280,150 @@ public void controller_AddChangeAndRemoveMarkerWithoutClusterManagerId() { Mockito.verify(spyMarkerCollection, times(1)).remove(marker); } + + @Test + public void controller_BatchAddMultipleMarkersWithClusterManagerId() { + final String clusterManagerId = "cm123"; + + // Create multiple markers with the same cluster manager + final List markers = new java.util.ArrayList<>(); + for (int i = 0; i < 5; i++) { + final Messages.PlatformMarker.Builder builder = defaultMarkerBuilder(); + builder + .setMarkerId("marker" + i) + .setClusterManagerId(clusterManagerId) + .setPosition( + new Messages.PlatformLatLng.Builder() + .setLatitude(1.0 + i) + .setLongitude(2.0 + i) + .build()); + markers.add(builder.build()); + } + + // Add all markers in one batch + controller.addMarkers(markers); + + // Verify addItems is called exactly once with all 5 markers + Mockito.verify(clusterManagersController, times(1)) + .addItems( + eq(clusterManagerId), + Mockito.argThat( + markerBuilders -> + markerBuilders.size() == 5 + && markerBuilders + .stream() + .allMatch(mb -> mb.clusterManagerId().equals(clusterManagerId)))); + + // Verify addItem is never called (we're using batch operation) + Mockito.verify(clusterManagersController, times(0)).addItem(any()); + } + + @Test + public void controller_BatchRemoveMultipleMarkersWithClusterManagerId() { + final String clusterManagerId = "cm123"; + + // First add markers + final List markers = new java.util.ArrayList<>(); + final List markerIds = new java.util.ArrayList<>(); + for (int i = 0; i < 5; i++) { + String markerId = "marker" + i; + markerIds.add(markerId); + final Messages.PlatformMarker.Builder builder = defaultMarkerBuilder(); + builder + .setMarkerId(markerId) + .setClusterManagerId(clusterManagerId) + .setPosition( + new Messages.PlatformLatLng.Builder() + .setLatitude(1.0 + i) + .setLongitude(2.0 + i) + .build()); + markers.add(builder.build()); + } + + controller.addMarkers(markers); + + // Remove all markers in one batch + controller.removeMarkers(markerIds); + + // Verify removeItems is called exactly once with all 5 markers + Mockito.verify(clusterManagersController, times(1)) + .removeItems( + eq(clusterManagerId), + Mockito.argThat( + markerBuilders -> + markerBuilders.size() == 5 + && markerBuilders + .stream() + .allMatch(mb -> mb.clusterManagerId().equals(clusterManagerId)))); + + // Verify removeItem is never called (we're using batch operation) + Mockito.verify(clusterManagersController, times(0)).removeItem(any()); + } + + @Test + public void controller_BatchChangeMarkersWithClusterManagerChange() { + final String clusterManagerId1 = "cm123"; + final String clusterManagerId2 = "cm456"; + + // First add markers to cluster manager 1 + final List initialMarkers = new java.util.ArrayList<>(); + for (int i = 0; i < 5; i++) { + final Messages.PlatformMarker.Builder builder = defaultMarkerBuilder(); + builder + .setMarkerId("marker" + i) + .setClusterManagerId(clusterManagerId1) + .setPosition( + new Messages.PlatformLatLng.Builder() + .setLatitude(1.0 + i) + .setLongitude(2.0 + i) + .build()); + initialMarkers.add(builder.build()); + } + controller.addMarkers(initialMarkers); + + // Reset mock to clear invocation counts + Mockito.reset(clusterManagersController); + + // Now change all markers to cluster manager 2 + final List changedMarkers = new java.util.ArrayList<>(); + for (int i = 0; i < 5; i++) { + final Messages.PlatformMarker.Builder builder = defaultMarkerBuilder(); + builder + .setMarkerId("marker" + i) + .setClusterManagerId(clusterManagerId2) // Different cluster manager + .setPosition( + new Messages.PlatformLatLng.Builder() + .setLatitude(3.0 + i) + .setLongitude(4.0 + i) + .build()); + changedMarkers.add(builder.build()); + } + controller.changeMarkers(changedMarkers); + + // Verify removeItems is called exactly once for cluster manager 1 with all 5 markers + Mockito.verify(clusterManagersController, times(1)) + .removeItems( + eq(clusterManagerId1), + Mockito.argThat( + markerBuilders -> + markerBuilders.size() == 5 + && markerBuilders + .stream() + .allMatch(mb -> mb.clusterManagerId().equals(clusterManagerId1)))); + + // Verify addItems is called exactly once for cluster manager 2 with all 5 markers + Mockito.verify(clusterManagersController, times(1)) + .addItems( + eq(clusterManagerId2), + Mockito.argThat( + markerBuilders -> + markerBuilders.size() == 5 + && markerBuilders + .stream() + .allMatch(mb -> mb.clusterManagerId().equals(clusterManagerId2)))); + + // Verify individual operations are never called (we're using batch operations) + Mockito.verify(clusterManagersController, times(0)).addItem(any()); + Mockito.verify(clusterManagersController, times(0)).removeItem(any()); + } } diff --git a/packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml index d721130dfee1..a1aaecfc5bfa 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml +++ b/packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml @@ -2,7 +2,7 @@ name: google_maps_flutter_android description: Android implementation of the google_maps_flutter plugin. repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22 -version: 2.18.12 +version: 2.18.13 environment: sdk: ^3.9.0