fix: Fixed scope cancellation that leads to bug when invalidating composable doesn't invalidate cluster

[y9san9]

Thank you for opening a Pull Request!

---

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #537 🦕

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[y9san9]

Signed CLA

[pioPirrung]

@dkhawk any updates here? This also seems to fix an issue related to canceling the image loader of coil. I think multiple people are interested in this fix

Issues: #210 #385 #542

[patrickcapadmi]

Is there any movement on this PR? We'd quite like to get this fix in to help images load in more reliably.

[y9san9]

By the way, my case was also image loading :)
But I have some other questions regarding how builtin clustering works, so I just copied the code and made my modifications

[kikoso]

@dkhawk any updates here? This also seems to fix an issue related to canceling the image loader of coil. I think multiple people are interested in this fix

Issues: #210 #385 #542

@pioPirrung , this does not seem to fix the Coil related stuff. The image is still not being rendered.

[pioPirrung]

@dkhawk any updates here? This also seems to fix an issue related to canceling the image loader of coil. I think multiple people are interested in this fix
Issues: #210 #385 #542

@pioPirrung , this does not seem to fix the Coil related stuff. The image is still not being rendered.

hey @kikoso, thanks for your quick reply and checking onto this issue. I tested it again locally and for me everything runs fine. The DebugLogger of coil does indeed still print that the request has been canceled for the given urls, but every image is shown instantly and correctly for me with this changes in the PR. I tested it now at least 10 times in a row. 🤷🏻‍♂️ Even with fresh install when the image cache is empty works every time. Without the changes in this PR I can reproduce that the image not getting rendered every time.

I'm using a NonHierarchicalViewBasedAlgorithm for the Clustering as someone in the issues mentioned that this helped him to solve the problem. So maybe this is why its working for me 🤷🏻‍♂️

[kikoso]

@pioPirrung, this does not seem to fix the Async loading of a Coil image, see the following code snippet:

GoogleMap(
               modifier = Modifier.fillMaxSize()
           ) {
               MarkerComposable(
                   keys = arrayOf("singapore4"),
                   state = MarkerState(
                       position = LatLng(
                           25.0, 25.0
                       )
                   ),
               ) {
                   Column(
                       horizontalAlignment = Alignment.CenterHorizontally,
                       modifier = Modifier
                           .clip(RoundedCornerShape(8.dp))
                           .background(Color.White),
                   ) {

                       AsyncImage(
                           model = "https://picsum.photos/200/300",
                           contentDescription = null,
                           contentScale = ContentScale.Crop,
                           modifier = Modifier
                               .padding(top = 4.dp, start = 6.dp, end = 6.dp)
                               .size(50.dp)
                               .clip(CircleShape)
                               .border(2.dp, Color.Black, CircleShape)
                       )
                       Text(
                           maxLines = 1,
                           textAlign = TextAlign.Center,
                           text = "Hello",
                           modifier = Modifier
                               .width(60.dp),
                           style = MaterialTheme.typography.body1
                       )
                   }
               }
           }

Under which circumstances is it fixing it for you? Can you provide an snippet, ideally?

[pioPirrung]

Thanks for your reply @kikoso. I did not tried it with plain MarkerComposable. For me (and I think the PR only targets Clustering) the fix works under the circumstance while using a Cluster. As I mentioned above I'm using a NonHierarchicalViewBasedAlgorithm. I did not tried it with the default one for Clustering.

Code Snippet

@OptIn(MapsComposeExperimentalApi::class)
@Composable
fun TestMapScreen() {
    val cameraPositionState = rememberCameraPositionState {
        position = CameraPosition.fromLatLngZoom(LatLng(49.897288, 10.8783964), 7f)
    }

    Box(
        modifier = Modifier.fillMaxSize()
    ) {
        GoogleMap(
            modifier = Modifier
                .fillMaxSize(),
            onMapLoaded = { },
            cameraPositionState = cameraPositionState,
            uiSettings = MapUiSettings(
                myLocationButtonEnabled = false,
                mapToolbarEnabled = false,
                zoomControlsEnabled = false
            )
        ) {
            val configuration = LocalConfiguration.current
            val screenHeight = configuration.screenHeightDp.dp
            val screenWidth = configuration.screenWidthDp.dp
            val clusterManager = rememberClusterManager<MyItem>()
            clusterManager?.setAlgorithm(
                NonHierarchicalViewBasedAlgorithm(
                    screenWidth.value.toInt(),
                    screenHeight.value.toInt()
                )
            )
            val renderer = rememberClusterRenderer(
                clusterContent = { cluster ->
                    MyItemClusterContent(
                        modifier = Modifier
                            .size(30.dp, 38.dp)
                            .clip(MarkerShape())
                            .background(
                                Color(
                                    red = 255,
                                    green = 165,
                                    blue = 0
                                )
                            ),
                        cluster = cluster
                    )
                },
                clusterItemContent = { item ->
                    MyItemMarker(item)
                },
                clusterManager = clusterManager,
            )
            SideEffect {
                clusterManager ?: return@SideEffect
                clusterManager.setOnClusterItemClickListener {
                    true
                }
            }
            SideEffect {
                if (clusterManager?.renderer != renderer && renderer != null) {
                    (renderer as DefaultClusterRenderer).minClusterSize = 2
                    clusterManager?.renderer = renderer
                }
            }

            if (clusterManager != null) {
                Clustering(
                    items = previewData,
                    clusterManager = clusterManager,
                )
            }
        }
    }
}

