Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Google maps plugin on Android fails to center initial location when surface size is changed. #106750

Closed
bparrishMines opened this issue Jun 28, 2022 · 6 comments · Fixed by flutter/engine#34414 or flutter/plugins#6097
Assignees
Labels
p: maps Google Maps plugin P1 High-priority issues at the top of the work list package flutter/packages repository. See also p: labels.

Comments

@bparrishMines
Copy link
Contributor

Steps to Reproduce

Running the following test fails when using Flutter master: https://github.com/flutter/plugins/blob/main/packages/google_maps_flutter/google_maps_flutter/example/integration_test/google_maps_test.dart#L428

The test runs successfully when the following line is removed https://github.com/flutter/plugins/blob/main/packages/google_maps_flutter/google_maps_flutter/example/integration_test/google_maps_test.dart#L428

await tester.binding.setSurfaceSize(const Size(800.0, 600.0));
Logs
06-27 08:46:06.083: I/flutter(22833): The following TestFailure was thrown running a test:
06-27 08:46:06.083: I/flutter(22833): Expected: <1100>
06-27 08:46:06.083: I/flutter(22833):   Actual: <540>
06-27 08:46:06.083: I/flutter(22833): When the exception was thrown, this was the stack:
06-27 08:46:06.083: I/flutter(22833): #4      main.<anonymous closure> (file:///tmp/cirrus-ci-build/packages/google_maps_flutter/google_maps_flutter/example/integration_test/google_maps_test.dart:464:7)
06-27 08:46:06.083: I/flutter(22833): <asynchronous suspension>
06-27 08:46:06.083: I/flutter(22833): #5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:171:15)
06-27 08:46:06.083: I/flutter(22833): <asynchronous suspension>
06-27 08:46:06.083: I/flutter(22833): #6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:841:5)
06-27 08:46:06.083: I/flutter(22833): <asynchronous suspension>
06-27 08:46:06.083: I/flutter(22833): This was caught by the test expectation on the following line:
06-27 08:46:06.083: I/flutter(22833):   file:///tmp/cirrus-ci-build/packages/google_maps_flutter/google_maps_flutter/example/integration_test/google_maps_test.dart line 464
06-27 08:46:06.083: I/flutter(22833): The test description was:
06-27 08:46:06.083: I/flutter(22833):   testInitialCenterLocationAtCenter
06-27 08:46:06.084: I/flutter(22833): 
@bparrishMines bparrishMines added plugin p: maps Google Maps plugin P1 High-priority issues at the top of the work list and removed P1 High-priority issues at the top of the work list labels Jun 28, 2022
@bparrishMines bparrishMines self-assigned this Jun 28, 2022
@bparrishMines
Copy link
Contributor Author

cc @dnfield @cyanglaz

@cyanglaz
Copy link
Contributor

cyanglaz commented Jun 30, 2022

The issue is that virtual display changes size based on the current physcial screen size: https://github.com/flutter/engine/blob/main/shell/platform/android/io/flutter/plugin/platform/VirtualDisplayController.java#L47-L74, so GoogleMap uses the new virtual display size. On the other hand, the GoogleMap flutter widget doesn't know virtual display has resized.
This leads to test trying to compare values based on 2 different sizes.

We can do 1 of the following:

  1. Revert the code that resizing the Virtual Display. This will crash user that set a virtual display size that is bigger than the current screen size. This crash is the default Android behavior and I think it is OK to align the behavior with Android.
  2. Update the test so that the surface size is smaller than the physical screen. Even we can probably find a very small number to ensure the size is smaller than the screen, this is still not a good solution as it doesn't cover all the cases.
  3. Only run the test on Hybrid Composition, not a good solution as it doesn't cover virtual display.

I'd go with 1.

@dnfield
Copy link
Contributor

dnfield commented Jun 30, 2022

I think reverting the code that resizes for now is fine. But we should still file a bug for this. In general, we should avoid letting Dart code cause native crashes - it ends up breaking hot reload workflows and results in poor DX and UX.

@bparrishMines
Copy link
Contributor Author

I agree with doing the first solution. Assuming we will continue to log the error and file a bug.

@bparrishMines
Copy link
Contributor Author

Fixed with flutter/plugins#6097

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2022
@flutter-triage-bot flutter-triage-bot bot added the package flutter/packages repository. See also p: labels. label Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: maps Google Maps plugin P1 High-priority issues at the top of the work list package flutter/packages repository. See also p: labels.
Projects
None yet
3 participants