fix: refresh retained marker visuals across providers (#54)

* fix: refresh retained marker visuals across providers

* fix(ios): keep one pending Google marker image load

* test(ios): cover Google marker pending image apply

* test(ios): assert pending Google marker load applies the icon

* fix(ios): reject stale marker image completions

Author: jkasprzyk17

README.md

@@ -347,7 +347,7 @@ Additional props:
 | `anchor`       | `{ x: 0.5, y: 1 }` | Point on the image aligned to the coordinate        |
 | `centerOffset` | —                  | Extra offset in dp (MapKit-style)                   |
 | `rotation`     | `0`                | Clockwise rotation in degrees                       |
-| `flat`         | `false`            | Rotate with map plane (Google Maps; limited on iOS) |
+| `flat`         | `false`            | Rotate with map plane (Google Maps; MapKit approximates via view transform) |
 | `opacity`      | `1`                | Marker opacity from 0 to 1                          |
 
 Platform notes:

docs/adr/0004-custom-view-markers.md

@@ -109,9 +109,8 @@ natively — unlike the null-rendering `<Marker>`.
 
 ### Phase 0 — quick win (independent)
 
-- Fix the existing gap where iOS Google (`GoogleMapOverlayController.updateMarker`) sets
-  `icon = nil` and never applies `MarkerDescriptor.image`, so `<Marker image>` is
-  consistent across all three backends (MapKit, iOS Google, Android Google).
+Done: iOS Google applies `MarkerDescriptor` image, anchor, centerOffset, rotation, flat,
+and opacity so `<Marker image>` is consistent across MapKit, iOS Google, and Android Google.
 
 ### Phase 1 — MVP custom views (Option A)
 

docs/roadmap.md

@@ -91,7 +91,7 @@ Delivered incrementally during Phases 3–5; polished for platform consistency i
 
 ## Custom view markers (ADR 0004)
 
-- [ ] Phase 0: apply `MarkerDescriptor.image` on iOS Google provider
+- [x] Phase 0: apply `MarkerDescriptor` image, anchor, centerOffset, rotation, flat, and opacity on iOS Google
 - [ ] Phase 1: `<Marker>` children via snapshot into existing image pipeline
 - [ ] Phase 2: `<MarkerView>` HybridView — live MapKit views, snapshot on Google
 

package/android/build.gradle

@@ -79,4 +79,5 @@ dependencies {
     implementation project(':react-native-nitro-modules')
     implementation 'com.google.android.gms:play-services-maps:19.0.0'
     implementation 'com.google.maps.android:android-maps-utils:3.8.2'
+    testImplementation 'junit:junit:4.13.2'
 }

package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt

@@ -36,7 +36,7 @@ class MapOverlayController(
   private var onMarkerPress: ((String) -> Unit)? = null
   private var onClusterPress: ((List<String>, Coordinate) -> Unit)? = null
   private var allMarkerDescriptors: Array<MarkerDescriptor> = emptyArray()
-  private var markersFingerprint: Int = 0
+  private var markersFingerprint: Long = 0L
   private var spatialIndex: MarkerSpatialIndex? = null
   private var refreshGeneration: Int = 0
   private var viewWidthPx: Int = 0
@@ -103,7 +103,7 @@ class MapOverlayController(
     polygons.clear()
     circles.clear()
     allMarkerDescriptors = emptyArray()
-    markersFingerprint = 0
+    markersFingerprint = 0L
     spatialIndex = null
     refreshGeneration += 1
     computeExecutor.shutdown()
@@ -142,7 +142,6 @@ class MapOverlayController(
 
   fun refreshViewportMarkers(
     animateEntering: Boolean = true,
-    updateRetained: Boolean = true,
     maxAnimatedMarkers: Int = MAX_ANIMATED_MARKERS_PER_DIFF,
   ) {
     val map = googleMap ?: return
@@ -172,30 +171,13 @@ class MapOverlayController(
           .map { ClusterElement.Single(it) }
       }
 
-      val nextKeys = HashSet<String>(elements.size)
-      val added = ArrayList<ClusterElement>()
-      val retained = ArrayList<ClusterElement>()
-      for (element in elements) {
-        val key = element.diffKey
-        if (!nextKeys.add(key)) {
-          continue
-        }
-        val version = element.renderVersion
-        if (displayedVersions[key] != null) {
-          if (updateRetained && displayedVersions[key] != version) {
-            retained.add(element)
-          }
-        } else {
-          added.add(element)
-        }
-      }
-      val removed = displayedVersions.keys - nextKeys
+      val diff = computeMarkerRenderDiff(elements, displayedVersions)
 
       mainHandler.post {
         if (generation != refreshGeneration) {
           return@post
         }
-        applyDiff(removed, added, retained, animateEntering, maxAnimatedMarkers)
+        applyDiff(diff, animateEntering, maxAnimatedMarkers)
       }
     }
   }
@@ -218,24 +200,22 @@ class MapOverlayController(
   }
 
   private fun applyDiff(
-    removedKeys: Set<String>,
-    added: List<ClusterElement>,
-    retained: List<ClusterElement>,
+    diff: MarkerRenderDiff,
     animateEntering: Boolean = true,
     maxAnimatedMarkers: Int = MAX_ANIMATED_MARKERS_PER_DIFF,
   ) {
     val map = googleMap ?: return
 
-    for (key in removedKeys) {
+    for (key in diff.removedKeys) {
       cancelEnteringAnimation(key)
       markers.remove(key)?.remove()
       markerVersions.remove(key)
       clusterByKey.remove(key)
     }
 
     var remainingAnimationBudget = maxAnimatedMarkers.coerceAtLeast(0)
-    val addedMarkers = ArrayList<AddedMarker>(minOf(added.size, remainingAnimationBudget))
-    for (element in added) {
+    val addedMarkers = ArrayList<AddedMarker>(minOf(diff.added.size, remainingAnimationBudget))
+    for (element in diff.added) {
       val key = element.diffKey
       when (element) {
         is ClusterElement.Single -> {
@@ -253,7 +233,8 @@ class MapOverlayController(
             markerIconFactory.applyVisualProps(element.descriptor, marker, key)
             markerVersions[key] = element.renderVersion
             if (shouldAnimate) {
-              addedMarkers.add(AddedMarker(key, marker, animation))
+              val targetAlpha = element.descriptor.opacity?.toFloat() ?: 1f
+              addedMarkers.add(AddedMarker(key, marker, animation, targetAlpha))
               remainingAnimationBudget -= 1
             }
           }
@@ -276,19 +257,18 @@ class MapOverlayController(
             markerVersions[key] = element.renderVersion
             clusterByKey[key] = element
             if (shouldAnimate) {
-              addedMarkers.add(AddedMarker(key, marker, animation))
+              addedMarkers.add(AddedMarker(key, marker, animation, targetAlpha = 1f))
               remainingAnimationBudget -= 1
             }
           }
         }
       }
     }
 
-    for (element in retained) {
+    for (element in diff.retained) {
       val key = element.diffKey
       val marker = markers[key] ?: continue
       cancelEnteringAnimation(key)
-      marker.alpha = 1f
       when (element) {
         is ClusterElement.Single -> {
           marker.tag = element.descriptor.id
@@ -303,6 +283,7 @@ class MapOverlayController(
           clusterByKey.remove(key)
         }
         is ClusterElement.Cluster -> {
+          marker.alpha = 1f
           marker.position = element.position
           marker.setIcon(iconFactory.icon(element.count))
           clusterByKey[key] = element
@@ -348,7 +329,7 @@ class MapOverlayController(
           val localElapsed = elapsed - (animatedMarker.animation.delayMs - startDelay)
           val progress = (localElapsed.toFloat() / animatedMarker.animation.durationMs.toFloat())
             .coerceIn(0f, 1f)
-          animatedMarker.marker.alpha = progress
+          animatedMarker.marker.alpha = progress * animatedMarker.targetAlpha
         }
       }
       addListener(object : AnimatorListenerAdapter() {
@@ -369,7 +350,7 @@ class MapOverlayController(
 
   private fun revealAnimatedMarkers(animated: List<AddedMarker>) {
     animated.forEach { animatedMarker ->
-      animatedMarker.marker.alpha = 1f
+      animatedMarker.marker.alpha = animatedMarker.targetAlpha
     }
   }
 
@@ -422,14 +403,14 @@ class MapOverlayController(
           markers[key] = marker
           markerIconFactory.applyVisualProps(descriptor, marker, key)
           markerVersions[key] = element.renderVersion
-          animateEntering(listOf(AddedMarker(key, marker, animation)))
+          val targetAlpha = descriptor.opacity?.toFloat() ?: 1f
+          animateEntering(listOf(AddedMarker(key, marker, animation, targetAlpha)))
         }
       },
       update = { marker, descriptor ->
         val element = ClusterElement.Single(descriptor)
         val key = "s:" + descriptor.id
         (marker.tag as? String)?.let { cancelEnteringAnimation(it) }
-        marker.alpha = 1f
         marker.tag = descriptor.id
         marker.position = LatLng(
           descriptor.coordinate.latitude,
@@ -495,7 +476,6 @@ class MapOverlayController(
     if (usesViewportPipeline()) {
       refreshViewportMarkers(
         animateEntering = true,
-        updateRetained = true,
         maxAnimatedMarkers = MAX_LIVE_ANIMATED_MARKERS_PER_DIFF,
       )
     }
@@ -640,5 +620,6 @@ class MapOverlayController(
     val key: String,
     val marker: Marker,
     val animation: ResolvedOverlayEnteringAnimation,
+    val targetAlpha: Float,
   )
 }

package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt

@@ -14,16 +14,7 @@ internal sealed interface ClusterElement {
 
   data class Single(val descriptor: MarkerDescriptor) : ClusterElement {
     override val diffKey: String get() = "s:" + descriptor.id
-    override val renderVersion: Long = renderSignature(
-      "single",
-      descriptor.id,
-      descriptor.coordinate.latitude,
-      descriptor.coordinate.longitude,
-      descriptor.title,
-      descriptor.subtitle,
-      descriptor.draggable,
-      descriptor.clusterable,
-    )
+    override val renderVersion: Long = descriptor.displayedIdentityVersion()
   }
 
   data class Cluster(
@@ -49,14 +40,6 @@ internal sealed interface ClusterElement {
   }
 }
 
-private fun renderSignature(vararg parts: Any?): Long {
-  var hash = -3750763034362895579L
-  for (part in parts) {
-    hash = 1099511628211L * hash + (part?.hashCode()?.toLong() ?: 0L)
-  }
-  return hash
-}
-
 /**
  * Grid-based marker clustering computed in geographic space.
  *

package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerDescriptor+DisplayedIdentity.kt

@@ -0,0 +1,24 @@
+package com.margelo.nitro.nitromaps
+
+/** Displayed-marker identity. Omits `enteringAnimation`; keep in sync with the Swift hasher. */
+internal fun MarkerDescriptor.displayedIdentityVersion(): Long =
+  renderSignature(
+    id,
+    coordinate.latitude,
+    coordinate.longitude,
+    title,
+    subtitle,
+    draggable,
+    clusterable,
+    image?.uri,
+    image?.width,
+    image?.height,
+    image?.scale,
+    anchor?.x,
+    anchor?.y,
+    centerOffset?.x,
+    centerOffset?.y,
+    rotation,
+    flat,
+    opacity,
+  )

package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerDescriptor+Fingerprint.kt

@@ -1,13 +1,22 @@
 package com.margelo.nitro.nitromaps
 
-internal fun Array<MarkerDescriptor>?.markersFingerprint(): Int {
+internal fun MarkerDescriptor.fingerprint(): Long =
+  renderSignature(
+    displayedIdentityVersion(),
+    enteringAnimation?.kind,
+    enteringAnimation?.duration,
+    enteringAnimation?.delay,
+    enteringAnimation?.reduceMotion,
+  )
+
+internal fun Array<MarkerDescriptor>?.markersFingerprint(): Long {
   if (this.isNullOrEmpty()) {
-    return 0
+    return 0L
   }
 
-  var hash = size
+  var hash = size.toLong()
   for (descriptor in this) {
-    hash = 31 * hash + descriptor.hashCode()
+    hash = 31L * hash + descriptor.fingerprint()
   }
   return hash
 }

package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerRenderDiff.kt

@@ -0,0 +1,35 @@
+package com.margelo.nitro.nitromaps
+
+internal data class MarkerRenderDiff(
+  val removedKeys: Set<String>,
+  val added: List<ClusterElement>,
+  val retained: List<ClusterElement>,
+)
+
+internal fun computeMarkerRenderDiff(
+  target: List<ClusterElement>,
+  displayed: Map<String, Long>,
+): MarkerRenderDiff {
+  val nextKeys = HashSet<String>(target.size)
+  val added = ArrayList<ClusterElement>()
+  val retained = ArrayList<ClusterElement>()
+
+  for (element in target) {
+    val key = element.diffKey
+    if (!nextKeys.add(key)) {
+      continue
+    }
+    val displayedVersion = displayed[key]
+    if (displayedVersion == null) {
+      added.add(element)
+    } else if (displayedVersion != element.renderVersion) {
+      retained.add(element)
+    }
+  }
+
+  return MarkerRenderDiff(
+    removedKeys = displayed.keys - nextKeys,
+    added = added,
+    retained = retained,
+  )
+}

package/android/src/main/java/com/margelo/nitro/nitromaps/RenderSignature.kt

@@ -0,0 +1,16 @@
+package com.margelo.nitro.nitromaps
+
+/**
+ * Absent values hash outside the `Int` range so no present value can collide with them.
+ * Without this, `null` and `0.0` are indistinguishable, because `0.0.hashCode() == 0`.
+ */
+private const val ABSENT_HASH = 0x1_0000_0000L
+
+/** Stable fold over the given parts, used to version render state for diffing. */
+internal fun renderSignature(vararg parts: Any?): Long {
+  var hash = -3750763034362895579L
+  for (part in parts) {
+    hash = 1099511628211L * hash + (part?.hashCode()?.toLong() ?: ABSENT_HASH)
+  }
+  return hash
+}