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

Clustering sometimes show default markers #569

Open
ArisGuimera opened this issue May 10, 2024 · 16 comments · May be fixed by #615
Open

Clustering sometimes show default markers #569

ArisGuimera opened this issue May 10, 2024 · 16 comments · May be fixed by #615
Assignees
Labels
triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@ArisGuimera
Copy link

I have a screen that contain a list of custom markers with clustering and works fine. The problem is sometime after open de screen my google maps show my customs markers AND the default markers on the same place.

Is just random, sometimes when screen is open first time, sometimes when screen is open after 5 times.

Please be sure to include as much information as possible:

Environment details

  • com.google.maps.android:maps-compose:4.4.2
  • com.google.maps.android:maps-compose-utils:4.4.2
  • compileSdk 34
  • Android 12 (API 31)

Steps to reproduce

  1. Navigate to the Map screens
  2. Go back
  3. Repeat

Code example

@Composable
    fun MapScreen(stepResult: HomeState.StepResult) {
    val sheetState = rememberModalBottomSheetState(
        initialValue = ModalBottomSheetValue.Hidden
    )

    stepResult.games.let { games ->
        val items = games.map {
            MapTenant(
                itemPosition = LatLng(it.latLng.latitude, it.latLng.longitude),
                itemTitle = it.name,
                itemSnippet = it.address,
                itemZIndex = 0f,
                item = it
            )
        }

        if (items.isEmpty()) {
            return
        }

        Box {
            TenantMapList(sheetState, items)
        }
    }
}

@Composable
fun GoogleMapClustering(items: List<MapTenant>, onMapSelected: (List<MapTenant>) -> Unit) {
    var cameraState = rememberCameraPositionState {
        position = CameraPosition.fromLatLngZoom(items.first().itemPosition, 10f)
    }

    GoogleMap(
        modifier = Modifier
            .fillMaxSize()
            .padding(top = 32.dp),
        cameraPositionState = cameraState
    ) {
        CustomUiClustering(items = items, onMapSelected = {
            onMapSelected(it)
        }, onItemSelected = {
            onMapSelected(listOf(it))
        })
    }
}

@OptIn(MapsComposeExperimentalApi::class)
@Composable
private fun CustomUiClustering(
    items: List<MapTenant>,
    onMapSelected: (List<MapTenant>) -> Unit,
    onItemSelected: (MapTenant) -> Unit
) {
    Clustering(items = items,
        onClusterClick = {
            onMapSelected(it.items.toList())
            false
        }, onClusterItemClick = {
            onItemSelected(it)
            true
        },
        clusterContent = { cluster ->
            CircleContent(
                modifier = Modifier.size(40.dp),
                text = "%,d".format(cluster.size),
            )
        },
        clusterItemContent = {
            CircleContent(
                modifier = Modifier.size(20.dp),
                text = "",
            )
        })
}

@Composable
private fun CircleContent(
    text: String,
    modifier: Modifier = Modifier,
) {
    Surface(
        modifier,
        shape = CircleShape,
        color = ComplementaryPopUp,
        contentColor = Color.White,
        border = BorderStroke(2.dp, defaultGradient)
    ) {
        Box(contentAlignment = Alignment.Center) {
            Text(
                text, fontSize = 16.sp, fontWeight = FontWeight.Black, textAlign = TextAlign.Center
            )
        }
    }
}

@Composable
fun TenantMapList(
    sheetState: ModalBottomSheetState,
    items: List<MapTenant>,
) {
    var selectedTenants by remember { mutableStateOf<List<MapTenant>>(emptyList()) }

    ModalBottomSheetLayout(sheetState = sheetState,
        sheetShape = RoundedCornerShape(topStart = 16.dp, topEnd = 16.dp),
        sheetBackgroundColor = ComplementaryPopUp,
        scrimColor = MaterialTheme.colors.onSurface.copy(alpha = 0.50f),
        sheetContent = {
            TenantsList(Modifier.wrapContentHeight(), selectedTenants)
        }) {

        val scope = rememberCoroutineScope()

        GoogleMapClustering(items) {
            selectedTenants = it
            scope.launch {
                sheetState.show()
            }
        }
    }
}