@Composable
fun MyItemMarker(item: MyItem) {
    val context = LocalContext.current
    Column(
        horizontalAlignment = Alignment.CenterHorizontally
    ) {
        Box(
            modifier = Modifier
                .size(30.dp, 38.dp)
                .clip(MarkerShape())
                .background(Color.Green),
        ) {
            AsyncImage(
                modifier = Modifier
                    .size(30.dp)
                    .padding(5.dp),
                model = ImageRequest.Builder(context)
                    .data(item.itemUrl)
                    .allowHardware(false)
                    .crossfade(false)
                    .error(R.drawable.placeholder)
                    .diskCacheKey(item.id)
                    .memoryCacheKey(item.id)
                    .fallback(R.drawable.placeholder)
                    .placeholder(R.drawable.placeholder)
                    .build(),
                contentDescription = null
            )
        }
        Column(
            modifier = Modifier.width(150.dp),
        ) {
            Text(
                modifier = Modifier.fillMaxWidth(),
                text = item.itemTitle,
                style = MaterialTheme.typography.labelSmall,
                textAlign = TextAlign.Center
            )
        }
    }
}

@Composable
fun MyItemClusterContent(
    modifier: Modifier = Modifier,
    cluster: Cluster<MyItem>
) {
    Column(
        horizontalAlignment = Alignment.CenterHorizontally
    ) {
        Box(
            modifier = modifier
        ) {
            Text(
                modifier = Modifier
                    .padding(5.dp)
                    .fillMaxWidth(),
                text = "%,d".format(cluster.size),
                fontSize = 16.sp,
                fontWeight = FontWeight.Black,
                textAlign = TextAlign.Center
            )
        }
        Column(
            modifier = Modifier.width(130.dp),
        ) {
            Text(
                modifier = Modifier.fillMaxWidth(),
                text = cluster.items.stream().findFirst().get().title,
                style = MaterialTheme.typography.bodyMedium,
                textAlign = TextAlign.Center
            )
            Text(
                modifier = Modifier.fillMaxWidth(),
                text = "+${cluster.size - 1} more",
                style = MaterialTheme.typography.labelSmall,
                textAlign = TextAlign.Center
            )
        }
    }
}

data class MyItem(
    val id: String = UUID.randomUUID().toString(),
    val itemPosition: LatLng,
    val itemTitle: String,
    val itemUrl: String = "https://picsum.photos/200/300",
) : ClusterItem {
    override fun getPosition(): LatLng =
        itemPosition

    override fun getTitle(): String =
        itemTitle

    override fun getSnippet(): String? = null

    override fun getZIndex(): Float? = null
}

val markerMain = LatLng(49.89, 10.87)
val previewData = listOf(
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
    MyItem(
        itemPosition = LatLng(
            markerMain.latitude + Random.nextFloat(),
            markerMain.longitude + Random.nextFloat(),
        ),
        itemTitle = "Hello"
    ),
)

And here is also a video of proof.

Screen_recording_20240417_182257.mp4

[kikoso]

Hi @pioPirrung . I was a bit confused after reading this comment, since they are specifically targeting MarkerComposable.

The fix does indeed seem to prevent the Coil Coroutine from being canceled and let the image load from either network or cache (you can activate the debugger on Coil and search for RealImageLoader). It does not fix the related Coil issues (#210 #385 #542) on the MarkerComposable, but that can be tackled on a different PR.

Verifying that this does not have any non-intended behaviors.

[pioPirrung]

Hi @kikoso, thanks again for your really quick reply. Ah now I see, sorry for the confusion. Initially I thought that this PR would fix it for MarkerComposable and for Clustering, but this isn't the case, that's correct. So sorry for that!

[danprado]

Confirming this fix worked for me when trying to load an AsyncImage with Coil.
Note: This fixes it for Clustering only. MarkerComposable is still broken.

[googlemaps-bot]

🎉 This PR is included in version 4.4.1 🎉

The release is available on:

• v4.4.1
• GitHub release

Your semantic-release bot 📦🚀

[jialbanc]

This issue is still unfixed when using only MarkerComposable