android-maps-compose icon indicating copy to clipboard operation
android-maps-compose copied to clipboard

[GoogleMap]: Markers aren't cleared out when provided backing field changes

Open bmc08gt opened this issue 2 years ago • 7 comments

Environment details

Kotlin 1.9.10 AGP 8.3.0-alpha11 Maps Compose 4.1.1 Compose Plugin 1.5.10

Steps to reproduce

  1. Add a GoogleMap() composable and render MarkerComposables from a List<T> provided by a ViewModel State or other data source
  2. Change the List<T> contents
  3. Observe the contents within the markers change but not the positions

Code example

@Composable
actual fun MapView(
    modifier: Modifier,
    contentPadding: PaddingValues,
    userLocation: LatLong?,
    resultLocations: List<Stay.Minimal>,
    useTotalPrice: Boolean,
    onMarkerSelectionChange: (String?) -> Unit,
    onMapMoved: () -> Unit,
) {
    [...]

    BoxWithConstraints(modifier = modifier) {
        GoogleMap(
            modifier = Modifier.matchParentSize(),
            contentPadding = contentPadding,
            contentDescription = "Map view for properties with applied filters and search parameters",
            cameraPositionState = cameraPositionState,
            properties = properties,
            uiSettings = uiSettings,
            onMapClick = { currentSelectedMarkerId = null }
        ) {
            resultLocations.forEach { listing ->
                val location = LatLng(listing.location.latitude, listing.location.longitude)
                val markerState = rememberMarkerState(
                    position = location
                )
                MarkerComposable(
                    keys = arrayOf(
                        listing.id,
                        location,
                        useTotalPrice,
                        currentSelectedMarkerId ?: "",
                        previousSelectedMarkers
                    ),
                    state = markerState,
                    tag = listing.id,
                    onClick = {
                        currentSelectedMarkerId = it.tag as? String
                        true
                    }
                ) {
                    PricePill(
                        price = "${listing.totalPriceOfStay()}".formatAsMoney(),
                        isSelected = currentSelectedMarkerId == listing.id,
                        wasPreviouslySelected = previousSelectedMarkers.contains(listing.id)
                    )
                }
            }

            [...]
        }
    }
}

However by creating my own MutableState for the List, initialized as emptyList() I'm able to have the map clear and redraw the new markers.

var markersToDraw by remember(resultLocations) {
      mutableStateOf(emptyList<Stay.Minimal>())
}
GoogleMap(
    modifier = Modifier.matchParentSize(),
    contentPadding = contentPadding,
    contentDescription = "Map view for properties with applied filters and search parameters",
    cameraPositionState = cameraPositionState,
    properties = properties,
    uiSettings = uiSettings,
    onMapClick = { currentSelectedMarkerId = null }
) {
    markersToDraw.forEach { listing ->
        val location = LatLng(listing.location.latitude, listing.location.longitude)
        val markerState = rememberMarkerState(
            position = location
        )
        MarkerComposable(
            keys = arrayOf(
                listing.id,
                location,
                useTotalPrice,
                currentSelectedMarkerId ?: "",
                previousSelectedMarkers
            ),
            state = markerState,
            tag = listing.id,
            onClick = {
                currentSelectedMarkerId = it.tag as? String
                true
            }
        ) {
            PricePill(
                price = "${listing.totalPriceOfStay()}".formatAsMoney(),
                isSelected = currentSelectedMarkerId == listing.id,
                wasPreviouslySelected = previousSelectedMarkers.contains(listing.id)
            )
        }
    }

    [...]

    LaunchedEffect(resultLocations) {
        markersToDraw = resultLocations
    }
}

Stack trace

N/A

Following these steps will guarantee the quickest resolution possible.

Thanks!

bmc08gt avatar Nov 10 '23 16:11 bmc08gt

If you would like to upvote the priority of this issue, please comment below or react on the original post above with :+1: so we can see what is popular when we triage.

@bmc08gt 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.

wangela avatar Nov 10 '23 16:11 wangela

Hi @bmc08gt ,

Your approach is correct. We have this sample here that provides a snippet showcasing how to update individual markers.

Maybe a question on the snippet above: are you sure the keys are unique on each update? They should trigger the recomposition.

kikoso avatar Nov 13 '23 11:11 kikoso

@bmc08gt I believe the most significant problem is your usage of rememberMarkerState(). You never actually update the MarkerState location. The rememberMarkerState() position parameter is only used for state initialization, not state updates.

bubenheimer avatar Nov 13 '23 17:11 bubenheimer

@bmc08gt I believe the most significant problem is your usage of rememberMarkerState(). You never actually update the MarkerState location. The rememberMarkerState() position parameter is only used for state initialization, not state updates.

Im creating entirely new markers with new coordinates (each with their own MarkerState). The markers were being reused even though I was reiterating through an entirely new list with new unique identifiers. They isn't always going to be the same amount of markers every time being rendered in the map (list is filtered results).

bmc08gt avatar Nov 13 '23 17:11 bmc08gt

I am having the same issue. Basically I move the map with a new set of markers to display. At some point (after a couple of moves) the markers don't update anymore. They're still showing in the old position, even though the Map was recomposed with the new positions and I can see the marker contents were recomposed as well. The fix with a backing mutable state works for me as well.

Edit: they key (ID) given to the MarkerComposable is unique every time, per every distinct marker.

oblakr24 avatar Nov 20 '23 12:11 oblakr24

I have the same issue.

I try to display one marker at a time. When clicking on the map, I attempt to update both the marker's position and its displayed text. While the position updates successfully, the text remains unchanged.

Code example: `

    var markerPosition: LatLng? by remember {
            mutableStateOf(null)
        }
        var counter by remember {
            mutableStateOf(0)
        }
        GoogleMap(
            modifier = Modifier.fillMaxSize(),
            cameraPositionState = cameraPositionState,
            onMapClick = {
                markerPosition = it
                counter++
            }) {
            markerPosition?.let {
                MarkerComposable(state = MarkerState(position = it)) {
                    Card {
                        Text(text = "marker $counter")
                    }
                }
            }
       }

`

I tried to migrate to MarkerComposable from solution with Bitmap descriptor. Unfortunately, I can no longer use the previous solution because, after a library update (not precisely sure which update), it results in constant flickering of the Marker, and it makes impossible to process click events on the marker.

LozariaVinok avatar Dec 20 '23 10:12 LozariaVinok

@bmc08gt in your original example, do you reuse the List object and simply clear and completely repopulate it when you send updates? If so, it may help to use a completely new List object for your "entirely new list".

What I believe is happening: Compose tries to align the old contents of the list with the new contents, and trying to reuse whatever it composed before, in particular remembered MarkerState. Because you never tell Compose to update the marker positions, you get new Marker contents, but old positions.

If it does not work despite using a new List object every time, additionally wrap your forEach loop with the new List object as a key:

key(resultLocations) {
    resultLocations.forEach {...}
}

bubenheimer avatar Jan 02 '24 22:01 bubenheimer