googlemaps / android-maps-compose

Jetpack Compose composables for the Maps SDK for Android
https://developers.google.com/maps/documentation/android-sdk/maps-compose
Apache License 2.0
1.15k stars 135 forks source link

Using rememberCameraPositionState and observing it via a lambda, makes entire map recompose instead of just of the lambda #216

Open ForceGT opened 1 year ago

ForceGT commented 1 year ago

I am using the rememberCameraPositionState like so, and then exposing a callback from my custom google map wrapper to observe the state

val cameraPositionState = 
        rememberCameraPositionState {
            position =
                CameraPosition.fromLatLngZoom(
                    mapInitialTargetLocation,
                    14f
                )

//Custom callback exposed from map            
 onMapCameraPositionStateChanged: (CameraPositionState) -> Unit = {},

However when I try to use this map, and trigger an operation from the viewModel, it recomposes infinitely

fun ParentComposable(){

 MapContent(viewModel::onCameraPositionStateChanged)

}

//Calling this function from above parent composable like so
// CustomerMap is my own wrapper around GoogleMap for convenience

private fun MapContent(
    onCameraStateChanged : (CameraPositionState) -> Unit
) {
        CustomerMap(
            ......,
            onMapCameraPositionStateChanged = onCameraStateChanged
         }

What am I doing wrong, or is it just a Google Map issue?

arriolac commented 1 year ago

That behavior is expected—this is why you can't set a camera change listener on the GoogleMap composable function. cameraPositionState.position changes as the camera is changed so you can observe based on that.

ForceGT commented 1 year ago

I could find out that the culprit was another unstable parameter in my composable wrapper around GoogleMap But as I said, I was observing CameraPositionState, and expected behaviour was only the lambda which returns the state should change, and not the entire GoogleMap

ForceGT commented 1 year ago

@arriolac Thanks for your answer, but exposing CameraPositionState is important for my business logic, since we are observing many more values, rather than just the position. What would you recommend mend in that case?

arriolac commented 1 year ago

I would have to see more of your implementation here like how is onMapCameraPositionStateChanged implemented? Also, what is the event handling code doing?

ForceGT commented 1 year ago

I finally removed the callback, and exposed a wrapper around the CameraPositionState, like so, where CustomerMap is a custom wrapper around GoogleMap

CustomerMap(
   ....
    myCameraPositionState: MutableState<MyCameraPositionState> = rememberMyCameraPositionState(),
)

where my rememberMyCameraPositionState is something like this

Composable
fun rememberMyCameraPositionState(
    key: String? = null,
    init: (() -> MyCameraPositionState)? = null
): MutableState<MyCameraPositionState> {
    return rememberSaveable(key, saver = MyCameraPositionState.Saver) {
        mutableStateOf(init?.invoke() ?: MyCameraPositionState.NONE)
    }
}

And then in my CustomerMap composable, I use a LaunchedEffect to update this

 val cameraPositionState =
        rememberCameraPositionState( )

    LaunchedEffect(cameraPositionState.position to cameraPositionState.isMoving) {
        myCameraPositionState.value = cameraPositionState.mapToMyCameraPositionState
    }

The main idea is that any dependency related to maps or maps-compose should not be needed outside this current module (where the map resides) to keep the dependencies clean

However I still feel, taking a MutableState as a method parameter, is not correct, what do you think?

aleksisjoberg commented 1 year ago

I think I have the same issue. This is especially a problem with TileOverlay, as it gets needlessly recomposed (=tiles removed & added) every time as the parent composable (GoogleMap) recomposes. Is there any way to prevent that, while still being able to observe changes to camera's position, which is needed to load more markers?

EDIT: Passing the TileOverlay as a parameter from the parent composable fixed my issue. At this point I don't really understand what's going on and why it works, but keeping the TileOverlay hierarchically "above" the component that manages map's state was the fix for me.

stale[bot] commented 1 year ago

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!