From 8a7c3bb22ab9efa097b6f70eaaef683a632cc1b4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 21 Apr 2020 10:29:15 -0700 Subject: [PATCH 1/5] fix camera init position workaround --- .../google_maps_flutter/CHANGELOG.md | 5 + .../example/test_driver/google_maps_e2e.dart | 106 ++++++++++++------ .../ios/Classes/GoogleMapController.m | 47 +++++--- .../google_maps_flutter/pubspec.yaml | 2 +- 4 files changed, 107 insertions(+), 53 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md index 5b3e35097f34..bdc8a7372c76 100644 --- a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md +++ b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.5.26+3 + +* iOS: observe the bounds update for the `GMSMapView` to reset the camera setting. +* Update UI related e2e tests to wait for camera update on the platform thread. + ## 0.5.26+2 * Fix UIKit availability warnings and CocoaPods podspec lint warnings. diff --git a/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart b/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart index 3b78d2f975a7..4c293727d6bb 100644 --- a/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart +++ b/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart @@ -383,6 +383,53 @@ void main() { expect(scrollGesturesEnabled, true); }); + testWidgets('testInitialCenterLocationAtCenter', (WidgetTester tester) async { + final Completer mapControllerCompleter = + Completer(); + final Key key = GlobalKey(); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: GoogleMap( + key: key, + initialCameraPosition: _kInitialCameraPosition, + onMapCreated: (GoogleMapController controller) { + mapControllerCompleter.complete(controller); + }, + ), + ), + ); + final GoogleMapController mapController = + await mapControllerCompleter.future; + + await tester.pumpAndSettle(); + // TODO(cyanglaz): Remove this after we added `mapRendered` callback, and `mapControllerCompleter.complete(controller)` above should happen + // in `mapRendered`. + // https://github.com/flutter/flutter/issues/54758 + await Future.delayed(Duration(seconds: 1)); + + ScreenCoordinate coordinate = + await mapController.getScreenCoordinate(_kInitialCameraPosition.target); + Rect rect = tester.getRect(find.byKey(key)); + if (Platform.isIOS) { + // On iOS, the coordinate value from the GoogleMapSdk doesn't include the devicePixelRatio`. + // So we don't need to do the conversion like we did below for other platforms. + expect(coordinate.x, (rect.center.dx - rect.topLeft.dx).round()); + expect(coordinate.y, (rect.center.dy - rect.topLeft.dy).round()); + } else { + expect( + coordinate.x, + ((rect.center.dx - rect.topLeft.dx) * + tester.binding.window.devicePixelRatio) + .round()); + expect( + coordinate.y, + ((rect.center.dy - rect.topLeft.dy) * + tester.binding.window.devicePixelRatio) + .round()); + } + }); + testWidgets('testGetVisibleRegion', (WidgetTester tester) async { final Key key = GlobalKey(); final LatLngBounds zeroLatLngBounds = LatLngBounds( @@ -401,13 +448,8 @@ void main() { }, ), )); - // 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 - await tester.pumpAndSettle(const Duration(seconds: 3)); + await tester.pumpAndSettle(); + final GoogleMapController mapController = await mapControllerCompleter.future; @@ -707,13 +749,11 @@ void main() { final GoogleMapController controller = await controllerCompleter.future; - // 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 - await tester.pumpAndSettle(const Duration(seconds: 3)); + await tester.pumpAndSettle(); + // TODO(cyanglaz): Remove this after we added `mapRendered` callback, and `mapControllerCompleter.complete(controller)` above should happen + // in `mapRendered`. + // https://github.com/flutter/flutter/issues/54758 + await Future.delayed(Duration(seconds: 1)); final LatLngBounds visibleRegion = await controller.getVisibleRegion(); final LatLng topLeft = @@ -744,13 +784,11 @@ void main() { final GoogleMapController controller = await controllerCompleter.future; - // 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 - await tester.pumpAndSettle(const Duration(seconds: 3)); + await tester.pumpAndSettle(); + // TODO(cyanglaz): Remove this after we added `mapRendered` callback, and `mapControllerCompleter.complete(controller)` above should happen + // in `mapRendered`. + // https://github.com/flutter/flutter/issues/54758 + await Future.delayed(Duration(seconds: 1)); double zoom = await controller.getZoomLevel(); expect(zoom, _kInitialZoomLevel); @@ -778,13 +816,11 @@ void main() { )); final GoogleMapController controller = await controllerCompleter.future; - // 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 - await tester.pumpAndSettle(const Duration(seconds: 3)); + await tester.pumpAndSettle(); + // TODO(cyanglaz): Remove this after we added `mapRendered` callback, and `mapControllerCompleter.complete(controller)` above should happen + // in `mapRendered`. + // https://github.com/flutter/flutter/issues/54758 + await Future.delayed(Duration(seconds: 1)); final LatLngBounds visibleRegion = await controller.getVisibleRegion(); final LatLng northWest = LatLng( @@ -818,13 +854,11 @@ void main() { home: Scaffold( body: SizedBox(height: 400, width: 400, child: map))))); - // 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 - await tester.pumpAndSettle(const Duration(seconds: 3)); + await tester.pumpAndSettle(); + // TODO(cyanglaz): Remove this after we added `mapRendered` callback, and `mapControllerCompleter.complete(controller)` above should happen + // in `mapRendered`. + // https://github.com/flutter/flutter/issues/54758 + await Future.delayed(Duration(seconds: 1)); // Simple call to make sure that the app hasn't crashed. final LatLngBounds bounds1 = await controller.getVisibleRegion(); @@ -906,5 +940,5 @@ void main() { final GoogleMapInspector inspector = await inspectorCompleter.future; final Uint8List bytes = await inspector.takeSnapshot(); expect(bytes?.isNotEmpty, true); - }); + }, skip: true); } diff --git a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m index 59bfaa01f04d..c3e8ca1b0246 100644 --- a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m +++ b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m @@ -50,10 +50,6 @@ @implementation FLTGoogleMapController { FlutterMethodChannel* _channel; BOOL _trackCameraPosition; NSObject* _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,38 @@ - (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)) { + // Rerely, frame can change without actually changing the size of the view; + // eg, consider a frame change such as: (0, 0, 0, 0) -> (10, 10, 0, 0) + // We ignore this type of changes. + return; + } + _cameraDidInitialSetup = YES; + [_mapView moveCamera:[GMSCameraUpdate setCamera:_mapView.camera]]; + } else { + [super observeValueForKeyPath:keyPath ofObject:object change:change context:context]; + } +} + +- (void)dealloc { + [_mapView removeObserver:self forKeyPath:@"frame"]; +} + - (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 +462,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 +526,8 @@ - (void)mapView:(GMSMapView*)mapView didLongPressAtCoordinate:(CLLocationCoordin static NSDictionary* PointToJson(CGPoint point) { return @{ - @"x" : @((int)point.x), - @"y" : @((int)point.y), + @"x" : @((int)(point.x + 0.5)), + @"y" : @((int)(point.y + 0.5)), }; } diff --git a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml index 3d5f7b5cd1ba..8c50efcc3c48 100644 --- a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml +++ b/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml @@ -1,7 +1,7 @@ name: google_maps_flutter description: A Flutter plugin for integrating Google Maps in iOS and Android applications. homepage: https://github.com/flutter/plugins/tree/master/packages/google_maps_flutter/google_maps_flutter -version: 0.5.26+2 +version: 0.5.26+3 dependencies: flutter: From 065f141eea4a37aeeb92db479baf54a2f3ba4c83 Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Thu, 23 Apr 2020 12:41:05 -0700 Subject: [PATCH 2/5] review fixes --- .../example/test_driver/google_maps_e2e.dart | 2 +- .../ios/Classes/GoogleMapController.m | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart b/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart index 4c293727d6bb..2eccbc4beba6 100644 --- a/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart +++ b/packages/google_maps_flutter/google_maps_flutter/example/test_driver/google_maps_e2e.dart @@ -940,5 +940,5 @@ void main() { final GoogleMapInspector inspector = await inspectorCompleter.future; final Uint8List bytes = await inspector.takeSnapshot(); expect(bytes?.isNotEmpty, true); - }, skip: true); + }); } diff --git a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m index c3e8ca1b0246..19ac74a225c6 100644 --- a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m +++ b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m @@ -137,16 +137,13 @@ - (void)observeValueForKeyPath:(NSString*)keyPath return; } _cameraDidInitialSetup = YES; + [_mapView removeObserver:self forKeyPath:@"frame"]; [_mapView moveCamera:[GMSCameraUpdate setCamera:_mapView.camera]]; } else { [super observeValueForKeyPath:keyPath ofObject:object change:change context:context]; } } -- (void)dealloc { - [_mapView removeObserver:self forKeyPath:@"frame"]; -} - - (void)onMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([call.method isEqualToString:@"map#show"]) { [self showAtX:ToDouble(call.arguments[@"x"]) Y:ToDouble(call.arguments[@"y"])]; @@ -526,8 +523,8 @@ - (void)mapView:(GMSMapView*)mapView didLongPressAtCoordinate:(CLLocationCoordin static NSDictionary* PointToJson(CGPoint point) { return @{ - @"x" : @((int)(point.x + 0.5)), - @"y" : @((int)(point.y + 0.5)), + @"x" : @(lroundf(point.x)), + @"y" : @(lroundf(point.y)), }; } From 118ff0cffac805b690199c1361d06ac1e9608bf1 Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Thu, 23 Apr 2020 12:43:46 -0700 Subject: [PATCH 3/5] update the comment --- .../google_maps_flutter/ios/Classes/GoogleMapController.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m index 19ac74a225c6..dedd2d45d184 100644 --- a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m +++ b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m @@ -131,9 +131,9 @@ - (void)observeValueForKeyPath:(NSString*)keyPath if (object == _mapView && [keyPath isEqualToString:@"frame"]) { CGRect bounds = _mapView.bounds; if (CGRectEqualToRect(bounds, CGRectZero)) { - // Rerely, frame can change without actually changing the size of the view; - // eg, consider a frame change such as: (0, 0, 0, 0) -> (10, 10, 0, 0) - // We ignore this type of changes. + // 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; From 9da1b65c1f49471782b08e06e1f0f94f57d4ac5e Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Thu, 23 Apr 2020 14:59:17 -0700 Subject: [PATCH 4/5] formatting --- .../google_maps_flutter/ios/Classes/GoogleMapController.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m index dedd2d45d184..321ddd318536 100644 --- a/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m +++ b/packages/google_maps_flutter/google_maps_flutter/ios/Classes/GoogleMapController.m @@ -133,7 +133,8 @@ - (void)observeValueForKeyPath:(NSString*)keyPath 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. + // So We only care about the size of the `_mapView`, ignore the frame changes when the size is + // zero. return; } _cameraDidInitialSetup = YES; From 4cbdcb7cb282955fa3bd72448b40be7a05e6b27d Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Thu, 23 Apr 2020 16:11:47 -0700 Subject: [PATCH 5/5] Trigger ci