-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[google_maps_flutter] Fix the visual jarring during the first gesture on the map #2629
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,10 +50,6 @@ @implementation FLTGoogleMapController { | |
| FlutterMethodChannel* _channel; | ||
| BOOL _trackCameraPosition; | ||
| NSObject<FlutterPluginRegistrar>* _registrar; | ||
| // Used for the temporary workaround for a bug that the camera is not properly positioned at | ||
| // initialization. https://github.com/flutter/flutter/issues/24806 | ||
| // TODO(cyanglaz): Remove this temporary fix once the Maps SDK issue is resolved. | ||
| // https://github.com/flutter/flutter/issues/27550 | ||
| BOOL _cameraDidInitialSetup; | ||
| FLTMarkersController* _markersController; | ||
| FLTPolygonsController* _polygonsController; | ||
|
|
@@ -119,9 +115,36 @@ - (instancetype)initWithFrame:(CGRect)frame | |
| } | ||
|
|
||
| - (UIView*)view { | ||
| [_mapView addObserver:self forKeyPath:@"frame" options:0 context:nil]; | ||
| return _mapView; | ||
| } | ||
|
|
||
| - (void)observeValueForKeyPath:(NSString*)keyPath | ||
| ofObject:(id)object | ||
| change:(NSDictionary*)change | ||
| context:(void*)context { | ||
| if (_cameraDidInitialSetup) { | ||
| // We only observe the frame for initial setup. | ||
| [_mapView removeObserver:self forKeyPath:@"frame"]; | ||
| return; | ||
| } | ||
| if (object == _mapView && [keyPath isEqualToString:@"frame"]) { | ||
| CGRect bounds = _mapView.bounds; | ||
| if (CGRectEqualToRect(bounds, CGRectZero)) { | ||
| // The workaround is to fix an issue that the camera location is not current when | ||
| // the size of the map is zero at initialization. | ||
| // So We only care about the size of the `_mapView`, ignore the frame changes when the size is | ||
| // zero. | ||
| return; | ||
| } | ||
| _cameraDidInitialSetup = YES; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not remove the observer here instead of on the next frame change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point! Thanks. will change. |
||
| [_mapView removeObserver:self forKeyPath:@"frame"]; | ||
| [_mapView moveCamera:[GMSCameraUpdate setCamera:_mapView.camera]]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the intended behavior if a the map frame is resized while it's being animated?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, this method will get called multiple times. However, since we are always setting the camera to the same location, it shouldn't have any behavior effects.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. animateWithCameraUpdate is implemented by multiple calls to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made this only happens once during initialization, so it shouldn't have any side effects that you worried about. |
||
| } else { | ||
| [super observeValueForKeyPath:keyPath ofObject:object change:change context:context]; | ||
| } | ||
| } | ||
|
|
||
| - (void)onMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { | ||
| if ([call.method isEqualToString:@"map#show"]) { | ||
| [self showAtX:ToDouble(call.arguments[@"x"]) Y:ToDouble(call.arguments[@"y"])]; | ||
|
|
@@ -437,16 +460,6 @@ - (void)mapView:(GMSMapView*)mapView willMove:(BOOL)gesture { | |
| } | ||
|
|
||
| - (void)mapView:(GMSMapView*)mapView didChangeCameraPosition:(GMSCameraPosition*)position { | ||
| if (!_cameraDidInitialSetup) { | ||
| // We suspected a bug in the iOS Google Maps SDK caused the camera is not properly positioned at | ||
| // initialization. https://github.com/flutter/flutter/issues/24806 | ||
| // This temporary workaround fix is provided while the actual fix in the Google Maps SDK is | ||
| // still being investigated. | ||
| // TODO(cyanglaz): Remove this temporary fix once the Maps SDK issue is resolved. | ||
| // https://github.com/flutter/flutter/issues/27550 | ||
| _cameraDidInitialSetup = YES; | ||
| [mapView moveCamera:[GMSCameraUpdate setCamera:_mapView.camera]]; | ||
| } | ||
| if (_trackCameraPosition) { | ||
| [_channel invokeMethod:@"camera#onMove" arguments:@{@"position" : PositionToJson(position)}]; | ||
| } | ||
|
|
@@ -511,8 +524,8 @@ - (void)mapView:(GMSMapView*)mapView didLongPressAtCoordinate:(CLLocationCoordin | |
|
|
||
| static NSDictionary* PointToJson(CGPoint point) { | ||
| return @{ | ||
| @"x" : @((int)point.x), | ||
| @"y" : @((int)point.y), | ||
| @"x" : @(lroundf(point.x)), | ||
| @"y" : @(lroundf(point.y)), | ||
| }; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a reliable way to detect when it's ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless we add a new callback API in the google map, something like "onMapRendered". This can be in a separate PR tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do that, I'm not blocking this PR since we're trading a 3 seconds timer with a 1 second one , but we should at least add a TODO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!