From 6261ed888d362147f56bcf31149a853c0d1efc40 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Mon, 29 Aug 2022 16:22:09 -0400 Subject: [PATCH 1/3] Change the default mode --- .../google_maps_flutter_android/CHANGELOG.md | 5 +++ .../google_maps_flutter_android/README.md | 42 +++++++++++++++++++ .../example/build.excerpt.yaml | 15 +++++++ .../example/lib/readme_excerpts.dart | 21 ++++++++++ .../example/pubspec.yaml | 1 + .../lib/src/google_maps_flutter_android.dart | 17 +++----- .../google_maps_flutter_android/pubspec.yaml | 2 +- .../google_maps_flutter_android_test.dart | 15 ++++++- 8 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 packages/google_maps_flutter/google_maps_flutter_android/example/build.excerpt.yaml create mode 100644 packages/google_maps_flutter/google_maps_flutter_android/example/lib/readme_excerpts.dart 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 38dff3d82470..dbf360f7368d 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,8 @@ +## 2.3.0 + +* Switches the default for `useAndroidViewSurface` to true, and adds + information about the current mode behaviors to the README. + ## 2.2.0 * Updates `useAndroidViewSurface` to require Hybrid Composition, making the diff --git a/packages/google_maps_flutter/google_maps_flutter_android/README.md b/packages/google_maps_flutter/google_maps_flutter_android/README.md index 5ac1df04a4df..877b9bbe9102 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/README.md +++ b/packages/google_maps_flutter/google_maps_flutter_android/README.md @@ -1,5 +1,7 @@ # google\_maps\_flutter\_android + + The Android implementation of [`google_maps_flutter`][1]. ## Usage @@ -8,5 +10,45 @@ This package is [endorsed][2], which means you can simply use `google_maps_flutter` normally. This package will be automatically included in your app when you do. +## Display Mode + +This plugin supports two different [platform view display modes][3]. The default +display mode is subject to change in the future, and will not be considered a +breaking change, so if you want to ensure a specific mode you can set it +explicitly: + + +```dart +import 'package:google_maps_flutter_android/google_maps_flutter_android.dart'; +import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart'; + +void main() { + // Require Hybrid Composition mode on Android. + final GoogleMapsFlutterPlatform mapsImplementation = + GoogleMapsFlutterPlatform.instance; + if (mapsImplementation is GoogleMapsFlutterAndroid) { + mapsImplementation.useAndroidViewSurface = true; + } + // ยทยทยท +} +``` + +### Hybrid Composition + +This is the current default mode, and corresponds to +`useAndroidViewSurface = true`. It ensures that the map display will work as +expected, at the cost of some performance. + +### Texture Layer Hybrid Composition + +This is a new display mode used by most plugins starting with Flutter 3.0, and +corresponds to `useAndroidViewSurface = false`. This is more performant than +Hybrid Composition, but currently [misses certain map updates][4]. + +This mode will likely become the default in future versions if/when the +missed updates issue can be resolved. + [1]: https://pub.dev/packages/google_maps_flutter [2]: https://flutter.dev/docs/development/packages-and-plugins/developing-packages#endorsed-federated-plugin +[3]: https://docs.flutter.dev/development/platform-integration/android/platform-views +[4]: https://github.com/flutter/flutter/issues/103686 diff --git a/packages/google_maps_flutter/google_maps_flutter_android/example/build.excerpt.yaml b/packages/google_maps_flutter/google_maps_flutter_android/example/build.excerpt.yaml new file mode 100644 index 000000000000..e317efa11cb3 --- /dev/null +++ b/packages/google_maps_flutter/google_maps_flutter_android/example/build.excerpt.yaml @@ -0,0 +1,15 @@ +targets: + $default: + sources: + include: + - lib/** + # Some default includes that aren't really used here but will prevent + # false-negative warnings: + - $package$ + - lib/$lib$ + exclude: + - '**/.*/**' + - '**/build/**' + builders: + code_excerpter|code_excerpter: + enabled: true diff --git a/packages/google_maps_flutter/google_maps_flutter_android/example/lib/readme_excerpts.dart b/packages/google_maps_flutter/google_maps_flutter_android/example/lib/readme_excerpts.dart new file mode 100644 index 000000000000..5911c062e444 --- /dev/null +++ b/packages/google_maps_flutter/google_maps_flutter_android/example/lib/readme_excerpts.dart @@ -0,0 +1,21 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/material.dart'; +// #docregion DisplayMode +import 'package:google_maps_flutter_android/google_maps_flutter_android.dart'; +import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart'; + +void main() { + // Require Hybrid Composition mode on Android. + final GoogleMapsFlutterPlatform mapsImplementation = + GoogleMapsFlutterPlatform.instance; + if (mapsImplementation is GoogleMapsFlutterAndroid) { + mapsImplementation.useAndroidViewSurface = true; + } + // #enddocregion DisplayMode + runApp(const MaterialApp()); + // #docregion DisplayMode +} +// #enddocregion DisplayMode diff --git a/packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml index fc1059bbdcd5..db2bc733d4ff 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml +++ b/packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml @@ -21,6 +21,7 @@ dependencies: google_maps_flutter_platform_interface: ^2.2.1 dev_dependencies: + build_runner: ^2.1.10 espresso: ^0.1.0+2 flutter_driver: sdk: flutter diff --git a/packages/google_maps_flutter/google_maps_flutter_android/lib/src/google_maps_flutter_android.dart b/packages/google_maps_flutter/google_maps_flutter_android/lib/src/google_maps_flutter_android.dart index 95dea4c5d938..06c5bdcd7e0f 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/lib/src/google_maps_flutter_android.dart +++ b/packages/google_maps_flutter/google_maps_flutter_android/lib/src/google_maps_flutter_android.dart @@ -471,19 +471,14 @@ class GoogleMapsFlutterAndroid extends GoogleMapsFlutterPlatform { return _channel(mapId).invokeMethod('map#takeSnapshot'); } - /// Set [GoogleMapsFlutterPlatform] to use [AndroidViewSurface] to build the Google Maps widget. + /// Set [GoogleMapsFlutterPlatform] to use [AndroidViewSurface] to build the + /// Google Maps widget. /// - /// This implementation uses hybrid composition to render the Google Maps - /// Widget on Android. This comes at the cost of some performance on Android - /// versions below 10. See - /// https://flutter.dev/docs/development/platform-integration/platform-views#performance for more - /// information. + /// See https://pub.dev/packages/google_maps_flutter_android#display-mode + /// for more information. /// - /// If set to true, the google map widget should be built with - /// [buildViewWithTextDirection] instead of [buildView]. - /// - /// Defaults to false. - bool useAndroidViewSurface = false; + /// Currently defaults to true, but the default is subject to change. + bool useAndroidViewSurface = true; Widget _buildView( int creationId, 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 2ee7c820db43..c820f31d1c46 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/plugins/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.2.0 +version: 2.3.0 environment: sdk: ">=2.14.0 <3.0.0" diff --git a/packages/google_maps_flutter/google_maps_flutter_android/test/google_maps_flutter_android_test.dart b/packages/google_maps_flutter/google_maps_flutter_android/test/google_maps_flutter_android_test.dart index cba23d072dbf..431c2472945e 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/test/google_maps_flutter_android_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_android/test/google_maps_flutter_android_test.dart @@ -124,9 +124,10 @@ void main() { }); test( - 'Default widget is AndroidView', + 'Does not use PlatformViewLink when using TLHC', () async { final GoogleMapsFlutterAndroid maps = GoogleMapsFlutterAndroid(); + maps.useAndroidViewSurface = false; final Widget widget = maps.buildViewWithConfiguration(1, (int _) {}, widgetConfiguration: const MapWidgetConfiguration( initialCameraPosition: @@ -150,4 +151,16 @@ void main() { expect(widget, isA()); }); + + testWidgets('Defaults to surface view', (WidgetTester tester) async { + final GoogleMapsFlutterAndroid maps = GoogleMapsFlutterAndroid(); + + final Widget widget = maps.buildViewWithConfiguration(1, (int _) {}, + widgetConfiguration: const MapWidgetConfiguration( + initialCameraPosition: + CameraPosition(target: LatLng(0, 0), zoom: 1), + textDirection: TextDirection.ltr)); + + expect(widget, isA()); + }); } From 8dc8684979507cae344a6c19dfa35ef69b014c89 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 30 Aug 2022 10:43:06 -0400 Subject: [PATCH 2/3] Deflake test --- .../example/integration_test/google_maps_test.dart | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart b/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart index 8fc1ede0eb0f..d7fa28660212 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart @@ -922,7 +922,16 @@ void main() { expect(iwVisibleStatus, false); await controller.showMarkerInfoWindow(marker.markerId); - iwVisibleStatus = await controller.isMarkerInfoWindowShown(marker.markerId); + // The Maps SDK doesn't return true for whether it is show immediately after + // showing it, so wait for it to become visible. + for (int i = 0; i < 100; i++) { + iwVisibleStatus = + await controller.isMarkerInfoWindowShown(marker.markerId); + await tester.pump(); + if (iwVisibleStatus) { + break; + } + } expect(iwVisibleStatus, true); await controller.hideMarkerInfoWindow(marker.markerId); From 25acd669629a81a1a24bceba3036f1f404088d59 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 30 Aug 2022 14:30:24 -0400 Subject: [PATCH 3/3] More deflaking --- .../integration_test/google_maps_test.dart | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart b/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart index d7fa28660212..0945740b1e45 100644 --- a/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_test.dart @@ -23,6 +23,28 @@ void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); GoogleMapsFlutterPlatform.instance.enableDebugInspection(); + // Repeatedly checks an asynchronous value against a test condition, waiting + // on frame between each check, returing the value if it passes the predicate + // before [maxTries] is reached. + // + // Returns null if the predicate is never satisfied. + // + // This is useful for cases where the Maps SDK has some internally + // asynchronous operation that we don't have visibility into (e.g., native UI + // animations). + Future waitForValueMatchingPredicate(WidgetTester tester, + Future Function() getValue, bool Function(T) predicate, + {int maxTries = 100}) async { + for (int i = 0; i < maxTries; i++) { + final T value = await getValue(); + if (predicate(value)) { + return value; + } + await tester.pump(); + } + return null; + } + testWidgets('uses surface view', (WidgetTester tester) async { final GoogleMapsFlutterAndroid instance = GoogleMapsFlutterPlatform.instance as GoogleMapsFlutterAndroid; @@ -484,12 +506,13 @@ void main() { final ExampleGoogleMapController mapController = await mapControllerCompleter.future; + // Wait for the visible region to be non-zero. final LatLngBounds firstVisibleRegion = - await mapController.getVisibleRegion(); - - expect(firstVisibleRegion, isNotNull); - expect(firstVisibleRegion.southwest, isNotNull); - expect(firstVisibleRegion.northeast, isNotNull); + await waitForValueMatchingPredicate( + tester, + () => mapController.getVisibleRegion(), + (LatLngBounds bounds) => bounds != zeroLatLngBounds) ?? + zeroLatLngBounds; expect(firstVisibleRegion, isNot(zeroLatLngBounds)); expect(firstVisibleRegion.contains(_kInitialMapCenter), isTrue); @@ -520,9 +543,6 @@ void main() { final LatLngBounds secondVisibleRegion = await mapController.getVisibleRegion(); - expect(secondVisibleRegion, isNotNull); - expect(secondVisibleRegion.southwest, isNotNull); - expect(secondVisibleRegion.northeast, isNotNull); expect(secondVisibleRegion, isNot(zeroLatLngBounds)); expect(firstVisibleRegion, isNot(secondVisibleRegion)); @@ -922,16 +942,13 @@ void main() { expect(iwVisibleStatus, false); await controller.showMarkerInfoWindow(marker.markerId); - // The Maps SDK doesn't return true for whether it is show immediately after - // showing it, so wait for it to become visible. - for (int i = 0; i < 100; i++) { - iwVisibleStatus = - await controller.isMarkerInfoWindowShown(marker.markerId); - await tester.pump(); - if (iwVisibleStatus) { - break; - } - } + // The Maps SDK doesn't always return true for whether it is shown + // immediately after showing it, so wait for it to report as shown. + iwVisibleStatus = await waitForValueMatchingPredicate( + tester, + () => controller.isMarkerInfoWindowShown(marker.markerId), + (bool visible) => visible) ?? + false; expect(iwVisibleStatus, true); await controller.hideMarkerInfoWindow(marker.markerId);