From 3d4bccd529da53f8983176f6e55237ef2b22a674 Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:51:35 -0600 Subject: [PATCH 1/2] feat(utils): update android-maps-utils to 6.1.0 and expose clustering executors/contexts - Bump com.google.maps.android:android-maps-utils and android-maps-utils-core to 6.1.0. - Expose mainContext and backgroundContext in rememberClusterManager to match ClusterManager coroutine context injection introduced in android-maps-utils 6.1.0 (PR #1822). - Expose executor: Executor? in rememberClusterRenderer and forward it through ReportingDefaultClusterRenderer and ComposeUiClusterRenderer to DefaultClusterRenderer (PR #1682). - Fix DisposableEffect key in Clustering.kt to key on clusterManager instead of itemsState so cluster items are not cleared and re-clustered on every items update prior to LaunchedEffect. - Override onClusterUpdated and onClusterItemUpdated in ComposeUiClusterRenderer so cached markers retain custom Compose icons, anchors, zIndex, and rotation when updated. --- gradle/libs.versions.toml | 2 +- maps-compose-utils/build.gradle.kts | 6 + .../compose/clustering/ClusterRenderer.kt | 125 ++++++++++--- .../android/compose/clustering/Clustering.kt | 128 ++++++++++++- .../compose/clustering/ClusterRendererTest.kt | 171 ++++++++++++++++++ 5 files changed, 397 insertions(+), 35 deletions(-) create mode 100644 maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e26ff992a..983d58ea8 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -30,7 +30,7 @@ materialIconsExtendedAndroid = "1.7.8" # Google Maps Platform mapsecrets = "2.0.1" -mapsutils = "6.0.0" +mapsutils = "6.1.0" # Testing androidCore = "1.7.0" diff --git a/maps-compose-utils/build.gradle.kts b/maps-compose-utils/build.gradle.kts index 27315b511..dde5aaab4 100644 --- a/maps-compose-utils/build.gradle.kts +++ b/maps-compose-utils/build.gradle.kts @@ -49,6 +49,10 @@ android { enableAndroidTestCoverage = true } } + + testOptions { + unitTests.isReturnDefaultValues = true + } } kotlin { @@ -84,5 +88,7 @@ dependencies { implementation(libs.kotlinx.coroutines.android) api(libs.maps.utils) + testImplementation(libs.kotlinx.coroutines.test) + testImplementation(libs.mockk) testImplementation(libs.test.junit) } diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt index dc5a84b0f..b4322df6c 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt @@ -43,6 +43,7 @@ import androidx.savedstate.setViewTreeSavedStateRegistryOwner import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.BitmapDescriptor import com.google.android.gms.maps.model.BitmapDescriptorFactory +import com.google.android.gms.maps.model.Marker import com.google.android.gms.maps.model.MarkerOptions import com.google.maps.android.clustering.Cluster import com.google.maps.android.clustering.ClusterItem @@ -56,6 +57,8 @@ import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import java.util.concurrent.Executor +import java.util.concurrent.Executors internal interface ClusterRendererItemState { val unclusteredItems: State> @@ -80,10 +83,12 @@ internal class ComposeUiClusterRenderer( private val clusterItemContentZIndexState: State, private val clusterContentRotationState: State, private val clusterItemContentRotationState: State, + executor: Executor = Executors.newSingleThreadExecutor(), ) : DefaultClusterRenderer( context, map, clusterManager, + executor, ), ClusterRendererItemState { @@ -127,22 +132,26 @@ internal class ComposeUiClusterRenderer( keysToViews.forEach { (key, viewInfo) -> when (key) { is ViewKey.Cluster -> { - getMarker(key.cluster)?.apply { - val props = viewInfo.view.properties - val anchor = props.anchor ?: clusterContentAnchorState.value - setAnchor(anchor.x, anchor.y) - zIndex = props.zIndex ?: clusterContentZIndexState.value - rotation = props.rotation ?: clusterContentRotationState.value + getMarker(key.cluster)?.let { marker -> + applyClusterMarkerProperties( + marker = marker, + properties = viewInfo.view.properties, + defaultAnchor = clusterContentAnchorState.value, + defaultZIndex = clusterContentZIndexState.value, + defaultRotation = clusterContentRotationState.value, + ) } } is ViewKey.Item -> { - getMarker(key.item)?.apply { - val props = viewInfo.view.properties - val anchor = props.anchor ?: clusterItemContentAnchorState.value - setAnchor(anchor.x, anchor.y) - zIndex = props.zIndex ?: clusterItemContentZIndexState.value - rotation = props.rotation ?: clusterItemContentRotationState.value + getMarker(key.item)?.let { marker -> + applyClusterMarkerProperties( + marker = marker, + properties = viewInfo.view.properties, + defaultAnchor = clusterItemContentAnchorState.value, + defaultZIndex = clusterItemContentZIndexState.value, + defaultRotation = clusterItemContentRotationState.value, + ) } } } @@ -266,22 +275,28 @@ internal class ComposeUiClusterRenderer( .collectLatest { when (key) { is ViewKey.Cluster -> { - getMarker(key.cluster)?.apply { - setIcon(renderViewToBitmapDescriptor(view)) - val anchor = view.properties.anchor ?: clusterContentAnchorState.value - setAnchor(anchor.x, anchor.y) - zIndex = view.properties.zIndex ?: clusterContentZIndexState.value - rotation = view.properties.rotation ?: clusterContentRotationState.value + getMarker(key.cluster)?.let { marker -> + marker.setIcon(renderViewToBitmapDescriptor(view)) + applyClusterMarkerProperties( + marker = marker, + properties = view.properties, + defaultAnchor = clusterContentAnchorState.value, + defaultZIndex = clusterContentZIndexState.value, + defaultRotation = clusterContentRotationState.value, + ) } } is ViewKey.Item -> { - getMarker(key.item)?.apply { - setIcon(renderViewToBitmapDescriptor(view)) - val anchor = view.properties.anchor ?: clusterItemContentAnchorState.value - setAnchor(anchor.x, anchor.y) - zIndex = view.properties.zIndex ?: clusterItemContentZIndexState.value - rotation = view.properties.rotation ?: clusterItemContentRotationState.value + getMarker(key.item)?.let { marker -> + applyClusterItemMarkerProperties( + marker = marker, + icon = renderViewToBitmapDescriptor(view), + properties = view.properties, + defaultAnchor = clusterItemContentAnchorState.value, + defaultZIndex = clusterItemContentZIndexState.value, + defaultRotation = clusterItemContentRotationState.value, + ) } } } @@ -344,6 +359,37 @@ internal class ComposeUiClusterRenderer( } } + override fun onClusterUpdated(cluster: Cluster, marker: Marker) { + super.onClusterUpdated(cluster, marker) + if (!scope.isActive) return + if (clusterContentState.value != null) { + val viewInfo = keysToViews[ViewKey.Cluster(cluster)] + applyClusterMarkerProperties( + marker = marker, + properties = viewInfo?.view?.properties, + defaultAnchor = clusterContentAnchorState.value, + defaultZIndex = clusterContentZIndexState.value, + defaultRotation = clusterContentRotationState.value, + ) + } + } + + override fun onClusterItemUpdated(item: T, marker: Marker) { + super.onClusterItemUpdated(item, marker) + if (!scope.isActive) return + if (clusterItemContentState.value != null) { + val viewInfo = keysToViews[ViewKey.Item(item)] ?: createAndAddView(ViewKey.Item(item)) + applyClusterItemMarkerProperties( + marker = marker, + icon = renderViewToBitmapDescriptor(viewInfo.view), + properties = viewInfo.view.properties, + defaultAnchor = clusterItemContentAnchorState.value, + defaultZIndex = clusterItemContentZIndexState.value, + defaultRotation = clusterItemContentRotationState.value, + ) + } + } + private fun renderViewToBitmapDescriptor(view: AbstractComposeView): BitmapDescriptor { /* AndroidComposeView triggers LayoutNode's layout phase in the View draw phase, so trigger a draw to an empty canvas to force that */ @@ -431,3 +477,34 @@ internal class ComposeUiClusterRenderer( */ internal fun resolveItemContentDescription(item: T, customContentDescription: String?): String? = customContentDescription ?: item.title + +internal fun applyClusterMarkerProperties( + marker: Marker, + properties: ClusteringMarkerProperties?, + defaultAnchor: Offset, + defaultZIndex: Float, + defaultRotation: Float, +) { + val anchor = properties?.anchor ?: defaultAnchor + marker.setAnchor(anchor.x, anchor.y) + marker.zIndex = properties?.zIndex ?: defaultZIndex + marker.rotation = properties?.rotation ?: defaultRotation +} + +internal fun applyClusterItemMarkerProperties( + marker: Marker, + icon: BitmapDescriptor, + properties: ClusteringMarkerProperties, + defaultAnchor: Offset, + defaultZIndex: Float, + defaultRotation: Float, +) { + marker.setIcon(icon) + applyClusterMarkerProperties( + marker = marker, + properties = properties, + defaultAnchor = defaultAnchor, + defaultZIndex = defaultZIndex, + defaultRotation = defaultRotation, + ) +} diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt index 11d804fb9..ef45b4d8b 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt @@ -48,8 +48,12 @@ import com.google.maps.android.compose.MapsComposeExperimentalApi import com.google.maps.android.compose.currentCameraPositionState import com.google.maps.android.compose.rememberComposeUiViewRenderer import com.google.maps.android.compose.rememberReattachClickListenersHandle +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.launch +import java.util.concurrent.Executor +import java.util.concurrent.Executors +import kotlin.coroutines.CoroutineContext /** * Properties for a marker in [Clustering]. @@ -617,7 +621,7 @@ internal fun Clustering( previousUsedExplicitKey = usesExplicitKey } } - DisposableEffect(itemsState) { + DisposableEffect(clusterManager) { onDispose { clusterManager.clearItems() clusterManager.cluster() @@ -634,22 +638,56 @@ internal fun Clustering( } } +/** + * Default Renderer for drawing Cluster and ClusterItem markers. + * + * @param clusterManager the [ClusterManager] that manages the clusters + * @param executor an optional [Executor] used by [DefaultClusterRenderer] to process view + * modifications off the main thread. Tests can pass a direct executor to make rendering + * deterministic. + */ @Composable @GoogleMapComposable @MapsComposeExperimentalApi -public fun rememberClusterRenderer(clusterManager: ClusterManager?): ClusterRenderer? { +public fun rememberClusterRenderer( + clusterManager: ClusterManager?, + executor: Executor? = null, +): ClusterRenderer? { val context = LocalContext.current val clusterRendererState: MutableState?> = remember { mutableStateOf(null) } clusterManager ?: return null - MapEffect(context) { map -> - val renderer = ReportingDefaultClusterRenderer(context, map, clusterManager) + MapEffect(context, executor) { map -> + val renderer = ReportingDefaultClusterRenderer( + context = context, + map = map, + clusterManager = clusterManager, + executor = executor ?: Executors.newSingleThreadExecutor(), + ) clusterRendererState.value = renderer } return clusterRendererState.value } +/** + * Default Renderer for drawing Cluster and ClusterItem markers. + * + * Retained for binary compatibility with binaries compiled against earlier versions. + */ +@Deprecated( + message = "Retained for binary compatibility.", + level = DeprecationLevel.HIDDEN, +) +@Composable +@GoogleMapComposable +@MapsComposeExperimentalApi +public fun rememberClusterRenderer(clusterManager: ClusterManager?): ClusterRenderer? = + rememberClusterRenderer( + clusterManager = clusterManager, + executor = null, + ) + /** * Default Renderer for drawing Composable. * @@ -661,6 +699,10 @@ public fun rememberClusterRenderer(clusterManager: ClusterMana * @param clusterItemContentZIndex the z-index of the non-clustered item * @param clusterContentRotation the rotation of the cluster in degrees clockwise about the marker's anchor point * @param clusterItemContentRotation the rotation of the non-clustered item in degrees clockwise about the marker's anchor point + * @param clusterManager the [ClusterManager] that manages the clusters + * @param executor an optional [Executor] used by [DefaultClusterRenderer] to process view + * modifications off the main thread. Tests can pass a direct executor to make rendering + * deterministic. */ @Composable @GoogleMapComposable @@ -675,6 +717,7 @@ public fun rememberClusterRenderer( clusterContentRotation: Float = 0.0f, clusterItemContentRotation: Float = 0.0f, clusterManager: ClusterManager?, + executor: Executor? = null, ): ClusterRenderer? { val clusterContentState = rememberUpdatedState(clusterContent) val clusterItemContentState = rememberUpdatedState(clusterItemContent) @@ -689,7 +732,7 @@ public fun rememberClusterRenderer( val clusterRendererState: MutableState?> = remember { mutableStateOf(null) } clusterManager ?: return null - MapEffect(context) { map -> + MapEffect(context, executor) { map -> val renderer = ComposeUiClusterRenderer( context, scope = this, @@ -704,6 +747,7 @@ public fun rememberClusterRenderer( clusterItemContentZIndexState, clusterContentRotationState, clusterItemContentRotationState, + executor = executor ?: Executors.newSingleThreadExecutor(), ) clusterRendererState.value = renderer awaitCancellation() @@ -711,18 +755,81 @@ public fun rememberClusterRenderer( return clusterRendererState.value } +/** + * Default Renderer for drawing Composable. + * + * Retained for binary compatibility with binaries compiled against earlier versions. + */ +@Deprecated( + message = "Retained for binary compatibility.", + level = DeprecationLevel.HIDDEN, +) @Composable @GoogleMapComposable @MapsComposeExperimentalApi -public fun rememberClusterManager(): ClusterManager? { +public fun rememberClusterRenderer( + clusterContent: @Composable ((Cluster) -> Unit)?, + clusterItemContent: @Composable ((T) -> Unit)?, + clusterContentAnchor: Offset = Offset(0.5f, 1.0f), + clusterItemContentAnchor: Offset = Offset(0.5f, 1.0f), + clusterContentZIndex: Float = 0.0f, + clusterItemContentZIndex: Float = 0.0f, + clusterContentRotation: Float = 0.0f, + clusterItemContentRotation: Float = 0.0f, + clusterManager: ClusterManager?, +): ClusterRenderer? = rememberClusterRenderer( + clusterContent = clusterContent, + clusterItemContent = clusterItemContent, + clusterContentAnchor = clusterContentAnchor, + clusterItemContentAnchor = clusterItemContentAnchor, + clusterContentZIndex = clusterContentZIndex, + clusterItemContentZIndex = clusterItemContentZIndex, + clusterContentRotation = clusterContentRotation, + clusterItemContentRotation = clusterItemContentRotation, + clusterManager = clusterManager, + executor = null, +) + +/** + * Remembers a [ClusterManager] configured with the given coroutine contexts. + * + * @param mainContext the coroutine context used to deliver clusters to the renderer. It must + * dispatch on the main thread. + * @param backgroundContext the coroutine context used to compute clusters off the main thread. + * Tests can pass a test dispatcher for both contexts to make clustering deterministic. + */ +@Composable +@GoogleMapComposable +@MapsComposeExperimentalApi +public fun rememberClusterManager( + mainContext: CoroutineContext = Dispatchers.Main, + backgroundContext: CoroutineContext = Dispatchers.Default, +): ClusterManager? { val context = LocalContext.current val clusterManagerState: MutableState?> = remember { mutableStateOf(null) } - MapEffect(context) { map -> - clusterManagerState.value = ClusterManager(context, map) + MapEffect(context, mainContext, backgroundContext) { map -> + clusterManagerState.value = ClusterManager(context, map, mainContext, backgroundContext) } return clusterManagerState.value } +/** + * Remembers a [ClusterManager]. + * + * Retained for binary compatibility with binaries compiled against earlier versions. + */ +@Deprecated( + message = "Retained for binary compatibility.", + level = DeprecationLevel.HIDDEN, +) +@Composable +@GoogleMapComposable +@MapsComposeExperimentalApi +public fun rememberClusterManager(): ClusterManager? = rememberClusterManager( + mainContext = Dispatchers.Main, + backgroundContext = Dispatchers.Default, +) + @OptIn(MapsComposeExperimentalApi::class) @Composable private fun rememberClusterManager( @@ -803,11 +910,12 @@ private fun ResetMapListeners(clusterManager: ClusterManager<*>) { } } -private class ReportingDefaultClusterRenderer( +internal class ReportingDefaultClusterRenderer( context: Context, map: GoogleMap, clusterManager: ClusterManager, -) : DefaultClusterRenderer(context, map, clusterManager), + executor: Executor = Executors.newSingleThreadExecutor(), +) : DefaultClusterRenderer(context, map, clusterManager, executor), ClusterRendererItemState { override val unclusteredItems = mutableStateOf(emptySet()) diff --git a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt new file mode 100644 index 000000000..aced8abbe --- /dev/null +++ b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt @@ -0,0 +1,171 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.maps.android.compose.clustering + +import android.content.Context +import android.graphics.drawable.ShapeDrawable +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.widget.TextView +import androidx.compose.ui.geometry.Offset +import com.google.android.gms.maps.GoogleMap +import com.google.android.gms.maps.model.BitmapDescriptor +import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.Marker +import com.google.maps.android.clustering.ClusterItem +import com.google.maps.android.clustering.ClusterManager +import com.google.maps.android.ui.RotationLayout +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkStatic +import io.mockk.runs +import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Test +import java.util.concurrent.Executor + +@OptIn(ExperimentalCoroutinesApi::class) +public class ClusterRendererTest { + + private data class TestItem( + override val position: LatLng, + override val title: String? = "Title", + override val snippet: String? = "Snippet", + override val zIndex: Float? = 0f, + ) : ClusterItem + + @Test + public fun testClusterManagerAndReportingRendererSupportCustomContextsAndExecutor() { + mockkStatic(LayoutInflater::class) + mockkConstructor(ShapeDrawable::class) + try { + every { anyConstructed().paint } returns mockk(relaxed = true) + val inflater = mockk(relaxed = true) + val rotationLayout = mockk(relaxed = true) + every { rotationLayout.findViewById(any()) } returns mockk(relaxed = true) + val container = mockk(relaxed = true) + every { container.getChildAt(0) } returns rotationLayout + every { inflater.inflate(any(), any()) } returns container + every { LayoutInflater.from(any()) } returns inflater + val context = mockk(relaxed = true) + val map = mockk(relaxed = true) + val mainDispatcher = UnconfinedTestDispatcher() + val backgroundDispatcher = UnconfinedTestDispatcher() + val clusterManager = ClusterManager( + context, + map, + mockk(relaxed = true), + mainDispatcher, + backgroundDispatcher, + ) + assertNotNull(clusterManager) + + val customExecutor = Executor { runnable -> runnable.run() } + val renderer = ReportingDefaultClusterRenderer( + context = context, + map = map, + clusterManager = clusterManager, + executor = customExecutor, + ) + assertNotNull(renderer) + assertEquals(emptySet(), renderer.unclusteredItems.value) + } finally { + unmockkConstructor(ShapeDrawable::class) + unmockkStatic(LayoutInflater::class) + } + } + + @Test + public fun testApplyClusterMarkerPropertiesUsesDefaultsWhenPropertiesNull() { + val marker = mockk(relaxed = true) + every { marker.setAnchor(any(), any()) } just runs + every { marker.zIndex = any() } just runs + every { marker.rotation = any() } just runs + + applyClusterMarkerProperties( + marker = marker, + properties = null, + defaultAnchor = Offset(0.25f, 0.75f), + defaultZIndex = 5.0f, + defaultRotation = 45.0f, + ) + + verify { marker.setAnchor(0.25f, 0.75f) } + verify { marker.zIndex = 5.0f } + verify { marker.rotation = 45.0f } + } + + @Test + public fun testApplyClusterMarkerPropertiesPrefersModifierOverrides() { + val marker = mockk(relaxed = true) + every { marker.setAnchor(any(), any()) } just runs + every { marker.zIndex = any() } just runs + every { marker.rotation = any() } just runs + + val properties = ClusteringMarkerProperties().apply { + anchor = Offset(0.1f, 0.2f) + zIndex = 9.0f + rotation = 180.0f + } + + applyClusterMarkerProperties( + marker = marker, + properties = properties, + defaultAnchor = Offset(0.5f, 1.0f), + defaultZIndex = 0.0f, + defaultRotation = 0.0f, + ) + + verify { marker.setAnchor(0.1f, 0.2f) } + verify { marker.zIndex = 9.0f } + verify { marker.rotation = 180.0f } + } + + @Test + public fun testApplyClusterItemMarkerPropertiesUpdatesIconAndMarkerProperties() { + val marker = mockk(relaxed = true) + val icon = mockk() + every { marker.setIcon(any()) } just runs + every { marker.setAnchor(any(), any()) } just runs + every { marker.zIndex = any() } just runs + every { marker.rotation = any() } just runs + + val properties = ClusteringMarkerProperties() + + applyClusterItemMarkerProperties( + marker = marker, + icon = icon, + properties = properties, + defaultAnchor = Offset(0.1f, 0.9f), + defaultZIndex = 3.0f, + defaultRotation = 90.0f, + ) + + verify { marker.setIcon(icon) } + verify { marker.setAnchor(0.1f, 0.9f) } + verify { marker.zIndex = 3.0f } + verify { marker.rotation = 90.0f } + } +} From b70b969a4b43dd0b6212f8f958d871cd1a56a4cc Mon Sep 17 00:00:00 2001 From: Dale Hawkins <107309+dkhawk@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:25:40 -0600 Subject: [PATCH 2/2] fix(utils): address review feedback on clustering renderer and effects --- maps-compose-utils/build.gradle.kts | 3 + .../compose/clustering/ClusterRenderer.kt | 214 ++++++++++++------ .../android/compose/clustering/Clustering.kt | 114 +++++++--- .../compose/clustering/ClusterRendererTest.kt | 117 +++++++++- 4 files changed, 331 insertions(+), 117 deletions(-) diff --git a/maps-compose-utils/build.gradle.kts b/maps-compose-utils/build.gradle.kts index dde5aaab4..e8f6360f0 100644 --- a/maps-compose-utils/build.gradle.kts +++ b/maps-compose-utils/build.gradle.kts @@ -51,6 +51,9 @@ android { } testOptions { + // Required to instantiate DefaultClusterRenderer in unit tests: its inner ViewModifier + // subclasses android.os.Handler(Looper.getMainLooper()), and JVM bytecode rules prevent + // MockK from skipping the superclass Handler. call in android.jar. unitTests.isReturnDefaultValues = true } } diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt index b4322df6c..67afbd757 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/ClusterRenderer.kt @@ -69,52 +69,110 @@ internal interface ClusterRendererItemState { * [clusterContentState] renders clusters, and [clusterItemContentState] renders non-clustered * items. */ -internal class ComposeUiClusterRenderer( - private val context: Context, - private val scope: CoroutineScope, - map: GoogleMap, - clusterManager: ClusterManager, - private val viewRendererState: State, - private val clusterContentState: State<@Composable ((Cluster) -> Unit)?>, - private val clusterItemContentState: State<@Composable ((T) -> Unit)?>, - private val clusterContentAnchorState: State, - private val clusterItemContentAnchorState: State, - private val clusterContentZIndexState: State, - private val clusterItemContentZIndexState: State, - private val clusterContentRotationState: State, - private val clusterItemContentRotationState: State, - executor: Executor = Executors.newSingleThreadExecutor(), -) : DefaultClusterRenderer( - context, - map, - clusterManager, - executor, -), +internal class ComposeUiClusterRenderer : + DefaultClusterRenderer, ClusterRendererItemState { + private val context: Context + private val scope: CoroutineScope + private val viewRendererState: State + private val clusterContentState: State<@Composable ((Cluster) -> Unit)?> + private val clusterItemContentState: State<@Composable ((T) -> Unit)?> + private val clusterContentAnchorState: State + private val clusterItemContentAnchorState: State + private val clusterContentZIndexState: State + private val clusterItemContentZIndexState: State + private val clusterContentRotationState: State + private val clusterItemContentRotationState: State + + constructor( + context: Context, + scope: CoroutineScope, + map: GoogleMap, + clusterManager: ClusterManager, + viewRendererState: State, + clusterContentState: State<@Composable ((Cluster) -> Unit)?>, + clusterItemContentState: State<@Composable ((T) -> Unit)?>, + clusterContentAnchorState: State, + clusterItemContentAnchorState: State, + clusterContentZIndexState: State, + clusterItemContentZIndexState: State, + clusterContentRotationState: State, + clusterItemContentRotationState: State, + ) : super(context, map, clusterManager) { + this.context = context + this.scope = scope + this.viewRendererState = viewRendererState + this.clusterContentState = clusterContentState + this.clusterItemContentState = clusterItemContentState + this.clusterContentAnchorState = clusterContentAnchorState + this.clusterItemContentAnchorState = clusterItemContentAnchorState + this.clusterContentZIndexState = clusterContentZIndexState + this.clusterItemContentZIndexState = clusterItemContentZIndexState + this.clusterContentRotationState = clusterContentRotationState + this.clusterItemContentRotationState = clusterItemContentRotationState + startObservingProperties() + } + + constructor( + context: Context, + scope: CoroutineScope, + map: GoogleMap, + clusterManager: ClusterManager, + viewRendererState: State, + clusterContentState: State<@Composable ((Cluster) -> Unit)?>, + clusterItemContentState: State<@Composable ((T) -> Unit)?>, + clusterContentAnchorState: State, + clusterItemContentAnchorState: State, + clusterContentZIndexState: State, + clusterItemContentZIndexState: State, + clusterContentRotationState: State, + clusterItemContentRotationState: State, + executor: Executor, + ) : super(context, map, clusterManager, executor) { + this.context = context + this.scope = scope + this.viewRendererState = viewRendererState + this.clusterContentState = clusterContentState + this.clusterItemContentState = clusterItemContentState + this.clusterContentAnchorState = clusterContentAnchorState + this.clusterItemContentAnchorState = clusterItemContentAnchorState + this.clusterContentZIndexState = clusterContentZIndexState + this.clusterItemContentZIndexState = clusterItemContentZIndexState + this.clusterContentRotationState = clusterContentRotationState + this.clusterItemContentRotationState = clusterItemContentRotationState + startObservingProperties() + } + override val unclusteredItems = mutableStateOf(emptySet()) - private val fakeCanvas = Canvas() - private val keysToViews = mutableMapOf, ViewInfo>() + private val fakeCanvas by lazy { Canvas() } + internal val keysToViews = mutableMapOf, ViewInfo>() - private val fakeLifecycleOwner = object : LifecycleOwner { - val lifecycleRegistry = LifecycleRegistry(this) - override val lifecycle: Lifecycle get() = lifecycleRegistry - } + internal var viewFactory: (ViewKey) -> InvalidatingComposeView = ::createInvalidatingComposeView - private val fakeSavedStateRegistryOwner = object : SavedStateRegistryOwner { - private val controller = SavedStateRegistryController.create(this).apply { - performAttach() - performRestore(null) + private val fakeLifecycleOwner by lazy { + object : LifecycleOwner { + val lifecycleRegistry = LifecycleRegistry(this) + override val lifecycle: Lifecycle get() = lifecycleRegistry } - override val savedStateRegistry: SavedStateRegistry get() = controller.savedStateRegistry - override val lifecycle: Lifecycle get() = fakeLifecycleOwner.lifecycle - init { - fakeLifecycleOwner.lifecycleRegistry.currentState = Lifecycle.State.RESUMED + } + + private val fakeSavedStateRegistryOwner by lazy { + object : SavedStateRegistryOwner { + private val controller = SavedStateRegistryController.create(this).apply { + performAttach() + performRestore(null) + } + override val savedStateRegistry: SavedStateRegistry get() = controller.savedStateRegistry + override val lifecycle: Lifecycle get() = fakeLifecycleOwner.lifecycle + init { + fakeLifecycleOwner.lifecycleRegistry.currentState = Lifecycle.State.RESUMED + } } } - init { + private fun startObservingProperties() { // Observe top-level Clustering property state updates across rotation, anchor, and zIndex. // When these states update from a parent recomposition, actively push the updated values // to any existing Marker objects currently on the map. @@ -203,39 +261,42 @@ internal class ComposeUiClusterRenderer( } } - private fun createAndAddView(key: ViewKey): ViewInfo { - val view = InvalidatingComposeView( - context, - getRotationOverride = { - when (key) { - is ViewKey.Cluster -> clusterContentRotationState.value - is ViewKey.Item -> clusterItemContentRotationState.value - } - }, - getAnchor = { - when (key) { - is ViewKey.Cluster -> clusterContentAnchorState.value - is ViewKey.Item -> clusterItemContentAnchorState.value - } - }, - getZIndex = { - when (key) { - is ViewKey.Cluster -> clusterContentZIndexState.value - is ViewKey.Item -> clusterItemContentZIndexState.value - } - }, - content = when (key) { - is ViewKey.Cluster -> { - { clusterContentState.value?.invoke(key.cluster) } - } + private fun createInvalidatingComposeView(key: ViewKey): InvalidatingComposeView = InvalidatingComposeView( + context, + getRotationOverride = { + when (key) { + is ViewKey.Cluster -> clusterContentRotationState.value + is ViewKey.Item -> clusterItemContentRotationState.value + } + }, + getAnchor = { + when (key) { + is ViewKey.Cluster -> clusterContentAnchorState.value + is ViewKey.Item -> clusterItemContentAnchorState.value + } + }, + getZIndex = { + when (key) { + is ViewKey.Cluster -> clusterContentZIndexState.value + is ViewKey.Item -> clusterItemContentZIndexState.value + } + }, + content = when (key) { + is ViewKey.Cluster -> { + { clusterContentState.value?.invoke(key.cluster) } + } - is ViewKey.Item -> { - { clusterItemContentState.value?.invoke(key.item) } - } - }, - ) + is ViewKey.Item -> { + { clusterItemContentState.value?.invoke(key.item) } + } + }, + ).also { view -> view.setViewTreeLifecycleOwner(fakeLifecycleOwner) view.setViewTreeSavedStateRegistryOwner(fakeSavedStateRegistryOwner) + } + + private fun createAndAddView(key: ViewKey): ViewInfo { + val view = viewFactory(key) val renderHandle = viewRendererState.value.startRenderingView(view) val rerenderJob = scope.launch { collectInvalidationsAndRerender(key, view) @@ -359,7 +420,7 @@ internal class ComposeUiClusterRenderer( } } - override fun onClusterUpdated(cluster: Cluster, marker: Marker) { + public override fun onClusterUpdated(cluster: Cluster, marker: Marker) { super.onClusterUpdated(cluster, marker) if (!scope.isActive) return if (clusterContentState.value != null) { @@ -374,14 +435,15 @@ internal class ComposeUiClusterRenderer( } } - override fun onClusterItemUpdated(item: T, marker: Marker) { + public override fun onClusterItemUpdated(item: T, marker: Marker) { super.onClusterItemUpdated(item, marker) if (!scope.isActive) return if (clusterItemContentState.value != null) { - val viewInfo = keysToViews[ViewKey.Item(item)] ?: createAndAddView(ViewKey.Item(item)) + val existingViewInfo = keysToViews[ViewKey.Item(item)] + val viewInfo = existingViewInfo ?: createAndAddView(ViewKey.Item(item)) applyClusterItemMarkerProperties( marker = marker, - icon = renderViewToBitmapDescriptor(viewInfo.view), + icon = if (existingViewInfo == null) renderViewToBitmapDescriptor(viewInfo.view) else null, properties = viewInfo.view.properties, defaultAnchor = clusterItemContentAnchorState.value, defaultZIndex = clusterItemContentZIndexState.value, @@ -413,19 +475,19 @@ internal class ComposeUiClusterRenderer( return BitmapDescriptorFactory.fromBitmap(bitmap) } - private sealed class ViewKey { + internal sealed class ViewKey { data class Cluster(val cluster: com.google.maps.android.clustering.Cluster) : ViewKey() data class Item(val item: T) : ViewKey() } - private class ViewInfo(val view: InvalidatingComposeView, val onRemove: () -> Unit) + internal class ViewInfo(val view: InvalidatingComposeView, val onRemove: () -> Unit) /** * An [AbstractComposeView] that calls [onInvalidate] whenever the Compose render layer is * invalidated. Works by reporting invalidations from its inner AndroidComposeView. */ - private class InvalidatingComposeView( + internal class InvalidatingComposeView( context: Context, private val getRotationOverride: () -> Float, private val getAnchor: () -> Offset, @@ -493,13 +555,15 @@ internal fun applyClusterMarkerProperties( internal fun applyClusterItemMarkerProperties( marker: Marker, - icon: BitmapDescriptor, + icon: BitmapDescriptor? = null, properties: ClusteringMarkerProperties, defaultAnchor: Offset, defaultZIndex: Float, defaultRotation: Float, ) { - marker.setIcon(icon) + if (icon != null) { + marker.setIcon(icon) + } applyClusterMarkerProperties( marker = marker, properties = properties, diff --git a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt index ef45b4d8b..c54a15424 100644 --- a/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt +++ b/maps-compose-utils/src/main/java/com/google/maps/android/compose/clustering/Clustering.kt @@ -577,7 +577,7 @@ internal fun Clustering( onMarkerDragStart = clusterManager.markerManager::onMarkerDragStart, ) val cameraPositionState = currentCameraPositionState - LaunchedEffect(cameraPositionState) { + LaunchedEffect(cameraPositionState, clusterManager) { snapshotFlow { cameraPositionState.isMoving } .collect { isMoving -> if (!isMoving) { @@ -643,8 +643,9 @@ internal fun Clustering( * * @param clusterManager the [ClusterManager] that manages the clusters * @param executor an optional [Executor] used by [DefaultClusterRenderer] to process view - * modifications off the main thread. Tests can pass a direct executor to make rendering - * deterministic. + * modifications off the main thread. Must be remembered across recompositions (e.g. with [remember]) + * to avoid recreating the renderer on every recomposition. Tests can pass a direct executor to make + * rendering deterministic. */ @Composable @GoogleMapComposable @@ -657,13 +658,21 @@ public fun rememberClusterRenderer( val clusterRendererState: MutableState?> = remember { mutableStateOf(null) } clusterManager ?: return null - MapEffect(context, executor) { map -> - val renderer = ReportingDefaultClusterRenderer( - context = context, - map = map, - clusterManager = clusterManager, - executor = executor ?: Executors.newSingleThreadExecutor(), - ) + MapEffect(context, clusterManager, executor) { map -> + val renderer = if (executor != null) { + ReportingDefaultClusterRenderer( + context = context, + map = map, + clusterManager = clusterManager, + executor = executor, + ) + } else { + ReportingDefaultClusterRenderer( + context = context, + map = map, + clusterManager = clusterManager, + ) + } clusterRendererState.value = renderer } @@ -701,8 +710,9 @@ public fun rememberClusterRenderer(clusterManager: ClusterMana * @param clusterItemContentRotation the rotation of the non-clustered item in degrees clockwise about the marker's anchor point * @param clusterManager the [ClusterManager] that manages the clusters * @param executor an optional [Executor] used by [DefaultClusterRenderer] to process view - * modifications off the main thread. Tests can pass a direct executor to make rendering - * deterministic. + * modifications off the main thread. Must be remembered across recompositions (e.g. with [remember]) + * to avoid recreating the renderer on every recomposition. Tests can pass a direct executor to make + * rendering deterministic. */ @Composable @GoogleMapComposable @@ -732,23 +742,41 @@ public fun rememberClusterRenderer( val clusterRendererState: MutableState?> = remember { mutableStateOf(null) } clusterManager ?: return null - MapEffect(context, executor) { map -> - val renderer = ComposeUiClusterRenderer( - context, - scope = this, - map, - clusterManager, - viewRendererState, - clusterContentState, - clusterItemContentState, - clusterContentAnchorState, - clusterItemContentAnchorState, - clusterContentZIndexState, - clusterItemContentZIndexState, - clusterContentRotationState, - clusterItemContentRotationState, - executor = executor ?: Executors.newSingleThreadExecutor(), - ) + MapEffect(context, clusterManager, executor) { map -> + val renderer = if (executor != null) { + ComposeUiClusterRenderer( + context = context, + scope = this, + map = map, + clusterManager = clusterManager, + viewRendererState = viewRendererState, + clusterContentState = clusterContentState, + clusterItemContentState = clusterItemContentState, + clusterContentAnchorState = clusterContentAnchorState, + clusterItemContentAnchorState = clusterItemContentAnchorState, + clusterContentZIndexState = clusterContentZIndexState, + clusterItemContentZIndexState = clusterItemContentZIndexState, + clusterContentRotationState = clusterContentRotationState, + clusterItemContentRotationState = clusterItemContentRotationState, + executor = executor, + ) + } else { + ComposeUiClusterRenderer( + context = context, + scope = this, + map = map, + clusterManager = clusterManager, + viewRendererState = viewRendererState, + clusterContentState = clusterContentState, + clusterItemContentState = clusterItemContentState, + clusterContentAnchorState = clusterContentAnchorState, + clusterItemContentAnchorState = clusterItemContentAnchorState, + clusterContentZIndexState = clusterContentZIndexState, + clusterItemContentZIndexState = clusterItemContentZIndexState, + clusterContentRotationState = clusterContentRotationState, + clusterItemContentRotationState = clusterItemContentRotationState, + ) + } clusterRendererState.value = renderer awaitCancellation() } @@ -794,9 +822,12 @@ public fun rememberClusterRenderer( * Remembers a [ClusterManager] configured with the given coroutine contexts. * * @param mainContext the coroutine context used to deliver clusters to the renderer. It must - * dispatch on the main thread. + * dispatch on the main thread. Custom contexts (e.g. `Dispatchers.Main + job`) must be remembered + * across recompositions (e.g. with [remember]) to avoid recreating the manager on every recomposition. * @param backgroundContext the coroutine context used to compute clusters off the main thread. - * Tests can pass a test dispatcher for both contexts to make clustering deterministic. + * Custom contexts (e.g. `Dispatchers.Default + job`) must be remembered across recompositions (e.g. + * with [remember]) to avoid recreating the manager on every recomposition. Tests can pass a test + * dispatcher for both contexts to make clustering deterministic. */ @Composable @GoogleMapComposable @@ -910,16 +941,25 @@ private fun ResetMapListeners(clusterManager: ClusterManager<*>) { } } -internal class ReportingDefaultClusterRenderer( - context: Context, - map: GoogleMap, - clusterManager: ClusterManager, - executor: Executor = Executors.newSingleThreadExecutor(), -) : DefaultClusterRenderer(context, map, clusterManager, executor), +internal class ReportingDefaultClusterRenderer : + DefaultClusterRenderer, ClusterRendererItemState { override val unclusteredItems = mutableStateOf(emptySet()) + constructor( + context: Context, + map: GoogleMap, + clusterManager: ClusterManager, + ) : super(context, map, clusterManager) + + constructor( + context: Context, + map: GoogleMap, + clusterManager: ClusterManager, + executor: Executor, + ) : super(context, map, clusterManager, executor) + override fun onClustersChanged(clusters: Set>) { super.onClustersChanged(clusters) unclusteredItems.value = clusters.filter { !shouldRenderAsCluster(it) } diff --git a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt index aced8abbe..aa740d1d9 100644 --- a/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt +++ b/maps-compose-utils/src/test/java/com/google/maps/android/compose/clustering/ClusterRendererTest.kt @@ -22,6 +22,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.TextView +import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.geometry.Offset import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.BitmapDescriptor @@ -29,6 +30,7 @@ import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.Marker import com.google.maps.android.clustering.ClusterItem import com.google.maps.android.clustering.ClusterManager +import com.google.maps.android.compose.ComposeUiViewRenderer import com.google.maps.android.ui.RotationLayout import io.mockk.every import io.mockk.just @@ -39,7 +41,11 @@ import io.mockk.runs import io.mockk.unmockkConstructor import io.mockk.unmockkStatic import io.mockk.verify +import io.mockk.verifyOrder +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.test.UnconfinedTestDispatcher import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull @@ -56,8 +62,9 @@ public class ClusterRendererTest { override val zIndex: Float? = 0f, ) : ClusterItem - @Test - public fun testClusterManagerAndReportingRendererSupportCustomContextsAndExecutor() { + private inline fun withMockedRendererDependencies( + block: (Context, GoogleMap, ClusterManager) -> R, + ): R { mockkStatic(LayoutInflater::class) mockkConstructor(ShapeDrawable::class) try { @@ -80,6 +87,16 @@ public class ClusterRendererTest { mainDispatcher, backgroundDispatcher, ) + return block(context, map, clusterManager) + } finally { + unmockkConstructor(ShapeDrawable::class) + unmockkStatic(LayoutInflater::class) + } + } + + @Test + public fun testClusterManagerAndReportingRendererSupportCustomContextsAndExecutor() { + withMockedRendererDependencies { context, map, clusterManager -> assertNotNull(clusterManager) val customExecutor = Executor { runnable -> runnable.run() } @@ -91,9 +108,71 @@ public class ClusterRendererTest { ) assertNotNull(renderer) assertEquals(emptySet(), renderer.unclusteredItems.value) - } finally { - unmockkConstructor(ShapeDrawable::class) - unmockkStatic(LayoutInflater::class) + } + } + + @Test + public fun testOnClusterItemUpdatedPreservesCustomZIndexAndAvoidsRedrawingCachedView() { + withMockedRendererDependencies { context, map, clusterManager -> + val scope = CoroutineScope(SupervisorJob() + UnconfinedTestDispatcher()) + try { + val viewRenderer = mockk(relaxed = true) + val renderer = ComposeUiClusterRenderer( + context = context, + scope = scope, + map = map, + clusterManager = clusterManager, + viewRendererState = mutableStateOf(viewRenderer), + clusterContentState = mutableStateOf(null), + clusterItemContentState = mutableStateOf({}), + clusterContentAnchorState = mutableStateOf(Offset(0.5f, 1.0f)), + clusterItemContentAnchorState = mutableStateOf(Offset(0.25f, 0.75f)), + clusterContentZIndexState = mutableStateOf(0.0f), + clusterItemContentZIndexState = mutableStateOf(9.0f), + clusterContentRotationState = mutableStateOf(0.0f), + clusterItemContentRotationState = mutableStateOf(45.0f), + executor = Executor { runnable -> runnable.run() }, + ) + + val item = TestItem( + position = LatLng(37.4220, -122.0841), + title = "Updated Title", + snippet = "Updated Snippet", + zIndex = 0f, + ) + val cachedView = mockk(relaxed = true) + val customProperties = ClusteringMarkerProperties().apply { + zIndex = 12.0f + } + every { cachedView.properties } returns customProperties + renderer.keysToViews[ComposeUiClusterRenderer.ViewKey.Item(item)] = + ComposeUiClusterRenderer.ViewInfo(cachedView, onRemove = {}) + + val marker = mockk(relaxed = true) + every { marker.position } returns item.position + every { marker.title } returns "Old Title" + every { marker.snippet } returns "Old Snippet" + every { marker.zIndex } returns 12.0f + every { marker.isInfoWindowShown } returns false + every { marker.setAnchor(any(), any()) } just runs + every { marker.zIndex = any() } just runs + every { marker.rotation = any() } just runs + + renderer.onClusterItemUpdated(item, marker) + + verifyOrder { + // DefaultClusterRenderer.onClusterItemUpdated overwrites zIndex with item.zIndex (0f) + marker.zIndex = 0f + // ComposeUiClusterRenderer.onClusterItemUpdated restores the custom Compose zIndex (12.0f) + marker.zIndex = 12.0f + } + verify { marker.setAnchor(0.25f, 0.75f) } + verify { marker.rotation = 45.0f } + verify(exactly = 0) { marker.setIcon(any()) } + verify(exactly = 0) { cachedView.draw(any()) } + } finally { + scope.cancel() + } } } @@ -168,4 +247,32 @@ public class ClusterRendererTest { verify { marker.zIndex = 3.0f } verify { marker.rotation = 90.0f } } + + @Test + public fun testApplyClusterItemMarkerPropertiesSkipsIconUpdateWhenIconNull() { + val marker = mockk(relaxed = true) + every { marker.setAnchor(any(), any()) } just runs + every { marker.zIndex = any() } just runs + every { marker.rotation = any() } just runs + + val properties = ClusteringMarkerProperties().apply { + zIndex = 7.0f + rotation = 30.0f + anchor = Offset(0.2f, 0.8f) + } + + applyClusterItemMarkerProperties( + marker = marker, + icon = null, + properties = properties, + defaultAnchor = Offset(0.5f, 1.0f), + defaultZIndex = 0.0f, + defaultRotation = 0.0f, + ) + + verify(exactly = 0) { marker.setIcon(any()) } + verify { marker.setAnchor(0.2f, 0.8f) } + verify { marker.zIndex = 7.0f } + verify { marker.rotation = 30.0f } + } }