[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

Updating contentPadding causes recompositions #293

Open
lucas-livefront opened this issue Mar 17, 2023 · 7 comments
Open

Updating contentPadding causes recompositions #293

lucas-livefront opened this issue Mar 17, 2023 · 7 comments
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

@lucas-livefront
Copy link
lucas-livefront commented Mar 17, 2023

Introduction

The crux of my issue is that when I provide contentPadding to the GoogleMap component it causes recompositions. I need to update the content padding often because I have a bottom sheet for displaying search results! (we modeled it after the Google Maps app) If I have many pins (20-50) on the map the UX is severely hindered and there is a lot of lag.

We really want to update the content padding dynamically as it creates a nice UX and allows us to better utilize the GoogleMap animation APIs. Not to mention, it allows us to keep the Google logo visible.

Anyway, since this API is merely a wrapper around the XML Google Maps implementation I suspect there are some limitations to how well you can optimize this behavior. However, I must expose this issue as it isn't an easy thing to fix on my end and it seriously affects the major production app that I work on with many millions of users.

I can do a few things to kludge this:

  1. Adjust the content padding only when the bottom sheet settles into a new position. This works but is ugly as the map components teleport around.
  2. Never update content padding and do math for centering the map and doing area searches. This isn't fun for obvious reasons.

Environment details

  1. Google Maps Compose 2.11.1
  2. Android

Steps to reproduce

I made a repo to reproduce this and illuminate the issues I am seeing!

Clone the repo!

git clone git@github.com:lucas-livefront/google-map-recompositions.git

Add your API key to local.properties by inserting this line and replacing <your-key> with a valid key!

MAPS_API_KEY=<your-key>

Then simply run the app and turn on the layout inspector and monitor the recompositions!
There are two examples: MapExample and CustomComponentExample. MapExample is meant to illustrate the many recompositions that occur when we adjust content padding. CustomComponentExample is meant to illustrate how we can avoid recompositions while still updating content padding!

In order to experiment with the two different examples go to MainActivity.kt and you can swap them out!

        setContent {
            MapsRecompositionsTheme {
                MapExample() // Swap me out with CustomComponentExample()
            }
        }

Demos

MapExample CustomComponentExample
Screen.Recording.2023-03-17.at.2.25.15.PM.mov
Screen.Recording.2023-03-17.at.2.27.12.PM.mov

April 7, 2023 Update

Due to the comment by @pseudoankit I added a MapEffectExample as well. This helps illustrate the pros and cons of that approach.

@lucas-livefront lucas-livefront added triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Mar 17, 2023
@wangela
Copy link
Member
wangela commented Mar 17, 2023

If you would like to upvote the priority of this issue, please comment below or react with 👍 so we can see what is popular when we triage.

@lucas-livefront Thank you for opening this issue. 🙏
Please check out these other resources that might help you get to a resolution in the meantime:

This is an automated message, feel free to ignore.

@DSteve595 DSteve595 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p3 Desirable enhancement or fix. May not be included in next release. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. triage me I really want to be triaged. labels Mar 20, 2023
@DSteve595
Copy link
Contributor
DSteve595 commented Mar 20, 2023

Thanks for the very detailed report!
I think we can avoid recomposing the GoogleMap's content when padding changes. Not immediately sure how much of the performance hit is from that recomposition or just the padding being a heavy operation that likely wasn't originally meant to be animated. I'll report back with more info.

@lucas-livefront
Copy link
Author

Much appreciated. The capacity to animate in a clean way is super valuable to my app!

@pseudoankit
Copy link

@lucas-livefront I am also having some sort of similar usecase and I need to update padding multiple time but I didn't faced any recomposition
below is my approach

MapEffect(key1 = padding) { map ->
        map.setPadding(padding.start, padding.top, padding.end, padding.bottom)
}

@lucas-livefront
Copy link
Author

@pseudoankit what type of object is map? I have not seen this setPadding function exposes by the Google Maps Compose API.

@pseudoankit
Copy link

@lucas-livefront you can call MapEffect block inside GoogleCompose and check this

@lucas-livefront
Copy link
Author
lucas-livefront commented Apr 7, 2023

Very nice find @pseudoankit. I was embarrassingly unaware of the MapEffect utility.

It does completely solve the recomposition issue however the animation is slightly delayed. 🤔
Definitely an improvement!

mapEffect.mp4

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

4 participants