mkPytsNortal
commented
2 years ago 
Closed felix-mittermeier closed 2 years ago
mkPytsNortal
commented
2 years ago
huycozy
commented
2 years ago Hi, I can reproduce this issue with provided sample code on latest stable and master channel.
Actually, there is an un-merged patch fixes that need to be updated in the example code.
deckerst
commented
2 years ago I see the same issue with huawei_map (buggy on any Android API version), as well as google_maps_flutter (buggy on Android API <=22, but fine on API >=23).
When this happens, clipping the platform view and drawing widgets over it fails too. It's like the platform view is completely independent of the Flutter widget tree.
littleGnAl
commented
2 years ago Any update on this?
jason-simmons
commented
2 years ago @blasten
blasten
commented
2 years ago @felix-mittermeier We changed how platform views are rendered by default in Flutter 3.0.0. Since the native view com.mapbox.mapboxsdk.maps.MapView uses GL directly, you would need to remove the check useHybridComposition, and replace initSurfaceAndroidView for initExpensiveAndroidView.
@override
Widget buildView(
Map<String, dynamic> creationParams,
OnPlatformViewCreatedCallback onPlatformViewCreated,
Set<Factory<OneSequenceGestureRecognizer>>? gestureRecognizers) {
if (defaultTargetPlatform == TargetPlatform.android) {
- if (useHybridComposition) {
return PlatformViewLink(
viewType: 'plugins.flutter.io/mapbox_gl',
surfaceFactory: (
BuildContext context,
PlatformViewController controller,
) {
return AndroidViewSurface(
controller: controller as AndroidViewController,
gestureRecognizers: gestureRecognizers ??
const <Factory<OneSequenceGestureRecognizer>>{},
hitTestBehavior: PlatformViewHitTestBehavior.opaque,
);
},
onCreatePlatformView: (PlatformViewCreationParams params) {
- final SurfaceAndroidViewController controller =
+ final AndroidViewController controller =
- PlatformViewsService.initSurfaceAndroidView(
+ PlatformViewsService.initExpensiveAndroidView(
id: params.id,
viewType: 'plugins.flutter.io/mapbox_gl',
layoutDirection: TextDirection.ltr,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
onFocus: () => params.onFocusChanged(true),
);
controller.addOnPlatformViewCreatedListener(
params.onPlatformViewCreated,
);
controller.addOnPlatformViewCreatedListener(
onPlatformViewCreated,
);
- controller.create();
return controller;
},
);
- } else {
- return AndroidView(
- viewType: 'plugins.flutter.io/mapbox_gl',
- onPlatformViewCreated: onPlatformViewCreated,
- gestureRecognizers: gestureRecognizers,
- creationParams: creationParams,
- creationParamsCodec: const StandardMessageCodec(),
- );
- }
} else if (defaultTargetPlatform == TargetPlatform.iOS) {
return UiKitView(
viewType: 'plugins.flutter.io/mapbox_gl',
onPlatformViewCreated: onPlatformViewCreated,
gestureRecognizers: gestureRecognizers,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
);
}
return Text(
'$defaultTargetPlatform is not yet supported by the maps plugin');
}I also suggest to add a Flutter SDK constraint to pubspec.yaml, so the new version of the plugin requires Flutter 3.0.0 or higher.
deckerst
commented
2 years ago @blasten thank you for the explanation. Do you know if we can expect a fix for google_maps_flutter soon? As mentioned previously, it's buggy on API <= 22.
littleGnAl
commented
2 years ago I'm not very sure whether this change will affect the performance who use Flutter SDK < 3.0 or not, should we keep the AndroidView implementation for capability?
felix-ht
commented
2 years ago @felix-mittermeier We changed how platform views are rendered by default in Flutter 3.0.0. Since the native view
com.mapbox.mapboxsdk.maps.MapViewuses GL directly, you would need to remove the checkuseHybridComposition, and replaceinitSurfaceAndroidViewforinitExpensiveAndroidView.
@blasten What will be the consequences of this for old Flutter versions?
felix-mittermeier
commented
2 years ago @blasten Thanks for your response but I am not sure your suggested changes are the solution to this issue. I tried it out and tested it but now the app crashes instantly as soon as the platform view is visible.
Physcial test device was the Google Pixel 3 XL running on Android 12.
mkPytsNortal
commented
2 years ago @blasten thanks, seems like that works for us with custom map view for Android
littleGnAl
commented
2 years ago @blasten Thanks for your response but I am not sure your suggested changes are the solution to this issue. I tried it out and tested it but now the app crashes instantly as soon as the platform view is visible.
Click this to show crash log Physcial test device was the Google Pixel 3 XL running on Android 12.
I face this crash too, I think this issue should be reopened. @felix-mittermeier
blasten
commented
2 years ago @felix-mittermeier Thanks for the report. We missed a commit in the stable branch. I just filed a request to cherry pick the required commit. Could you confirm that this change works on the master channel?
blasten
commented
2 years ago I'm not very sure whether this change will affect the performance who use Flutter SDK < 3.0 or not, should we keep the AndroidView implementation for capability?
@littleGnAl No, it doesn't. The affected plugin would need to add a Flutter SDK constraint, so pub resolves to the right version of the plugin depending on the version of Flutter that is used. https://dart.dev/tools/pub/pubspec#flutter-sdk-constraints
littleGnAl
commented
2 years ago @felix-mittermeier Thanks for the report. We missed a commit in the stable branch. I just filed a request to cherry pick the required commit. Could you confirm that this change works on the master channel?
@blasten Thanks for the reply, it works fine on the master channel for my case.
littleGnAl
commented
2 years ago I'm not very sure whether this change will affect the performance who use Flutter SDK < 3.0 or not, should we keep the AndroidView implementation for capability?
@littleGnAl No, it doesn't. The affected plugin would need to add a Flutter SDK constraint, so pub resolves to the right version of the plugin depending on the version of Flutter that is used. https://dart.dev/tools/pub/pubspec#flutter-sdk-constraints
@blasten How can I compatible it without adding the Flutter SDK constraint? Our plugin supported the Flutter SDK >= 2.0, I'm not very willing to increase the minimal support for this change.
felix-mittermeier
commented
2 years ago @felix-mittermeier Thanks for the report. We missed a commit in the stable branch. I just filed a request to cherry pick the required commit. Could you confirm that this change works on the master channel?
@blasten Thanks for your answer. I tried it out switching to the master branch and the app still crashes immediately due to some native code but the message changed. The one I posted above is gone now but now we get this instead:
Edit: But this looks more MapBox related now because the onStyleLoadedCallback got called twice. We are not sure yet if this is some kind of MapBox issue or if the platform view get initialized twice for some reasons now. Will further investigate on this but cherry picking the one commit you mentioned into stable would definitely fix the first issue 👍🏻
blasten
commented
2 years ago But this looks more MapBox related now because the onStyleLoadedCallback got called twice.
@felix-mittermeier - I verified this behavior, and it wasn't intended. I was able to fix the issue by removing the create() call. Please see the updated patch: https://github.com/flutter/flutter/issues/103630#issuecomment-1129224567
On a side note, MapBox appears to be using Android's SurfaceView. Is there any way that MapBox can use Android's TextureView? TextureView allows you to use the fast path in the Flutter engine, so it improves performance, and fixes this issue too.
apkuhar
commented
2 years ago Even the Virtual Display method is broken in 3.0.1. I'm using AndroidView to show the camera preview. and after updating to 3.0.1, On Android 10: i can't round the corners on the native view anymore and cant render any content on top of it. On Android 7: i get an empty gray view.
AAverin
commented
2 years ago @blasten Debugging Mapbox, it looks like PlatformViewCreatedCallback is triggered twice by Flutter, using latest Flutter master.
littleGnAl
commented
2 years ago Even the Virtual Display method is broken in 3.0.1. I'm using AndroidView to show the camera preview. and after updating to 3.0.1, On Android 10: i can't round the corners on the native view anymore and cant render any content on top of it. On Android 7: i get an empty gray view.
@apkuhar On Android 7, I think you should set the setZOrderOnTop or setZOrderMediaOverlay for the SurfaceView
apkuhar
commented
2 years ago I took @blasten recommendation with initExpensiveAndroidView. Visually it worked like AndroidView in Flutter 2.10. I could clip it to round the corners, I could overlay over it. But the performance is much worse than AndroidView in Flutter 2.10.
Edit: Kind of sad, because same app on iOS using Flutter 3.0 is great.
AAverin
commented
2 years ago Any update on this major issue?
Mapbox maps are now basically unusable due to PlatformViewCreatedCallback is triggered twice by Flutter, resulting in all the logic running twice and breaking the map.
AAverin
commented
2 years ago Applying all the suggested fixes, there are big performance issues and still some problems with native view overlapping everything on Android
felix-mittermeier
commented
2 years ago Applying all the suggested fixes, there are big performance issues and still some problems with native view overlapping everything on Android
Oh dear, that does not sound very satisfying and makes the impression there is no quick fix for it :/ But thanks a lot for testing it again with the mentioned change.
AAverin
commented
2 years ago I have tried to do a fix for Mapbox in this PR: https://github.com/flutter-mapbox-gl/maps/pull/1059 Even applying all the suggestions, there is still an issue with native view displayed on top of everything when navigating to another screen and a new native NPE that was not present there before.
blasten
commented
2 years ago there is still an issue with native view displayed on top of everything when navigating to another screen and a new native NPE that was not present there before.
@AAverin I was looking at https://github.com/flutter-mapbox-gl/maps/pull/1059, but I didn't see the steps to repro. Let me know how to reproduce the issue. I'm looking at this issue to see what can be done to improve performance.
AAverin
commented
2 years ago @blasten Is there some kind of minimal example of a platform plugin I can play with to reproduce the problem?
benneca
commented
2 years ago so does this affect all uses of Android's SurfaceView? I have this issue when using the adv_camera plugin as well and it also uses a surface view. I can correct the issue when switching the AndroidView to a PlatformViewLink with AndroidViewSurface (like the example above), provided I switch to the master channel. However, I still have the same issue on stable even with the PlatformViewLink. @blasten you mentioned something about cherry-picking, will this resolve the issue only when switching to the PlatformViewLink with AndroidViewSurface (like the provided example) or will this cherry-picking work with the AndroidView. Or is this ultimately an issue with Androids' SurfaceView not being compatible with clipping?
p-shapovalov
commented
2 years ago so does this affect all uses of Android's SurfaceView?
I'm afraid so. In my custom plugin platform view with AR shows on top of Stack widget and overlap other widgets - same as in several linked issues above. Surprisingly it also add periodic freezes for ios version. I'm afraid for now there is no way to upgrade to 3.0 if there is a PlatformView somewhere in project
felix-mittermeier
commented
2 years ago Would be quite a drastic step but maybe it would make sense to revert all the changes made in the flutter framework related to platform views (to the state in 2.10.5). If it is way more buggy now and the performance is worse I don't see the advantages of these changes currently. Feel free to correct me if I am wrong but that is just my feeling.
Ahmadre
commented
2 years ago My whole Flutter UI is hidden behind my PlatformView on Android.
I cannot use Flutter 3.* because of this Regression-Error....
blasten
commented
2 years ago @felix-mittermeier I'm looking at this issue. I think it's possible to do a soft-revert for SurfaceView. At the moment, I'm working on improving the tests, so this regression don't happen in the future.
sirmamedical
commented
2 years ago Would be quite a drastic step but maybe it would make sense to revert all the changes made in the flutter framework related to platform views (to the state in 2.10.5). If it is way more buggy now and the performance is worse I don't see the advantages of these changes currently. Feel free to correct me if I am wrong but that is just my feeling.
I believe this is the right step. Nothing related to the platform views works properly now. And because there are other breaking changes, staying with the 2.10.5 is not an option because the newer versions of the most of the plugins are incompatible with the previous Flutter version. Please do a hotfix and revert it to the previous state.
blasten
commented
2 years ago https://github.com/flutter/engine/pull/33599 will fix this issue
blasten
commented
2 years ago This screen recording was taken after applying https://github.com/flutter/engine/pull/33599 to the mapbox GL example app on the master branch.
https://user-images.githubusercontent.com/1410613/170386680-fc230915-930c-49af-81cb-b9eaef62d315.mp4
The only issue I saw is this fatal exception from the mapboxsdk. It does not seem related to platform views. Has anyone seen this issue before?
I used a device running Android 13 (T).
AAverin
commented
2 years ago @blasten I will be happy to test once the fix is merged to master.
The exception also started with Flutter 3.0 and I don't yet know what is causing it exactly. It is an NPE and nothing changed in the way mapbox was integrated in the recent updates.
vitable-dev
commented
2 years ago Are there any updates on this? Would it be possible to create a separate branch of flutter and merge https://github.com/flutter/engine/pull/33599 so we who don't build for Linux can be unblocked?
vitable-dev
commented
2 years ago Any updates on this?
sirmamedical
commented
2 years ago This is a breaking bug for many applications, because it completely breaks the UI. Please, make applying the fix top priority, it is not just a P4!!!
farabisameer
commented
2 years ago this is causing major issues on our apps! Can you please prioritize this fix! Thanks!!
c0dezli
commented
2 years ago Please this is a P1 breaking bug, we need a fix for this asap
felix-mittermeier
commented
2 years ago @sirmamedical @farabisameer @C0deZLee While I agree with you that it is important of course, it has to be said that Flutter has a strict priority system. Comments like "P1 breaking bug" are therefore not useful at all IMO because P0/P1 are only bugs that prevent the whole Flutter tool from building. So in other words "ultra urgent for ALL Flutter developers" - but if you are not using platform views in your app, you won't even notice this issue.
As written here: https://github.com/flutter/flutter/wiki/Issue-hygiene
P3 is the highest priority a bug can normally have.
So this bug could be promoted to P3 (which probably also would make sense according to the count of thumbs) but that's it. I read this kind of comments so often but there is a clear system for priorities and I think this has to be respected instead of questioned over and over.
Here is the overview:
penhorwood
commented
2 years ago @felix-mittermeier I understand and agree with your priority system Flutter has in place. As for this bug it does seems to be a regression created by the Flutter team in version 3.0.0 and continues in 3.0.1. While I can downgrade to 2.10.5 to build my app all versions of 3.x are completely broken. Not sure how that doesn't equal a regression.
pitazzo
commented
2 years ago This issue is also affecting our team. In order to evaluate the possibility of downgrading the Flutter SDK, how long should we expect to take merging https://github.com/flutter/engine/pull/33599 ?
Rossdex
commented
2 years ago This is affecting our app and stopping a new release - we use the mapbox_gl plugin and have a map on our first page.
The map stays visible when the user opens the drawer or pushes a page on top.
felix-mittermeier
commented
2 years ago This is affecting our app and stopping a new release - we use the mapbox_gl plugin and have a map on our first page.
The map stays visible when the user opens the drawer or pushes a page on top.
Can't you use Flutter 2.10.5 for building the release? That's how we do it currently - and if this gets fixed we'll start using the 3.x.x version.
Edit: Don't understand why I get downvotes for a "question"? This was just a suggestion because using 2.10.5 to build the release is still better than not being able to build a release at all, right? 🤔
penhorwood
commented
2 years ago What I don't get is 4 days ago I was told this issue was waiting on a test case wrapped up in this pull request: https://github.com/flutter/engine/pull/33574. The pull request was merged but the pull request https://github.com/flutter/engine/pull/33599 is still a Draft. So what else are we waiting on?
cfsbhawkins
commented
2 years ago What I don't get is 4 days ago I was told this issue was waiting on a test case wrapped up in this pull request: flutter/engine#33574. The pull request was merged but the pull request flutter/engine#33599 is still a Draft. So what else are we waiting on?
If I'm following right, it looks like the test cases might have been reverted out by another dev because builds were failing? See: https://github.com/flutter/engine/pull/33813
penhorwood
commented
2 years ago I read over the pull request. I find the way they do their stuff very confusing. We use git and pull request with an online system at work but nothing this confusing.
Steps to Reproduce:
Build the example app from https://github.com/flutter-mapbox-gl/maps with Flutter 3.0.0, run it and open for example "Place symbol". How you can see in the screenshot in https://github.com/flutter-mapbox-gl/maps/issues/1041 the platform view (the map) is mispositioned and even drawn under the status bar.
Expected results:
The map gets drawn within its parent container and fills it. To see how it is supposed to look like you can run the app with any previous flutter version like 2.10.5.
Due to the fact that this issue only affect Android I assume this is related to some changes to the Android platform views which also were mentioned in the release notes on medium.com.
Logs
``` [✓] Flutter (Channel stable, 3.0.0, on macOS 12.3.1 21E258 darwin-arm, locale de-DE) • Flutter version 3.0.0 at /Users/f/Documents/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision ee4e09cce0 (3 days ago), 2022-05-09 16:45:18 -0700 • Engine revision d1b9a6938a • Dart version 2.17.0 • DevTools version 2.12.2 [✓] Android toolchain - develop for Android devices (Android SDK version 32.1.0-rc1) • Android SDK at /Users/f/Library/Android/sdk • Platform android-32, build-tools 32.1.0-rc1 • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 13.3.1) • Xcode at /Applications/Xcode.app/Contents/Developer • CocoaPods version 1.11.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2021.1) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763) [✓] VS Code (version 1.67.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.40.0 [✓] Connected device (3 available) • Pixel 3 XL (mobile) • 192.168.37.124:5555 • android-arm64 • Android 12 (API 31) • iPhone (mobile) • fb804538ab944b670e4061e2b3978d027625c70b • ios • iOS 14.6 18F72 • Chrome (web) • chrome • web-javascript • Google Chrome 101.0.4951.64 [✓] HTTP Host Availability • All required HTTP hosts are available • No issues found! ```