Repository navigation
[google_maps_flutter] Convert overlay controllers to Swift - #12638
Merged
auto-submit[bot] merged 7 commits intoAug 27, 2026
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the ground overlay and tile overlay controllers from Objective-C to Swift across the google_maps_flutter_ios_sdk9, google_maps_flutter_ios_sdk10, and shared code packages, updating the main map controllers and unit tests accordingly. Feedback on these changes highlights two main areas for improvement: first, ensuring that the receiver callback in requestTileFor is always invoked to prevent potential resource leaks or rendering hangs in the Google Maps SDK; second, passing nil as the initial icon when creating GMSGroundOverlay in the add method to avoid a redundant and potentially expensive FGMIconFromBitmap call.
Comment on lines
+117
to
+151
| if let position = groundOverlay.position { | ||
| guard let zoomLevel = groundOverlay.zoomLevel else { | ||
| fatalError("If ground overlay is initialized with position, zoomLevel is required") | ||
| } | ||
| gmsOverlay = GMSGroundOverlay( | ||
| position: CLLocationCoordinate2D( | ||
| latitude: position.latitude, | ||
| longitude: position.longitude | ||
| ), | ||
| icon: FGMIconFromBitmap(groundOverlay.image, assetProvider, screenScale), | ||
| zoomLevel: CGFloat(zoomLevel.doubleValue) | ||
| ) | ||
| } else { | ||
| isCreatedWithBounds = true | ||
| // TODO(stuartmorgan): Convert these to Pigeon error handling rather | ||
| // than being fatal. The original Obj-C code used NSAssert instead of | ||
| // passing errors back, which isn't how we should handle errors. | ||
| guard let bounds = groundOverlay.bounds else { | ||
| fatalError("If ground overlay is initialized without position, bounds are required") | ||
| } | ||
| let gmsBounds = GMSCoordinateBounds( | ||
| coordinate: CLLocationCoordinate2D( | ||
| latitude: bounds.northeast.latitude, | ||
| longitude: bounds.northeast.longitude | ||
| ), | ||
| coordinate: CLLocationCoordinate2D( | ||
| latitude: bounds.southwest.latitude, | ||
| longitude: bounds.southwest.longitude | ||
| ) | ||
| ) | ||
| gmsOverlay = GMSGroundOverlay( | ||
| bounds: gmsBounds, | ||
| icon: FGMIconFromBitmap(groundOverlay.image, assetProvider, screenScale) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
In add, FGMIconFromBitmap is called to create the icon for GMSGroundOverlay. However, immediately after creating the overlay, controller.update is called, which calls GroundOverlayController.update and sets the icon again. We can pass nil as the initial icon to avoid the redundant and potentially expensive FGMIconFromBitmap call.
if let position = groundOverlay.position {
guard let zoomLevel = groundOverlay.zoomLevel else {
fatalError("If ground overlay is initialized with position, zoomLevel is required")
}
gmsOverlay = GMSGroundOverlay(
position: CLLocationCoordinate2D(
latitude: position.latitude,
longitude: position.longitude
),
icon: nil,
zoomLevel: CGFloat(zoomLevel.doubleValue)
)
} else {
isCreatedWithBounds = true
// TODO(stuartmorgan): Convert these to Pigeon error handling rather
// than being fatal. The original Obj-C code used NSAssert instead of
// passing errors back, which isn't how we should handle errors.
guard let bounds = groundOverlay.bounds else {
fatalError("If ground overlay is initialized without position, bounds are required")
}
let gmsBounds = GMSCoordinateBounds(
coordinate: CLLocationCoordinate2D(
latitude: bounds.northeast.latitude,
longitude: bounds.northeast.longitude
),
coordinate: CLLocationCoordinate2D(
latitude: bounds.southwest.latitude,
longitude: bounds.southwest.longitude
)
)
gmsOverlay = GMSGroundOverlay(
bounds: gmsBounds,
icon: nil
)
}
Collaborator
Author
There was a problem hiding this comment.
Indeed; changed as in the other PR.
Collaborator
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request converts the ground overlay and tile overlay controllers from Objective-C to Swift across the google_maps_flutter_ios_sdk10, google_maps_flutter_ios_sdk9, and shared iOS code packages. Feedback on the changes highlights potential stability issues: first, the use of fatalError for validation in GroundOverlayController could crash production applications, where assertionFailure with graceful fallback should be used instead; second, a missing nil check on tileProviderDelegate in TileProviderController could prevent the completion handler from running, potentially causing the Google Maps SDK to hang.
Collaborator
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request converts the ground overlay and tile overlay controllers from Objective-C to Swift across the iOS SDK 9, SDK 10, and shared code packages, alongside corresponding test updates. The review feedback suggests using pixel dimensions and preserving the original scale and orientation in handleResultTile to prevent resolution loss on retina displays, and mapping error.details to a string before nil-coalescing to avoid implicit coercion compiler warnings.
vashworth
approved these changes
Aug 27, 2026
stuartmorgan-g
added
the
autosubmit
This was referenced Aug 28, 2026
pull Bot
pushed a commit
to Mu-L/flutter
that referenced
this pull request
Aug 28, 2026
…er#191965) flutter/packages@bd3cbc1...cd4cdd0 2026-08-28 [email protected] [material_ui] Add all M3 templates and generated code to `temporarily_excluded/` before migration (flutter/packages#12661) 2026-08-28 [email protected] Roll Flutter from 15d8908 to e8dca90 (58 revisions) (flutter/packages#12660) 2026-08-28 [email protected] [various] Update pigeon dev_dependency to ^27.3.2 (flutter/packages#12615) 2026-08-27 [email protected] [camera_android_camerax] Enforce CHANGELOG backticks and add eval commit author check to pre-push-skill (flutter/packages#12624) 2026-08-27 [email protected] [material_ui] Prevent stale async suggestions in SearchAnchor (flutter/packages#12478) 2026-08-27 [email protected] [go_router_builder] Support analyzer 14 (flutter/packages#12614) 2026-08-27 [email protected] [cupertino_ui] Use super parameters in more places (flutter/packages#12459) 2026-08-27 [email protected] [material_ui] Remove no-shuffle from progress indicator test (flutter/packages#12505) 2026-08-27 [email protected] [two_dimensional_scrollables] Activate leak testing and fix memory leaks (flutter/packages#11653) 2026-08-27 [email protected] [material_ui] Add helper methods in gen_defaults template (flutter/packages#12637) 2026-08-27 [email protected] [cupertino_ui] fix CupertinoIcons font not being included in examples and fix TextEditingController leaks (flutter/packages#12228) 2026-08-27 [email protected] [camera_android_camerax] Check Git hooks configuration in check-readiness skill (flutter/packages#12628) 2026-08-27 [email protected] [ci] sync back pr for branch release only run when release succeeds (flutter/packages#12581) 2026-08-27 [email protected] [google_maps_flutter] Convert overlay controllers to Swift (flutter/packages#12638) 2026-08-27 [email protected] [go_router_builder] Fixes text golden test to ignore platform specific newline (flutter/packages#12652) 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
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…2638) This converts GroundOverlayController and TileOverlayController to Swift in the `_sdk*` packages. The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review. The conversion process was: - Initial conversion via Gemini, with explicit instruction to keep the structure the same. - Side-by-side manual review of the old and new versions of the files. - Manual fixes and improvements. This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local `mapView` instance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive. Part of flutter/flutter#119108 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…2638) This converts GroundOverlayController and TileOverlayController to Swift in the `_sdk*` packages. The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review. The conversion process was: - Initial conversion via Gemini, with explicit instruction to keep the structure the same. - Side-by-side manual review of the old and new versions of the files. - Manual fixes and improvements. This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local `mapView` instance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive. Part of flutter/flutter#119108 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This converts GroundOverlayController and TileOverlayController to Swift in the
_sdk*packages.The remaining Obj-C code will be migrated in a series of follow-up PRs to keep the scope of each PR reasonable for review.
The conversion process was:
This includes test changes beyond what we would normally expect for a language conversion PR (e.g., name changes from dropping the prefix) because the conversion to Swift exposed a latent issue in the tests. Several tests had a pattern where a helper method was creating a local
mapViewinstance, using it to construct the subcontroller under test, and then returning that subcontroller—at which point the map view goes out of scope. The subcontrollers have weak references to the map view, so it can be deallocated before the test actually runs, and doing so causes the tests to fail (as many operations in the subcontroller no-op when the map view is nil). In cases where the helper is only used once, it can be inlined; in cases where it's used repeatedly we can instead return multiple values and keep the map view alive.Part of flutter/flutter#119108
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2