[google_maps_flutter] Support for Ground Overlay #8432

jokerttu · 2025-01-15T07:44:45Z

Adds Ground Overlay support for google_maps_flutter plugin.

Fixes flutter/flutter#26479

Android:

iOS:

Web:

Documentation for groundOverlays parameter on app-facing plugin:

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

jokerttu · 2025-01-16T11:28:28Z

...es/google_maps_flutter/google_maps_flutter/example/integration_test/src/tiles_inspector.dart

@@ -22,7 +21,7 @@ void main() {
 }

 void runTests() {
-  const double floatTolerance = 1e-8;
+  const double floatTolerance = 1e-6;


Added bigger tolerance as on Android and iOS tests failed as diff was bigger than 1e-8

jokerttu · 2025-01-16T11:31:14Z

...es/google_maps_flutter/google_maps_flutter/example/integration_test/src/tiles_inspector.dart

-          Color.fromARGB(255, 0, 0, 191),
-          0.6,
+      );
+      await tester.pumpAndSettle(const Duration(seconds: 3));


Without this line tests timeouted on Web platform, and we were not able to test ground overlay below.

Were they timing out in all tests, or wasm specifically? I know I had to revisit a bunch of plugins and add a few pumps and pumpAndSettle when enabling the wasm tests. See:

[ci] Run web tests in wasm (unit + integration). #8111

@ditman
Note that google_maps_flutter tests are currently excluded for web platform because failing tests:
https://github.com/flutter/packages/blob/main/script/configs/exclude_integration_web.yaml

New implemented tests pass with and without wasm enabled, but only if I disable other test sets (that fail because other reasons). The new tests are located in tiles_inspector.dart and I just run this set, which includes tests for tiles, heatmaps, and ground overlays. This file should probably be renamed or split?

Fixing other tests is out of scope of this PR, but this must include working integration tests for web for new features and functionality.

So when only tiles_inspector.dart tests are enabled tests pass with following commands:

Without wasm

dart run ./script/tool/bin/flutter_plugin_tools.dart drive-examples --web --run-chromedriver --packages=google_maps_flutter/google_maps_flutter

and with wasm

dart run ./script/tool/bin/flutter_plugin_tools.dart drive-examples --web --wasm --run-chromedriver --packages=google_maps_flutter/google_maps_flutter

...es/google_maps_flutter/google_maps_flutter/example/integration_test/src/tiles_inspector.dart

jokerttu · 2025-01-16T11:32:16Z

...es/google_maps_flutter/google_maps_flutter/example/integration_test/src/tiles_inspector.dart

