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

Add and default to Hybrid composition on Android #916

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

yoavrofe
Copy link
Collaborator

Using Virtual Display on Android 12 may lead to problems.
This PR adds and defaults to [Hybrid Composition](Hybrid Composition). The problem I noticed doesn't exist when Hybrid Composition is used.

I suggest using Hybrid Composition for android 10 and above, and setting MapboxMap.useHybridComposition=false on Android 9 and below.

This is the default also on the Google Maps plugin maintained by the Flutter team, although there's an error there where the property useAndroidViewSurface is reversed.

@felix-ht
Copy link
Collaborator

felix-ht commented Feb 22, 2022

Would this cause issues on android 9 below if the useHybridComposition is not set to false?

if yes it might be a good idea to set the default based on the android version here - and not have user miss it and wonder why its broken for some devices.

@felix-ht
Copy link
Collaborator

felix-ht commented Mar 2, 2022

@yoavrofe any updates?

@yoavrofe
Copy link
Collaborator Author

yoavrofe commented Mar 2, 2022

I agree that this would be better. However, determining the Android version requires native code or an external package, and is asynchronous, so it may add to the complexity of the library. I personally decided to do it when initializing my app.

@felix-ht
Copy link
Collaborator

felix-ht commented Mar 2, 2022

@yoavrofe The asynchronous nature makes this much harder indeed. I also checked for hybrid composition for android <= 9 - it seems to just cause a slight reduction in performance.

So lets get this merged - can you run flutter format?

Might also be great if you could add some short documentation on the use of hybrid composition to the readme.

Copy link
Collaborator

@felix-ht felix-ht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the formatting and added code to the the composition mode in the example

@felix-ht felix-ht merged commit b29092e into flutter-mapbox-gl:master Mar 16, 2022
@felix-ht felix-ht had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN March 16, 2022 15:08 Failure
@felix-ht felix-ht had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN March 16, 2022 15:08 Failure
@felix-ht felix-ht had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN March 16, 2022 15:09 Failure
@felix-ht felix-ht had a problem deploying to ANDROID_CI_DOWNLOADS_TOKEN March 16, 2022 15:09 Failure
@AAverin
Copy link
Contributor

AAverin commented Mar 24, 2022

I am getting really weird behaviour, seemingly after this PR.
I have a map on the screen, then I push another route on the top that has a different map instance inside. And then suddenly my old map from the old route is displayed on top of everything.
Is this something that can be caused by Hybrid composition?

@felix-ht
Copy link
Collaborator

@AAverin hard to say - you can just turn of hybrid composition to check if this is the cause

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants