[go: nahoru, domu]

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

Compose GoogleMap without jank #141

Open
bubenheimer opened this issue Jun 9, 2022 · 16 comments
Open

Compose GoogleMap without jank #141

bubenheimer opened this issue Jun 9, 2022 · 16 comments
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@bubenheimer
Copy link
Contributor

Adding GoogleMap to the composition causes huge amounts of jank. The UI thread can be blocked on the order of seconds.

When incorporating MapView via AndroidView directly, a developer has some control and flexibility about working around MapView's jank issues upon initialization/loading. When incorporating GoogleMap via android-maps-compose, the library should offer options to reign in jank; I as a developer have less control over mitigating jank directly. Otherwise please point out jank mitigation strategies.

My jankiest use case displays GoogleMap in an accompanist HorizontalPager. Once GoogleMap enters the composition the pager stops in the middle of flipping through pages, for one or two seconds on emulator. The best mitigation I have found so far displays a "Loading...." page initially and waits with adding GoogleMap to the composition until the "Loading...." page is displayed with 0 horizontallOffset, meaning the user stopped flipping through pages to patiently stare at the "Loading...." screen. It sounds ridiculous, because it is. However, this way the jank is largely obscured.

@bubenheimer bubenheimer added triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jun 9, 2022
@barbeau
Copy link
Contributor
barbeau commented Jun 9, 2022

@bubenheimer Thanks for the report!

Have you tried a release build of the app? This video by the Andorid Jetpack Compose team has some good tips for testing performance:
https://youtu.be/EOQB8PTLkpY?t=58

@bubenheimer
Copy link
Contributor Author
bubenheimer commented Jun 9, 2022

The jank is caused by MapView initialization work that blocks the main thread. It's unrelated to Compose. It's a well-known issue with MapView. Because maps-compose wraps MapView, the ability to work around the View world's MapView issues is largely taken away from me.

@arriolac
Copy link
Member
arriolac commented Jun 9, 2022

One idea is for the MapView to be cached outside of the composition so when it re-enters the composition the MapView won't have to be reinitialized. However, this approach may have some gotchas (like when should it be invalidated).

@bubenheimer you mentioned jank mitigation strategies. How would you approach that if you had full control of the AndroidView? Perhaps some of those ideas can be integrated in the lib.

@bubenheimer
Copy link
Contributor Author

The jank is much worse the first time I create a MapView and that is the primary use case that I'd hope to be optimized. When I go back to the same screen to create another MapView it's much faster. So I'm not sure that additional caching would help here.

The jank mitigation strategies that I've used for this have always been one-off; delaying or prioritizing MapView initialization to happen at specific points where jank is less obstructive. For example, in terms of "caching", preloading a map at some point could be a one-off strategy. Prior to incorporating maps-compose I was able to check what the current page is in my HorizontalPager and delay map init until that point (combined with a "Loading..." screen); this seemed to work ok for a while, but with maps-compose the behavior was different, and I've ended up using the more brittle horizontalOffset == 0 strategy I mentioned above.

One thing that looked interesting but which I never tried is AsyncLayoutInflater. That could be something to explore.

There are likely other approaches to pull this off, hopefully ones with more general applicability.

@ln-12
Copy link
Contributor
ln-12 commented Sep 27, 2022

Hey @bubenheimer, have you found a workaround yet?

I have a similar issue when navigating back from another screen. The map is destroyed when I navigate away and reconstructed when the screen is loaded the second time. As I am using a slideInHorizontally + fadeIn animation, this step is really janky, even on a Pixel 6 with a release build.

I used the AsyncLayoutInflater before in another app, but I don't see that it is compatible view the compose wrapper. You probably could create a XML layout with a map and load that using the async inflater but I did not try it out.

@bubenheimer
Copy link
Contributor Author

@ln-12 I don't have a workaround. I changed my UI to a non-paging design.

One thing I use to alleviate the lengthy Maps jank is to keep the activity around - don't destroy. Of course, Maps uses a large amount of memory that you will be wasting.

@proninyaroslav
Copy link
proninyaroslav commented Oct 3, 2022

I found the same problem with Accompanist WebView, loading even a small HTML page from memory causes jank in the composable container where the WebView is located, for example, all composables in a Column have a white background until the WebView is rendered in it.

@ln-12
Copy link
Contributor
ln-12 commented Nov 8, 2022

I opened a ticker on the issue tracker (as this problem is not specific to the maps sdk) for anyone that might be interested: https://issuetracker.google.com/issues/257110114

Edit: the issue was closed as it is not specific to compose. As suggested here I am now creating my view at app startup and save the reference in my view model. Afterwards, I simply take that instance which does not have to be reconstructed.

@proninyaroslav
Copy link

@ln-12
How to be in a situation when there can be a lot of MapView/WebView instances? For example, loading multiple pages in a backstack.

@ln-12
Copy link
Contributor
ln-12 commented Nov 10, 2022

@proninyaroslav I case of the webview, I simply provide the saved view instance via the factory parameter. The url is set from my view model inside the web view state on each initial composition. I think you would also have to use the same instance multiple times with updated configurations in case of a map view.

That's of course no perfect solution, but in my specific case it is good enough for the moment.

@proninyaroslav
Copy link
proninyaroslav commented Nov 10, 2022

@ln-12
I tried to implement this but I'm getting an exception. I'm using Compose Navigation. First, I load page A, then on a tap inside page A, page B is loaded (A goes to the backstack). Both pages use the same WebView instance. When I click the back button, then I go from page B to page A (from the backstack) and at that moment an exception occurs:

java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child's parent first.
   at android.view.ViewGroup.addViewInner(ViewGroup.java:5248)
   at android.view.ViewGroup.addView(ViewGroup.java:5077)
   at android.view.ViewGroup.addView(ViewGroup.java:5017)
   at android.view.ViewGroup.addView(ViewGroup.java:4989)
   at androidx.compose.ui.viewinterop.AndroidViewHolder.setView$ui_release(AndroidViewHolder.android.kt:95)
   at androidx.compose.ui.viewinterop.ViewFactoryHolder.setFactory(AndroidView.android.kt:179)
   at androidx.compose.ui.viewinterop.AndroidView_androidKt$AndroidView$1.invoke(AndroidView.android.kt:116)
   at androidx.compose.ui.viewinterop.AndroidView_androidKt$AndroidView$1.invoke(AndroidView.android.kt:113)
   at androidx.compose.ui.viewinterop.AndroidView_androidKt$AndroidView$$inlined$ComposeNode$1.invoke(Composables.kt:254)

Maybe this is a problem in Accompanist WebView.

@stale
Copy link
stale bot commented Jun 18, 2023

This issue has been automatically marked as stale because it has not had recent activity. Please comment here if it is still valid so that we can reprioritize. Thank you!

@stale stale bot added the stale label Jun 18, 2023
@ln-12
Copy link
Contributor
ln-12 commented Jun 18, 2023

This issue has been automatically marked as stale because it has not had recent activity. Please comment here if it is still valid so that we can reprioritize. Thank you!

I think this issue should still be considered.

@stale stale bot removed the stale label Jun 18, 2023
@philip-segerfast
Copy link
philip-segerfast commented Jun 20, 2023

Would probably be useful in the LazyColumn(or Pager etc) scenario if we were able to pass our own MapView to the GoogleMap composable so it can re-use a cached one instead of creating new instances for each item. This would allow us to mitigate jank easily.
Could be a good temporary solution.

Currently I have to use Maps Compose in a RecyclerView.ViewHolder to mitigate jank which is highly unpractical.

@kikoso kikoso added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed triage me I really want to be triaged. labels Jul 10, 2023
@railianmaksym
Copy link

Hey guys, someone find a solution or any updates about this issue?
I see a big lag during opening the map tab in HorizontalPager so I believe I should move to the android view solution for the map.

@sakaijunsoccer
Copy link

As @proninyaroslav said, I have the same problem with Web View. I would like to know if there is any way to do work around. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

9 participants