-      expect(wll1.weight, wll2.weight);
-      expect(wll1.point.latitude, moreOrLessEquals(wll2.point.latitude));
-      expect(wll1.point.longitude, moreOrLessEquals(wll2.point.longitude));
+  group('Heatmaps', () {


Added Heatmap tests to own group, like tile overlay tests and ground overlay tests.

...ps_flutter_platform_interface/lib/src/platform_interface/google_maps_inspector_platform.dart

stuartmorgan-g

This looks really good! I did a pass over everything apart from web, and mostly just have minor comments and questions.

...google_maps_flutter/google_maps_flutter_platform_interface/lib/src/types/ground_overlay.dart

stuartmorgan-g · 2025-01-17T19:25:57Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMGroundOverlayController.m

+}
+
+- (void)didTapGroundOverlayWithIdentifier:(NSString *)identifier {
+  if (!identifier) {


Here and in the next method: the identifier params were annotated as non-nullable in the interface, so the implementation code shouldn't be handling nil.

Good catch. Removed the null check.

It's still here in this method.

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMUtils.h

stuartmorgan-g · 2025-01-17T19:28:39Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMUtils.h

+NS_ASSUME_NONNULL_BEGIN
+
+@interface FGMUtils : NSObject
+ (UIImage *)iconFromBitmap:(FGMPlatformBitmap *)platformBitmap


These new APIs need declaration comments.

stuartmorgan-g · 2025-01-17T19:34:15Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMUtils.h

Can this be given a more specific name? It's already the case that this plugin has at least one other file of utils (the conversion function file).

I renamed it FGMImageUtils.

stuartmorgan-g · 2025-01-17T19:36:37Z

packages/google_maps_flutter/google_maps_flutter_ios/pigeons/messages.dart

+  final PlatformBitmap image;
+  final PlatformLatLng? position;
+  final PlatformLatLngBounds? bounds;
+  final PlatformDoublePair? anchor;


Isn't this conceptually a point? We already have PlatformPoint, so shouldn't need a new PlatformDoublePair for this unless there's a conceptual difference in meaning here.

Changed this to PlatformPoint.

stuartmorgan-g

Just some scattered minor issues remaining. Feel free to split out the first sub-PR at this point!

...google_maps_flutter/google_maps_flutter_platform_interface/lib/src/types/ground_overlay.dart

stuartmorgan-g · 2025-01-24T19:56:04Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+    if (west <= east) {
+      longitudeOffset = position.longitude - west;
+    } else {
+      longitudeOffset = position.longitude - west;


This is the same in both branches; is that a copy/paste mistake?

If not, it should be factored out of the if/else.

(And if so, that suggests missing test coverage for the behavior in these branches.)

stuartmorgan-g · 2025-01-27T16:04:18Z

...ogle_maps_flutter_ios/example/ios14/ios/RunnerTests/GoogleMapsGroundOverlayControllerTests.m

+@import GoogleMaps;
+
+#import <OCMock/OCMock.h>
+#import <google_maps_flutter_ios/FGMGroundOverlayController_Test.h>


It should be before all other imports; see https://google.github.io/styleguide/objcguide.html#order-of-includes

stuartmorgan-g · 2025-01-27T16:05:28Z

...ogle_maps_flutter_ios/example/ios14/ios/RunnerTests/GoogleMapsGroundOverlayControllerTests.m

+
+#import <google_maps_flutter_ios/FGMGroundOverlayController_Test.h>
+
+#import <OCMock/OCMock.h>


This should be grouped with the other framework imports. Grouping is by type of thing import, not by @ vs #.

stuartmorgan-g · 2025-01-27T16:07:16Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMGroundOverlayController.h

+/// Returns true if a ground overlay with the given identifier exists on the map.
+- (bool)hasGroundOverlaysWithIdentifier:(NSString *)identifier;
+
+/// Returns FGMPlatformGroundOverlay for identifier.


"Returns the ground overlay with the given identifier."

stuartmorgan-g · 2025-01-27T16:10:55Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMGroundOverlayController.m

+                              registrar:(NSObject<FlutterPluginRegistrar> *)registrar
+                            screenScale:(CGFloat)screenScale {
+  [self setConsumeTapEvents:groundOverlay.clickable];
+  [self setVisible:groundOverlay.visible];


Based on investigation of similar methods in a recent PR (which hasn't landed), this needs to be last to avoid potential flickering (likely due to Google Maps using its own render thread), so should be at the end of the method, and have a comment saying that nothing should be added after it.

stuartmorgan-g · 2025-01-27T16:13:13Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMGroundOverlayController.m

+}
+
+- (void)didTapGroundOverlayWithIdentifier:(NSString *)identifier {
+  if (!identifier) {


It's still here in this method.

stuartmorgan-g · 2025-01-27T16:15:31Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMImageUtils.m

+static UIImage *scaleImage(UIImage *image, double scale);
+static UIImage *scaledImageWithScale(UIImage *image, CGFloat scale);
+static UIImage *scaledImageWithSize(UIImage *image, CGSize size);
+static UIImage *scaledImage(UIImage *image, NSNumber *width, NSNumber *height, CGFloat screenScale);


The declaration comments for all of these functions should be here, on the declarations, rather than on the definitions below.

stuartmorgan-g · 2025-01-27T16:16:13Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FGMImageUtils.m

+#import "FGMImageUtils.h"
+#import "Foundation/Foundation.h"
+
+static UIImage *scaleImage(UIImage *image, double scale);


This should be scaled, not scale, for consistency in naming.

stuartmorgan-g · 2025-01-27T16:18:10Z

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTGoogleMapJSONConversions.m

+                                                    NSNumber *zoomLevel) {
+  /// Dummy image is used as image is required field of FGMPlatformGroundOverlay and converting
+  /// image back to bitmap image is not currently supported.
+  FGMPlatformBitmap *mockImage =


This should be called placeholderImage rather than mockImage, as "mock" generally refers to a specific test construct.

@jokerttu

…#8518) This PR contains platform interface for the upcoming ground overlays support (#8432). Original PR with all the changes hasn't been approved yet but it was [okayed](#8432 (review)) to create the first sub-PR. I'm not the author of the original PR but helping @jokerttu while he's on vacation. Linked issue: flutter/flutter#26479

jokerttu · 2025-02-05T17:40:12Z

I've addressed the last remaining review comments and split the platform-specific implementations into a separate PR: #8563.
Apologies for the delay!

reidbaker · 2025-02-24T21:19:46Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+   * @return the identifier of the ground overlay.
+   * @throws IllegalArgumentException if required fields are missing or invalid.
+   */
+  static String interpretGroundOverlayOptions(


Please add non null or nullable annotations to the objects in this method.

reidbaker · 2025-02-24T21:20:43Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+   *     asset files stored in the application's assets directory.
+   * @param density the density of the display, used to calculate pixel dimensions.
+   * @param wrapper the BitmapDescriptorFactoryWrapper to create BitmapDescriptor.
+   * @return the identifier of the ground overlay.


If you know can you add more about the mutability of the ground overlay identifier. For example how long it is a valid id.

reidbaker · 2025-02-24T21:22:59Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+    sink.setClickable(groundOverlay.getClickable());
+    sink.setImage(toBitmapDescriptor(groundOverlay.getImage(), assetManager, density, wrapper));
+    if (groundOverlay.getPosition() != null) {
+      assert groundOverlay.getWidth() != null;


Asserts I believe are compiled out so the else condition can still NPE here. Can you convert this assert to a check or handle null width?

reidbaker · 2025-02-24T21:24:48Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+    sink.setImage(toBitmapDescriptor(groundOverlay.getImage(), assetManager, density, wrapper));
+    if (groundOverlay.getPosition() != null) {
+      assert groundOverlay.getWidth() != null;
+      if (groundOverlay.getHeight() != null) {


I believe these are the same except the width value. Can you minimize the scope by either making the check cover width only?

reidbaker · 2025-02-24T21:26:56Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+        new Messages.PlatformGroundOverlay.Builder()
+            .setGroundOverlayId(groundOverlayId)
+            .setImage(dummyImage)
+            .setWidth((double) groundOverlay.getWidth())


Why use doubles here but floats above for width?

Pigeon uses doubles and google maps uses floats, so conversion between these need to be done while converting values between PlatformGroundOverlay and GoogleMaps SDK GroundOverlay object

reidbaker · 2025-02-24T21:27:34Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

+            .setVisible(groundOverlay.isVisible())
+            .setClickable(groundOverlay.isClickable());
+
+    if (isCreatedWithBounds) {


It is possible to determine this instead of having it be passed in? For example could a non null getBounds or getPosition indicate that one of those should be used?

The code above in interpretGroundOverlayOptions seem to imply that position is more important that bounds. This code seem to imply that if bounds are used then position should not be.

Google maps object internally have both, bounds and position when the object is created using either of them. When bounds are used, the position representes the achor point in LatLng format for the ground overlay. And if position (with width/height) the bounds are calculated automatically as well.
For integration testing purposes we need track which creation method was used, to be able to check that conversions are handled correctly on each platform.

reidbaker · 2025-02-24T21:33:12Z

...s_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/GroundOverlaySink.java

+
+/** Receiver of GroundOverlayOptions configuration. */
+interface GroundOverlaySink {
+  void setTransparency(float transparency);


Can you add javadocs to these methods for where someone can learn more about the inputs either here or in groundOverlayController

For example what are the min and max values for transparency. Can zindex be negative? what does setClickable do? etc.

reidbaker · 2025-02-24T21:34:30Z

...er_android/android/src/main/java/io/flutter/plugins/googlemaps/GroundOverlaysController.java

+    GroundOverlayController groundOverlayController =
+        groundOverlayIdToController.get(groundOverlayId);
+    if (groundOverlayController != null) {
+      groundOverlayController.remove();


Does the order of these removals matter? if so please add a comment explaining the order.

Order does not matter here

reidbaker · 2025-02-24T21:36:10Z

...er_android/android/src/main/java/io/flutter/plugins/googlemaps/GroundOverlaysController.java

+    String groundOverlayId = platformGroundOverlay.getGroundOverlayId();
+    GroundOverlayController groundOverlayController =
+        groundOverlayIdToController.get(groundOverlayId);
+    if (groundOverlayController != null) {


It is an error to call this with something that cant get a groundOverlayController? If so whos error is it (plugin or host app)?

Api has updateGroundOverlays method takes GroundOverlayUpdates object as parameter:
https://github.com/flutter/packages/blob/ff7724c18a803542fc709f942c3bf1cecf7e4864/packages/google_maps_flutter/google_maps_flutter_platform_interface/lib/src/types/ground_overlay_updates.dart
This should be handled by the dart code so that changes only contains items that are available on map.
Not sure would it be good to throw error here if change is triggered for groundoverlay that is not added to the map beforehand. It is indeed misuse of the api, and this is tested in integration tests.

This PR contains platform implementations for the ground overlays support (#8432). Linked issue: flutter/flutter#26479

…r 3.27/Dart 3.6

jokerttu · 2025-03-18T15:51:50Z

@stuartmorgan This PR now contains only app-facing plugin changes.
Had to update minimum supported SDK versions to Flutter 3.27/Dart 3.6 to match the android package:

packages/packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml

Lines 7 to 9 in 297d5a1

    
           environment: 
        
             sdk: ^3.6.0 
        
             flutter: ">=3.27.0"

stuartmorgan-g

LGTM!

…ackages#8432)

flutter/packages@d450e1b...dd781d4 2025-03-19 [email protected] [camera_avfoundation] Tests backfilling - part 4 (flutter/packages#8854) 2025-03-18 [email protected] [camera_avfoundation] Tests backfilling - part 5 (flutter/packages#8873) 2025-03-18 [email protected] [video_player]: reduce video player position update interval from 500ms to 100ms (flutter/packages#8346) 2025-03-18 [email protected] [google_maps_flutter] Support for Ground Overlay (flutter/packages#8432) 2025-03-18 [email protected] [google_maps_flutter] Ground overlay support - platform impls (flutter/packages#8563) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

github-actions bot added p: google_maps_flutter platform-android platform-ios platform-web labels Jan 15, 2025

jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch from c94285d to 19a7808 Compare January 16, 2025 10:34

jokerttu commented Jan 16, 2025

View reviewed changes

...es/google_maps_flutter/google_maps_flutter/example/integration_test/src/tiles_inspector.dart Show resolved Hide resolved

jokerttu commented Jan 16, 2025

View reviewed changes

...ps_flutter_platform_interface/lib/src/platform_interface/google_maps_inspector_platform.dart Outdated Show resolved Hide resolved

jokerttu commented Jan 16, 2025

View reviewed changes

...ps_flutter_platform_interface/lib/src/platform_interface/google_maps_inspector_platform.dart Outdated Show resolved Hide resolved

jokerttu marked this pull request as ready for review January 16, 2025 11:38

jokerttu requested review from cbracken, reidbaker, ditman and stuartmorgan-g as code owners January 16, 2025 11:38

reidbaker requested a review from jesswrd January 17, 2025 16:28

stuartmorgan-g requested changes Jan 17, 2025

View reviewed changes

jokerttu requested a review from stuartmorgan-g January 22, 2025 19:40

stuartmorgan-g requested changes Jan 27, 2025

View reviewed changes

This was referenced Jan 28, 2025

Ground overlay: address PR comments CodemateLtd/packages#16

Merged

[google_maps_flutter] Support for Ground Overlay - platform interface #8518

Merged

jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch from 9f427b8 to cda9938 Compare February 4, 2025 13:57

jokerttu mentioned this pull request Feb 4, 2025

[google_maps_flutter] Ground overlay support - platform impls #8563

Merged

11 tasks

reidbaker reviewed Feb 24, 2025

View reviewed changes

jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch 2 times, most recently from dcf54a1 to caf5ead Compare March 17, 2025 10:09

auto-submit bot pushed a commit that referenced this pull request Mar 18, 2025

[google_maps_flutter] Ground overlay support - platform impls (#8563)

297d5a1

This PR contains platform implementations for the ground overlays support (#8432). Linked issue: flutter/flutter#26479

[google_maps_flutter] Add support for ground overlays

89366c8

jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch from caf5ead to 89366c8 Compare March 18, 2025 15:33

github-actions bot removed platform-android platform-ios platform-web labels Mar 18, 2025

[google_maps_flutter] Update minimum supported SDK versions to Flutte…

fea82e3

…r 3.27/Dart 3.6

jokerttu requested a review from stuartmorgan-g March 18, 2025 15:51

stuartmorgan-g approved these changes Mar 18, 2025

View reviewed changes

jokerttu added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2025

auto-submit bot merged commit f2e2987 into flutter:main Mar 18, 2025
82 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 19, 2025

f2e298707 [google_maps_flutter] Support for Ground Overlay (flutter/p…

cb17348

…ackages#8432)

engine-flutter-autoroll mentioned this pull request Mar 19, 2025

Roll Packages from d450e1bfa713 to dd781d4bcb7f (5 revisions) flutter/flutter#165489

Merged

darshankawar mentioned this pull request Mar 21, 2025

[google_maps_flutter] Can't display GroundOverlay on iOS when the image is loaded from the network as 'BytesMapBitmap' (works on Web and Android) flutter/flutter#165563

Closed


		#import <google_maps_flutter_ios/FGMGroundOverlayController_Test.h>

		#import <OCMock/OCMock.h>

[google_maps_flutter] Support for Ground Overlay #8432

[google_maps_flutter] Support for Ground Overlay #8432

Conversation

jokerttu commented Jan 15, 2025 • edited Loading

Pre-launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jokerttu Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

stuartmorgan-g left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jokerttu commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jokerttu commented Mar 18, 2025

stuartmorgan-g left a comment

Choose a reason for hiding this comment

jokerttu commented Jan 15, 2025 •

edited

Loading

jokerttu Jan 16, 2025 •

edited

Loading

stuartmorgan-g left a comment •

edited

Loading