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

Consider using the new AndroidView in 1.4.0-rc01 #285

Closed
arriolac opened this issue Mar 8, 2023 · 3 comments · May be fixed by #436
Closed

Consider using the new AndroidView in 1.4.0-rc01 #285

arriolac opened this issue Mar 8, 2023 · 3 comments · May be fixed by #436
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

@arriolac
Copy link
Member

arriolac commented Mar 8, 2023

Is your feature request related to a problem? Please describe.
The new AndroidView introduced in 1.4.0-rc01 is optimized to reuse the underlying View when used within a lazy list. Maps Compose usage within lazy lists (LazyColumn, LazyRow, Pager, etc.) could see some performance gains with this new change.

Describe the solution you'd like
Try/test out the new AndroidView overload but wait to release once 1.4.0 is stable.

Describe alternatives you've considered
Do nothing.

Additional context
https://developer.android.com/jetpack/androidx/releases/compose-ui#1.4.0-rc01

@arriolac arriolac added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. triage me I really want to be triaged. labels Mar 8, 2023
@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
@kikoso
Copy link
Collaborator

kikoso commented Sep 29, 2023

This has been already fixed in #377

@kikoso kikoso closed this as completed Sep 29, 2023
@philip-segerfast
Copy link

philip-segerfast commented Oct 5, 2023

This issue is not fixed.
The dependency is updated but AndroidView doesn't specify an onReset implementation which is necessary for enabling re-use of the underlying View.

From the javadoc:

By default, AndroidView does not automatically pool or reuse Views. If placed inside of a reusable container (including inside a LazyRow or LazyColumn), the View instances will always be discarded and recreated if the composition hierarchy containing the AndroidView changes, even if its group structure did not change and the View could have conceivably been reused.
Views are eligible for reuse if AndroidView is given a non-null onReset callback. Since it is expensive to discard and recreate View instances, reusing Views can lead to noticeable performance improvements — especially when building a scrolling list of AndroidViews. It is highly recommended to specify an onReset implementation and opt-in to View reuse when possible.

The current implementation looks like this which isn't specifying an onReset implementation (v3.0.0):

AndroidView(modifier = modifier, factory = { mapView })

Currently, if the GoogleMap composable is placed in a LazyColumn, the mapView is re-created/disposed for every item while scrolling.

This is very important for our app because it shows multiple maps in a scrollable column. Meanwhile we're stuck with RecyclerView.

@arriolac
Copy link
Member Author

More info is documented in: https://developer.android.com/jetpack/compose/migrate/interoperability-apis/views-in-compose#androidview_in_lazy_lists Basically you'll need to update the implementation to provide a non-null onReset value on the AndroidView overload.

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
3 participants