Thanks!

@ArisGuimera ArisGuimera 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 May 10, 2024
@wangela
Copy link
Member

wangela commented May 10, 2024

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

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

@ThomasLefebvre
Copy link

Hi Aris,

I have exactly the same problem, it's very random, sometimes it happens after the 50th launch, sometimes on the first.

Apart from the graphics bug, what's more annoying is the crash if the user clicks on the default marker.

I'll let you know if I make any progress,

Thomas

@ArisGuimera
Copy link
Author

Hi Aris,

I have exactly the same problem, it's very random, sometimes it happens after the 50th launch, sometimes on the first.

Apart from the graphics bug, what's more annoying is the crash if the user clicks on the default marker.

I'll let you know if I make any progress,

Thomas

I haven't found any solution, but if I fixed I will let you know

@kikoso kikoso self-assigned this May 14, 2024
@kikoso
Copy link
Collaborator

kikoso commented May 14, 2024

Hi @ArisGuimera , @ThomasLefebvre ,

What you get is similar to the one described in this issue?

#549

@ArisGuimera
Copy link
Author

Hi @ArisGuimera , @ThomasLefebvre ,

What you get is similar to the one described in this issue?

#549

I think so but my Android version is 12.

@JakeChandrasakera
Copy link

I have the same issue, it occasionally happens on debug builds but occurs a lot more for release builds.

I can also replicate the issue if I use

public fun <T : ClusterItem> Clustering(
    items: Collection<T>,
    clusterManager: ClusterManager<T>,
)

where I use rememberClusterRenderer and set the clustering algorithm to NonHierarchicalViewBasedAlgorithm.

If I update the items list to be empty after the default markers appear over my custom ones, then the custom ones get removed, but the default ones remain.

Clearing all the markers with a MapEffect and calling map.clear() works, but I haven't found a way to programmatically detect when the default markers are being displayed.

@Thomaswguy
Copy link

We are seeing the same issue 😔. Any advise or help on this would be greatly appreciated!

@mars885
Copy link

mars885 commented Jun 27, 2024

My team has also encountered this issue. We are using the 5.0.4 version of the lib.

@ArisGuimera
Copy link
Author

My team has also encountered this issue. We are using the 5.0.4 version of the lib.

Amazing!!

@cwsiteplan
Copy link

+1

2 similar comments
@alexNguyenDM
Copy link

+1

@kbatrak
Copy link

kbatrak commented Jul 25, 2024

+1

@gmazzotta-bit
Copy link

(I deleted my previous message because it was wrong).

After debugging a bit I think I have a better understanding of what is going on. From what I can see ClusterManager has no way to set immediately a ClusterRenderer and it takes a bit for ComposeUiClusterRenderer to be ready and set as renderer. I suspect that if you have your cluster items ready and add them immediately to the map, the default renderer kicks in before the custom one takes its place.

I'm currently using the suggested implementation for the case in which I need a custom renderer, this one. If I wait for both ClusterManager and ClusterRenderer to be non null before adding the items (here), then I stop seeing the default markers.

If I don't wait for ClusterRenderer and add a delay before setting the renderer here, I first see the default markers and after the delay they get replaced with the custom markers. So I think there is maybe something missing in DefaultClusterRenderer.onRemove(). My guess would be that potentially ongoing/queued RenderTasks are not cancelled, but I did not properly verify this.

@gmazzotta-bit
Copy link

I forgot to mention that if you are not using a custom renderer, then you are likely using this.

This line should probably be:

if (clusterManager != null && renderer != null) {

You can easily copy that function into your project and see if doing the above helps.

@cwsiteplan
Copy link

@gmazzotta-bit thanks for your hints - that sounds very reasonable 👍

@darronschall
Copy link

darronschall commented Aug 21, 2024

I ran into this as well on a production app and came up with essentially the same workaround that @gmazzotta-bit suggested in #569 (comment)

Here's two screenshots of the bug I was experiencing, where the default marker (both single item and cluster) displayed underneath my composable marker content:

image

image

I extracted my fix into PR #615. I have not seen the issue reproduce since incorporating this change into my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet