diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/CircleDescriptor+CircleOptions.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/CircleDescriptor+CircleOptions.kt index 161033f..c1d32e8 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/CircleDescriptor+CircleOptions.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/CircleDescriptor+CircleOptions.kt @@ -1,5 +1,7 @@ package com.margelo.nitro.nitromaps +import android.graphics.Color +import com.google.android.gms.maps.model.Circle import com.google.android.gms.maps.model.CircleOptions import com.google.android.gms.maps.model.LatLng @@ -15,3 +17,13 @@ fun CircleDescriptor.toCircleOptions(): CircleOptions { return options } + +/** Updates an existing circle in place, with the same defaults as [toCircleOptions]. */ +fun CircleDescriptor.applyTo(circle: Circle) { + circle.center = LatLng(center.latitude, center.longitude) + circle.radius = radius + circle.strokeColor = strokeColor?.toColorInt() ?: Color.BLACK + circle.fillColor = fillColor?.toColorInt() ?: Color.TRANSPARENT + circle.strokeWidth = (strokeWidth ?: 2.0).toFloat() + circle.isClickable = tappable != false +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index 15067f2..04d1c75 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -15,6 +15,7 @@ import com.google.android.gms.maps.CameraUpdateFactory import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.GoogleMapOptions import com.google.android.gms.maps.MapView +import com.google.android.gms.maps.model.CameraPosition import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.LatLngBounds import com.google.android.gms.maps.model.MapStyleOptions @@ -39,6 +40,8 @@ class GoogleMapProviderAdapter( private var pendingPolygons: Array? = null private var pendingCircles: Array? = null private val mainHandler = Handler(Looper.getMainLooper()) + private var lastAppliedRegion: Region? = null + private var lastAppliedRegionCamera: CameraPosition? = null private val googleMapIdAtCreation: String? = normalizeGoogleMapId(initialGoogleMapId) @@ -573,19 +576,36 @@ class GoogleMapProviderAdapter( private fun applyRegion(region: Region, animated: Boolean = false) { val map = googleMap ?: return - val bounds = region.toLatLngBounds() - val paddingPx = _mapPadding.toPaddingPixels() + runWhenMapViewLaidOut { fitCamera(map, region, animated) } + } - val runUpdate = { - val update = CameraUpdateFactory.newLatLngBounds(bounds, paddingPx) - if (animated) { - map.animateCamera(update) - } else { - map.moveCamera(update) - } + private fun fitCamera(map: GoogleMap, region: Region, animated: Boolean) { + val lastRegion = lastAppliedRegion + val lastCamera = lastAppliedRegionCamera + if ( + lastRegion != null && + lastCamera != null && + region.approximatelyEquals(lastRegion) && + map.cameraPosition.approximatelyEquals(lastCamera) + ) { + // Same region as last time and the camera has not moved since, so the + // fit would land on the camera the map already shows. + return } - runWhenMapViewLaidOut(runUpdate) + val update = CameraUpdateFactory.newLatLngBounds( + region.toLatLngBounds(), + _mapPadding.toPaddingPixels(), + ) + if (animated) { + map.animateCamera(update) + // The camera settles later; there is nothing reliable to remember yet. + lastAppliedRegionCamera = null + } else { + map.moveCamera(update) + lastAppliedRegionCamera = map.cameraPosition + } + lastAppliedRegion = region } private fun updateMapCamera( @@ -749,6 +769,8 @@ class GoogleMapProviderAdapter( _mapType = MapType.STANDARD _region = null _camera = null + lastAppliedRegion = null + lastAppliedRegionCamera = null scrollEnabled = true zoomEnabled = true rotateEnabled = true diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapApproximateEquality.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapApproximateEquality.kt index bd70dc7..adbca3c 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapApproximateEquality.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapApproximateEquality.kt @@ -2,6 +2,7 @@ package com.margelo.nitro.nitromaps object MapApproximateEquality { const val COORDINATE_EPSILON: Double = 1e-6 + const val SPAN_EPSILON: Double = 1e-6 const val ZOOM_EPSILON: Float = 1e-4f const val ANGLE_EPSILON: Float = 1e-3f } diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt index ed08196..08cfb67 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt @@ -11,6 +11,7 @@ import com.google.android.gms.maps.CameraUpdateFactory import com.google.android.gms.maps.GoogleMap import com.google.android.gms.maps.model.Circle import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.LatLngBounds import com.google.android.gms.maps.model.Marker import com.google.android.gms.maps.model.MarkerOptions import com.google.android.gms.maps.model.Polygon @@ -31,6 +32,9 @@ class MapOverlayController( private val polylines = LinkedHashMap() private val polygons = LinkedHashMap() private val circles = LinkedHashMap() + private val polylineVersions = HashMap() + private val polygonVersions = HashMap() + private val circleVersions = HashMap() private val markerEnterAnimators = HashMap() private var clusteringEnabled = false private var onMarkerPress: ((String) -> Unit)? = null @@ -38,7 +42,16 @@ class MapOverlayController( private var allMarkerDescriptors: Array = emptyArray() private var markersFingerprint: Long = 0L private var spatialIndex: MarkerSpatialIndex? = null + /** Invalidates in-flight refresh results (viewport diffs). */ private var refreshGeneration: Int = 0 + + /** + * Invalidates in-flight index builds. Kept apart from [refreshGeneration] so + * a burst of refreshes during a gesture cannot keep discarding the index + * build for a dataset that has not changed. + */ + private var datasetGeneration: Int = 0 + private val refreshInbox = RefreshInbox() private var viewWidthPx: Int = 0 private var viewHeightPx: Int = 0 private var idleRefreshRunnable: Runnable? = null @@ -102,10 +115,15 @@ class MapOverlayController( polylines.clear() polygons.clear() circles.clear() + polylineVersions.clear() + polygonVersions.clear() + circleVersions.clear() allMarkerDescriptors = emptyArray() markersFingerprint = 0L spatialIndex = null refreshGeneration += 1 + advanceDatasetGeneration() + refreshInbox.discardPending() computeExecutor.shutdown() computeExecutor = Executors.newSingleThreadExecutor() } @@ -120,6 +138,7 @@ class MapOverlayController( markersFingerprint = fingerprint allMarkerDescriptors = next spatialIndex = null + advanceDatasetGeneration() reapplyMarkers() } @@ -154,43 +173,52 @@ class MapOverlayController( } val bounds = map.projection.visibleRegion.latLngBounds - val latitudeSpan = bounds.northeast.latitude - bounds.southwest.latitude - val clustering = clusteringEnabled - val widthPx = viewWidthPx - val heightPx = viewHeightPx - val displayedVersions = HashMap(markerVersions) refreshGeneration += 1 - val generation = refreshGeneration + val request = ViewportRefreshRequest( + generation = refreshGeneration, + index = index, + bounds = bounds, + latitudeSpan = bounds.northeast.latitude - bounds.southwest.latitude, + clustering = clusteringEnabled, + widthPx = viewWidthPx, + heightPx = viewHeightPx, + displayedVersions = HashMap(markerVersions), + animateEntering = animateEntering, + maxAnimatedMarkers = maxAnimatedMarkers, + ) + if (!refreshInbox.post(request)) { + // A compute task is already queued and will pick this request up. + return + } computeExecutor.execute { - val candidates = index.candidates(bounds) - val elements: List = if (clustering) { - MarkerClusterEngine.clusters(candidates, bounds, widthPx, heightPx, density) - } else { - MarkerViewportFilter.displaySubset(candidates, bounds, latitudeSpan) - .map { ClusterElement.Single(it) } - } - - val diff = computeMarkerRenderDiff(elements, displayedVersions) + val pending = refreshInbox.take() ?: return@execute + val diff = computeViewportDiff(pending) mainHandler.post { - if (generation != refreshGeneration) { + if (pending.generation != refreshGeneration) { return@post } - applyDiff(diff, animateEntering, maxAnimatedMarkers) + applyDiff(diff, pending.animateEntering, pending.maxAnimatedMarkers) } } } private fun rebuildIndexAndRefresh() { val descriptors = allMarkerDescriptors + val builtForDataset = datasetGeneration + // Diffs computed against the previous index are stale from here on. refreshGeneration += 1 - val generation = refreshGeneration computeExecutor.execute { + if (!refreshInbox.isCurrent(builtForDataset)) { + // A newer dataset superseded this build before it started. + return@execute + } + val index = MarkerSpatialIndex(descriptors) mainHandler.post { - if (generation != refreshGeneration) { + if (builtForDataset != datasetGeneration) { return@post } spatialIndex = index @@ -199,6 +227,29 @@ class MapOverlayController( } } + private fun computeViewportDiff(request: ViewportRefreshRequest): MarkerRenderDiff { + val candidates = request.index.candidates(request.bounds) + val elements: List = if (request.clustering) { + MarkerClusterEngine.clusters( + candidates, + request.bounds, + request.widthPx, + request.heightPx, + density, + ) + } else { + MarkerViewportFilter.displaySubset(candidates, request.bounds, request.latitudeSpan) + .map { ClusterElement.Single(it) } + } + + return computeMarkerRenderDiff(elements, request.displayedVersions) + } + + private fun advanceDatasetGeneration() { + datasetGeneration += 1 + refreshInbox.recordDataset(datasetGeneration) + } + private fun applyDiff( diff: MarkerRenderDiff, animateEntering: Boolean = true, @@ -515,64 +566,89 @@ class MapOverlayController( fun updatePolylines(descriptors: Array?) { val map = googleMap ?: return - reconcile( + reconcileShapes( current = polylines, + versions = polylineVersions, next = descriptors?.associateBy { it.id } ?: emptyMap(), + version = { it.renderVersion() }, remove = { it.remove() }, add = { descriptor -> map.addPolyline(descriptor.toPolylineOptions()).also { polyline -> polyline.tag = descriptor.id } }, - update = { polyline, descriptor -> - polyline.remove() - map.addPolyline(descriptor.toPolylineOptions()).also { replacement -> - replacement.tag = descriptor.id - } - }, + update = { polyline, descriptor -> descriptor.applyTo(polyline) }, ) } fun updatePolygons(descriptors: Array?) { val map = googleMap ?: return - reconcile( + reconcileShapes( current = polygons, + versions = polygonVersions, next = descriptors?.associateBy { it.id } ?: emptyMap(), + version = { it.renderVersion() }, remove = { it.remove() }, add = { descriptor -> map.addPolygon(descriptor.toPolygonOptions()).also { polygon -> polygon.tag = descriptor.id } }, - update = { polygon, descriptor -> - polygon.remove() - map.addPolygon(descriptor.toPolygonOptions()).also { replacement -> - replacement.tag = descriptor.id - } - }, + update = { polygon, descriptor -> descriptor.applyTo(polygon) }, ) } fun updateCircles(descriptors: Array?) { val map = googleMap ?: return - reconcile( + reconcileShapes( current = circles, + versions = circleVersions, next = descriptors?.associateBy { it.id } ?: emptyMap(), + version = { it.renderVersion() }, remove = { it.remove() }, add = { descriptor -> map.addCircle(descriptor.toCircleOptions()).also { circle -> circle.tag = descriptor.id } }, - update = { circle, descriptor -> - circle.remove() - map.addCircle(descriptor.toCircleOptions()).also { replacement -> - replacement.tag = descriptor.id - } - }, + update = { circle, descriptor -> descriptor.applyTo(circle) }, ) } + /** + * Like [reconcile], but keeps a render version per id: an unchanged + * descriptor is skipped and a changed one is updated in place instead of + * being removed and re-added. + */ + private fun reconcileShapes( + current: MutableMap, + versions: MutableMap, + next: Map, + version: (Descriptor) -> Long, + remove: (T) -> Unit, + add: (Descriptor) -> T?, + update: (T, Descriptor) -> Unit, + ) { + for (removedId in current.keys - next.keys) { + current.remove(removedId)?.let(remove) + versions.remove(removedId) + } + + for ((id, descriptor) in next) { + val nextVersion = version(descriptor) + val existing = current[id] + if (existing == null) { + add(descriptor)?.let { created -> + current[id] = created + versions[id] = nextVersion + } + } else if (versions[id] != nextVersion) { + update(existing, descriptor) + versions[id] = nextVersion + } + } + } + private fun reconcile( current: MutableMap, next: Map, @@ -599,6 +675,77 @@ class MapOverlayController( } } + /** + * One viewport query, cluster or filter pass, and diff, computed off the UI + * thread against an immutable spatial index. + */ + private data class ViewportRefreshRequest( + val generation: Int, + val index: MarkerSpatialIndex, + val bounds: LatLngBounds, + val latitudeSpan: Double, + val clustering: Boolean, + val widthPx: Int, + val heightPx: Int, + val displayedVersions: Map, + val animateEntering: Boolean, + val maxAnimatedMarkers: Int, + ) + + /** + * Coalesces refresh requests between the UI thread (producer) and the + * compute executor (consumer). At most one compute task is queued at a time; + * a request posted while one is queued replaces the pending request instead + * of adding another task, so a long gesture cannot build a backlog of stale + * work. The latest dataset generation is mirrored here so a queued index + * build can bail out before computing. + */ + private class RefreshInbox { + private val lock = Any() + private var pending: ViewportRefreshRequest? = null + private var isComputeQueued = false + private var latestDatasetGeneration = 0 + + fun recordDataset(generation: Int) { + synchronized(lock) { + latestDatasetGeneration = generation + } + } + + fun isCurrent(datasetGeneration: Int): Boolean { + return synchronized(lock) { datasetGeneration == latestDatasetGeneration } + } + + /** Returns true when the caller must enqueue a compute task. */ + fun post(request: ViewportRefreshRequest): Boolean { + return synchronized(lock) { + pending = request + if (isComputeQueued) { + false + } else { + isComputeQueued = true + true + } + } + } + + /** Hands the latest request to the compute task and frees the slot. */ + fun take(): ViewportRefreshRequest? { + return synchronized(lock) { + isComputeQueued = false + val request = pending + pending = null + request + } + } + + fun discardPending() { + synchronized(lock) { + pending = null + } + } + } + private companion object { /** Non-clustered datasets at or below this size reconcile synchronously. */ const val ASYNC_THRESHOLD = 500 diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt index 8cd2a94..bc45033 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt @@ -27,7 +27,9 @@ internal class MarkerIconFactory( private val density: Float, private val markerRegistry: () -> Map, ) { - private val cache = object : LruCache(64) {} + private val cache = object : LruCache(iconCacheBytes()) { + override fun sizeOf(key: String, value: CachedIcon): Int = value.byteCount + } private val sizeCache = object : LruCache>(64) {} private val mainHandler = Handler(Looper.getMainLooper()) private val appliedIconKeys = WeakHashMap() @@ -39,6 +41,8 @@ internal class MarkerIconFactory( var onIconApplied: () -> Unit, ) + private class CachedIcon(val descriptor: BitmapDescriptor, val byteCount: Int) + fun applyVisualProps( descriptor: MarkerDescriptor, marker: Marker, @@ -99,7 +103,7 @@ internal class MarkerIconFactory( } cache.get(iconKey)?.let { cached -> - marker.setIcon(cached) + marker.setIcon(cached.descriptor) setApplied(marker, iconKey) onIconApplied() return @@ -158,8 +162,8 @@ internal class MarkerIconFactory( onLoaded: (BitmapDescriptor?) -> Unit, ) { val key = cacheKey(image) - cache.get(key)?.let { - deliverOnMainThread { onLoaded(it) } + cache.get(key)?.let { cached -> + deliverOnMainThread { onLoaded(cached.descriptor) } return } @@ -352,7 +356,7 @@ internal class MarkerIconFactory( private fun cacheBitmap(key: String, bitmap: Bitmap): BitmapDescriptor { val descriptor = BitmapDescriptorFactory.fromBitmap(bitmap) - cache.put(key, descriptor) + cache.put(key, CachedIcon(descriptor, bitmap.allocationByteCount.coerceAtLeast(1))) sizeCache.put(key, bitmap.width.toFloat() to bitmap.height.toFloat()) return descriptor } @@ -468,6 +472,16 @@ internal class MarkerIconFactory( private const val DEFAULT_MARKER_HEIGHT_DP = 52f private const val MAX_DECODE_DIMENSION = 2048 private const val MAX_DECODE_PIXELS = 2048L * 2048L + private const val MIN_ICON_CACHE_BYTES = 1024 * 1024 + private const val MAX_ICON_CACHE_BYTES = 32 * 1024 * 1024 + + /** Icon cache budget in decoded bytes: a slice of the heap, capped well below it. */ + private fun iconCacheBytes(): Int { + val budget = Runtime.getRuntime().maxMemory() / 16 + return budget + .coerceIn(MIN_ICON_CACHE_BYTES.toLong(), MAX_ICON_CACHE_BYTES.toLong()) + .toInt() + } private val loadExecutor: ExecutorService = Executors.newSingleThreadExecutor() diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/PolygonDescriptor+PolygonOptions.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/PolygonDescriptor+PolygonOptions.kt index fd9ce29..30167fa 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/PolygonDescriptor+PolygonOptions.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/PolygonDescriptor+PolygonOptions.kt @@ -1,6 +1,8 @@ package com.margelo.nitro.nitromaps +import android.graphics.Color import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.Polygon import com.google.android.gms.maps.model.PolygonOptions fun PolygonDescriptor.toPolygonOptions(): PolygonOptions { @@ -14,3 +16,12 @@ fun PolygonDescriptor.toPolygonOptions(): PolygonOptions { return options } + +/** Updates an existing polygon in place, with the same defaults as [toPolygonOptions]. */ +fun PolygonDescriptor.applyTo(polygon: Polygon) { + polygon.points = coordinates.map { LatLng(it.latitude, it.longitude) } + polygon.strokeColor = strokeColor?.toColorInt() ?: Color.BLACK + polygon.fillColor = fillColor?.toColorInt() ?: Color.TRANSPARENT + polygon.strokeWidth = (strokeWidth ?: 2.0).toFloat() + polygon.isClickable = tappable == true +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/PolylineDescriptor+PolylineOptions.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/PolylineDescriptor+PolylineOptions.kt index ca7001e..642da75 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/PolylineDescriptor+PolylineOptions.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/PolylineDescriptor+PolylineOptions.kt @@ -1,6 +1,8 @@ package com.margelo.nitro.nitromaps +import android.graphics.Color import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.Polyline import com.google.android.gms.maps.model.PolylineOptions fun PolylineDescriptor.toPolylineOptions(): PolylineOptions { @@ -13,3 +15,11 @@ fun PolylineDescriptor.toPolylineOptions(): PolylineOptions { return options } + +/** Updates an existing polyline in place, with the same defaults as [toPolylineOptions]. */ +fun PolylineDescriptor.applyTo(polyline: Polyline) { + polyline.points = coordinates.map { LatLng(it.latitude, it.longitude) } + polyline.color = strokeColor?.toColorInt() ?: Color.BLACK + polyline.width = (strokeWidth ?: 4.0).toFloat() + polyline.isClickable = tappable == true +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+ApproximateEquality.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+ApproximateEquality.kt new file mode 100644 index 0000000..7fe8165 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+ApproximateEquality.kt @@ -0,0 +1,14 @@ +package com.margelo.nitro.nitromaps + +import kotlin.math.abs + +fun Region.approximatelyEquals( + other: Region, + coordinateEpsilon: Double = MapApproximateEquality.COORDINATE_EPSILON, + spanEpsilon: Double = MapApproximateEquality.SPAN_EPSILON, +): Boolean { + return abs(latitude - other.latitude) < coordinateEpsilon && + abs(longitude - other.longitude) < coordinateEpsilon && + abs(latitudeDelta - other.latitudeDelta) < spanEpsilon && + abs(longitudeDelta - other.longitudeDelta) < spanEpsilon +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/ShapeDescriptor+RenderVersion.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/ShapeDescriptor+RenderVersion.kt new file mode 100644 index 0000000..ca620a3 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/ShapeDescriptor+RenderVersion.kt @@ -0,0 +1,51 @@ +package com.margelo.nitro.nitromaps + +/** + * Render versions for shape overlay descriptors. + * + * Overlay controllers keep the version of every shape they have shown so a + * descriptor that is sent again unchanged costs one hash instead of a native + * remove-and-add, and a changed one is updated in place. + */ +private fun Array.geometrySignature(): Long { + var hash = size.toLong() + for (coordinate in this) { + hash = 1099511628211L * hash + java.lang.Double.doubleToLongBits(coordinate.latitude) + hash = 1099511628211L * hash + java.lang.Double.doubleToLongBits(coordinate.longitude) + } + return hash +} + +internal fun PolylineDescriptor.renderVersion(): Long = + renderSignature( + "polyline", + id, + coordinates.geometrySignature(), + strokeColor, + strokeWidth, + tappable, + ) + +internal fun PolygonDescriptor.renderVersion(): Long = + renderSignature( + "polygon", + id, + coordinates.geometrySignature(), + fillColor, + strokeColor, + strokeWidth, + tappable, + ) + +internal fun CircleDescriptor.renderVersion(): Long = + renderSignature( + "circle", + id, + center.latitude, + center.longitude, + radius, + fillColor, + strokeColor, + strokeWidth, + tappable, + ) diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/ShapeRenderVersionTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/ShapeRenderVersionTest.kt new file mode 100644 index 0000000..5ab8c4f --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/ShapeRenderVersionTest.kt @@ -0,0 +1,100 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test + +class ShapeRenderVersionTest { + private val route = arrayOf(Coordinate(52.2297, 21.0122), Coordinate(52.237, 21.017)) + + private fun polyline( + id: String = "route", + coordinates: Array = route, + strokeColor: String? = "#FF0000", + strokeWidth: Double? = 4.0, + tappable: Boolean? = true, + ) = PolylineDescriptor(id, coordinates, strokeColor, strokeWidth, tappable) + + private fun polygon( + id: String = "district", + coordinates: Array = route, + fillColor: String? = "#007AFF33", + strokeColor: String? = "#007AFF", + strokeWidth: Double? = 2.0, + tappable: Boolean? = false, + ) = PolygonDescriptor(id, coordinates, fillColor, strokeColor, strokeWidth, tappable) + + private fun circle( + id: String = "radius", + center: Coordinate = Coordinate(52.22, 21.01), + radius: Double = 800.0, + fillColor: String? = "#34C75933", + strokeColor: String? = "#34C759", + strokeWidth: Double? = 2.0, + tappable: Boolean? = true, + ) = CircleDescriptor(id, center, radius, fillColor, strokeColor, strokeWidth, tappable) + + @Test + fun `equal descriptors share a version`() { + assertEquals(polyline().renderVersion(), polyline(coordinates = route.copyOf()).renderVersion()) + assertEquals(polygon().renderVersion(), polygon(coordinates = route.copyOf()).renderVersion()) + assertEquals(circle().renderVersion(), circle().renderVersion()) + } + + @Test + fun `every polyline field changes the version`() { + val base = polyline().renderVersion() + assertNotEquals("id", base, polyline(id = "other").renderVersion()) + assertNotEquals( + "coordinate", + base, + polyline(coordinates = arrayOf(route[0], Coordinate(52.24, 21.02))).renderVersion(), + ) + assertNotEquals("point count", base, polyline(coordinates = arrayOf(route[0])).renderVersion()) + assertNotEquals("strokeColor", base, polyline(strokeColor = "#00FF00").renderVersion()) + assertNotEquals("cleared strokeColor", base, polyline(strokeColor = null).renderVersion()) + assertNotEquals("strokeWidth", base, polyline(strokeWidth = 5.0).renderVersion()) + assertNotEquals("tappable", base, polyline(tappable = false).renderVersion()) + } + + @Test + fun `every polygon field changes the version`() { + val base = polygon().renderVersion() + assertNotEquals("id", base, polygon(id = "other").renderVersion()) + assertNotEquals( + "coordinate", + base, + polygon(coordinates = arrayOf(route[0], Coordinate(52.24, 21.02))).renderVersion(), + ) + assertNotEquals("fillColor", base, polygon(fillColor = "#00000000").renderVersion()) + assertNotEquals("strokeColor", base, polygon(strokeColor = "#000000").renderVersion()) + assertNotEquals("strokeWidth", base, polygon(strokeWidth = 3.0).renderVersion()) + assertNotEquals("tappable", base, polygon(tappable = true).renderVersion()) + } + + @Test + fun `every circle field changes the version`() { + val base = circle().renderVersion() + assertNotEquals("id", base, circle(id = "other").renderVersion()) + assertNotEquals("center", base, circle(center = Coordinate(52.23, 21.01)).renderVersion()) + assertNotEquals("radius", base, circle(radius = 900.0).renderVersion()) + assertNotEquals("fillColor", base, circle(fillColor = "#00000000").renderVersion()) + assertNotEquals("strokeColor", base, circle(strokeColor = "#000000").renderVersion()) + assertNotEquals("strokeWidth", base, circle(strokeWidth = 3.0).renderVersion()) + assertNotEquals("tappable", base, circle(tappable = false).renderVersion()) + } + + @Test + fun `absent and zero valued fields are distinguishable`() { + assertNotEquals(polyline(strokeWidth = null).renderVersion(), polyline(strokeWidth = 0.0).renderVersion()) + assertNotEquals(circle(tappable = null).renderVersion(), circle(tappable = false).renderVersion()) + } + + @Test + fun `region equality tolerates rounding but not real changes`() { + val region = Region(52.2297, 21.0122, 0.1, 0.2) + assertEquals(true, region.approximatelyEquals(Region(52.2297 + 1e-9, 21.0122, 0.1, 0.2))) + assertEquals(false, region.approximatelyEquals(Region(52.2397, 21.0122, 0.1, 0.2))) + assertEquals(false, region.approximatelyEquals(Region(52.2297, 21.0122, 0.1, 0.3))) + } +} diff --git a/package/ios/GoogleMapOverlayController.swift b/package/ios/GoogleMapOverlayController.swift index d033447..ec26cdb 100644 --- a/package/ios/GoogleMapOverlayController.swift +++ b/package/ios/GoogleMapOverlayController.swift @@ -26,6 +26,9 @@ final class GoogleMapOverlayController { private var polylines: [String: GMSPolyline] = [:] private var polygons: [String: GMSPolygon] = [:] private var circles: [String: GMSCircle] = [:] + private var polylineVersions: [String: ShapeRenderVersion] = [:] + private var polygonVersions: [String: ShapeRenderVersion] = [:] + private var circleVersions: [String: ShapeRenderVersion] = [:] private let markerPipeline: MarkerRenderPipeline private let visualApplier = GoogleMarkerVisualApplier() private var clusterIconCache: [String: UIImage] = [:] @@ -160,6 +163,7 @@ final class GoogleMapOverlayController { func updatePolylines(_ descriptors: [PolylineDescriptor]?) { reconcile( current: &polylines, + versions: &polylineVersions, next: descriptors ?? [], make: makePolyline, update: updatePolyline @@ -169,6 +173,7 @@ final class GoogleMapOverlayController { func updatePolygons(_ descriptors: [PolygonDescriptor]?) { reconcile( current: &polygons, + versions: &polygonVersions, next: descriptors ?? [], make: makePolygon, update: updatePolygon @@ -178,6 +183,7 @@ final class GoogleMapOverlayController { func updateCircles(_ descriptors: [CircleDescriptor]?) { reconcile( current: &circles, + versions: &circleVersions, next: descriptors ?? [], make: makeCircle, update: updateCircle @@ -375,6 +381,9 @@ final class GoogleMapOverlayController { polylines.removeAll() polygons.removeAll() circles.removeAll() + polylineVersions.removeAll() + polygonVersions.removeAll() + circleVersions.removeAll() } private func makePolyline(_ descriptor: PolylineDescriptor) -> GMSPolyline { @@ -429,8 +438,11 @@ final class GoogleMapOverlayController { circle.userData = descriptor.id } + /// Keeps a render version per overlay id so a descriptor that is sent again + /// unchanged costs one hash, and a changed one is updated in place. private func reconcile( current: inout [String: Overlay], + versions: inout [String: ShapeRenderVersion], next descriptors: [Descriptor], make: (Descriptor) -> Overlay, update: (Overlay, Descriptor) -> Void @@ -442,23 +454,29 @@ final class GoogleMapOverlayController { var nextIds = Set() for descriptor in descriptors { nextIds.insert(descriptor.id) + let version = descriptor.renderVersion() if let overlay = current[descriptor.id] { - update(overlay, descriptor) + if versions[descriptor.id] != version { + update(overlay, descriptor) + } } else { let overlay = make(descriptor) overlay.map = mapView current[descriptor.id] = overlay } + versions[descriptor.id] = version } for id in Set(current.keys).subtracting(nextIds) { current.removeValue(forKey: id)?.map = nil + versions.removeValue(forKey: id) } } } private protocol IdentifiedOverlayDescriptor { var id: String { get } + func renderVersion() -> ShapeRenderVersion } extension PolylineDescriptor: IdentifiedOverlayDescriptor {} diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index 95c5b6a..8b00701 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -18,6 +18,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { private var myLocationObservation: NSKeyValueObservation? private weak var followedLocationMapView: GMSMapView? private var _googleMapId: String? + private var lastAppliedRegion: Region? + private var lastAppliedRegionCamera: GMSCameraPosition? fileprivate lazy var overlayController: GoogleMapOverlayController = { let controller = GoogleMapOverlayController(mapView: view) @@ -261,6 +263,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { isUserRegionChange = false isUserGestureMoving = false lastLiveMarkerRefreshTime = 0 + lastAppliedRegion = nil + lastAppliedRegionCamera = nil isMapReady = false hasDeliveredMapReady = false view.delegate = nil @@ -301,11 +305,24 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } private func applyRegion(_ region: Region, animated: Bool = false) { + if let lastAppliedRegion, + let lastAppliedRegionCamera, + region.approximatelyEquals(lastAppliedRegion), + view.camera.approximatelyEquals(lastAppliedRegionCamera) { + // Same region as last time and the camera has not moved since, so the + // fit would land on the camera the map already shows. + return + } + applyCameraUpdate( GMSCameraUpdate.fit(region.toGMSCoordinateBounds(), with: mapPadding?.toUIEdgeInsets() ?? .zero), animated: animated, duration: nil ) + self.lastAppliedRegion = region + // `moveCamera` updates `camera` synchronously; an animation does not, so + // there is nothing reliable to remember until it settles. + lastAppliedRegionCamera = animated ? nil : view.camera } private func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double? = nil) { diff --git a/package/ios/MapOverlayController.swift b/package/ios/MapOverlayController.swift index 73a6378..c3929e0 100644 --- a/package/ios/MapOverlayController.swift +++ b/package/ios/MapOverlayController.swift @@ -24,6 +24,7 @@ final class MapOverlayController { private var displayedAnnotationVersions: [String: Int] = [:] private let markerPipeline = MarkerRenderPipeline() private var shapeOverlays: [String: MKOverlay] = [:] + private var shapeVersions: [String: ShapeRenderVersion] = [:] private var overlayStyles: [ObjectIdentifier: OverlayStyle] = [:] var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? @@ -55,6 +56,7 @@ final class MapOverlayController { displayedAnnotations.removeAll() displayedAnnotationVersions.removeAll() shapeOverlays.removeAll() + shapeVersions.removeAll() overlayStyles.removeAll() } @@ -206,7 +208,8 @@ final class MapOverlayController { strokeWidth: CGFloat(descriptor.strokeWidth ?? 4), tappable: descriptor.tappable ?? false ) - } + }, + renderVersion: { $0.renderVersion() } ) } @@ -225,7 +228,8 @@ final class MapOverlayController { strokeWidth: CGFloat(descriptor.strokeWidth ?? 2), tappable: descriptor.tappable ?? false ) - } + }, + renderVersion: { $0.renderVersion() } ) } @@ -244,7 +248,8 @@ final class MapOverlayController { strokeWidth: CGFloat(descriptor.strokeWidth ?? 2), tappable: descriptor.tappable ?? true ) - } + }, + renderVersion: { $0.renderVersion() } ) } @@ -263,13 +268,17 @@ final class MapOverlayController { renderer = MKCircleRenderer(overlay: overlay) } + apply(style, to: renderer) + + return renderer + } + + private func apply(_ style: OverlayStyle, to renderer: MKOverlayPathRenderer) { renderer.strokeColor = style.strokeColor renderer.lineWidth = style.strokeWidth if let fillColor = style.fillColor { renderer.fillColor = fillColor } - - return renderer } func overlayId(at point: CGPoint) -> String? { @@ -303,50 +312,80 @@ final class MapOverlayController { shapeOverlays[id].flatMap { overlayStyles[ObjectIdentifier($0)]?.kind } } + /// Reconciles one overlay kind against its descriptors. + /// + /// Each shown overlay keeps a render version. A descriptor sent again + /// unchanged is skipped; a style-only change restyles the cached renderer in + /// place; a geometry change replaces the overlay at its previous z-position, + /// because MapKit overlay geometry is immutable. private func reconcileShapeOverlays( _ descriptors: [Descriptor], kind: OverlayKind, makeOverlay: (Descriptor) -> MKOverlay, - makeStyle: (Descriptor) -> OverlayStyle + makeStyle: (Descriptor) -> OverlayStyle, + renderVersion: (Descriptor) -> ShapeRenderVersion ) { guard let mapView else { return } - let nextIds = Set( - descriptors.compactMap { descriptor -> String? in - let style = makeStyle(descriptor) - return style.kind == kind ? style.id : nil - } - ) - let existingIds = Set( - shapeOverlays.compactMap { id, overlay -> String? in - overlayStyles[ObjectIdentifier(overlay)]?.kind == kind ? id : nil - } - ) - - for removedId in existingIds.subtracting(nextIds) { - if let overlay = shapeOverlays.removeValue(forKey: removedId) { - overlayStyles.removeValue(forKey: ObjectIdentifier(overlay)) - mapView.removeOverlay(overlay) - } - } + var nextIds = Set() + nextIds.reserveCapacity(descriptors.count) for descriptor in descriptors { let style = makeStyle(descriptor) guard style.kind == kind else { continue } + nextIds.insert(style.id) + let version = renderVersion(descriptor) + if let existingOverlay = shapeOverlays[style.id], + let existingVersion = shapeVersions[style.id] { + if existingVersion == version { + continue + } + + if existingVersion.geometry == version.geometry { + // Style-only change: restyle the cached renderer in place. + overlayStyles[ObjectIdentifier(existingOverlay)] = style + if let renderer = mapView.renderer(for: existingOverlay) as? MKOverlayPathRenderer { + apply(style, to: renderer) + renderer.setNeedsDisplay() + } + shapeVersions[style.id] = version + continue + } + } + + let overlay = makeOverlay(descriptor) if let existingOverlay = shapeOverlays[style.id] { overlayStyles.removeValue(forKey: ObjectIdentifier(existingOverlay)) + let previousIndex = mapView.overlays.firstIndex { $0 === existingOverlay } mapView.removeOverlay(existingOverlay) + if let previousIndex { + mapView.insertOverlay(overlay, at: previousIndex) + } else { + mapView.addOverlay(overlay) + } + } else { + mapView.addOverlay(overlay) } - - let overlay = makeOverlay(descriptor) shapeOverlays[style.id] = overlay overlayStyles[ObjectIdentifier(overlay)] = style - mapView.addOverlay(overlay) + shapeVersions[style.id] = version + } + + let removedIds = shapeOverlays.compactMap { id, overlay -> String? in + overlayStyles[ObjectIdentifier(overlay)]?.kind == kind && !nextIds.contains(id) ? id : nil + } + for removedId in removedIds { + guard let overlay = shapeOverlays.removeValue(forKey: removedId) else { + continue + } + shapeVersions.removeValue(forKey: removedId) + overlayStyles.removeValue(forKey: ObjectIdentifier(overlay)) + mapView.removeOverlay(overlay) } } } diff --git a/package/ios/MarkerClusterEngine.swift b/package/ios/MarkerClusterEngine.swift index d45c6b6..c9ead35 100644 --- a/package/ios/MarkerClusterEngine.swift +++ b/package/ios/MarkerClusterEngine.swift @@ -124,7 +124,7 @@ enum MarkerClusterEngine { private static let mergeGap = ClusterBadgeMetrics.mergeGap private struct Bucket { - var key = "" + let key: String var count = 0 var sumLat = 0.0 var sumLon = 0.0 @@ -135,6 +135,26 @@ enum MarkerClusterEngine { var memberIds: [String] = [] var first: MarkerDescriptor? + init(key: String) { + self.key = key + } + + /// Adds one marker. Called through `Dictionary.subscript(_:default:)` so + /// the bucket is mutated in place and `memberIds` keeps a unique buffer. + mutating func include(_ descriptor: MarkerDescriptor, lat: Double, lon: Double) { + count += 1 + sumLat += lat + sumLon += lon + minLat = min(minLat, lat) + maxLat = max(maxLat, lat) + minLon = min(minLon, lon) + maxLon = max(maxLon, lon) + if first == nil { + first = descriptor + } + memberIds.append(descriptor.id) + } + /// Folds another bucket's members in. The receiver keeps its own key/first, /// so callers should seed groups with the dominant (largest) bucket. mutating func absorb(_ other: Bucket) { @@ -194,20 +214,10 @@ enum MarkerClusterEngine { let col = Int((lon / cellLon).rounded(.down)) let key = "\(row):\(col)" - var bucket = buckets[key] ?? Bucket() - bucket.key = key - bucket.count += 1 - bucket.sumLat += lat - bucket.sumLon += lon - bucket.minLat = min(bucket.minLat, lat) - bucket.maxLat = max(bucket.maxLat, lat) - bucket.minLon = min(bucket.minLon, lon) - bucket.maxLon = max(bucket.maxLon, lon) - if bucket.first == nil { - bucket.first = descriptor - } - bucket.memberIds.append(descriptor.id) - buckets[key] = bucket + // Copying the bucket out, appending, and writing it back shared the + // member array with the dictionary's copy, so every append copied the + // whole array (O(k²) per cell). The default subscript mutates in place. + buckets[key, default: Bucket(key: key)].include(descriptor, lat: lat, lon: lon) } let merged = mergeOverlapping( @@ -356,13 +366,92 @@ final class MarkerRenderPipeline { private static let asyncThreshold = 500 static let liveRefreshInterval: TimeInterval = 0.1 + /// The inputs of one refresh: what to show for a viewport, diffed against + /// what is shown, and where to deliver the result. + private struct RefreshParameters { + let displayedVersions: [String: Int] + let region: MKCoordinateRegion + let viewSize: CGSize + let apply: (MarkerRenderDiff) -> Void + } + + /// One viewport query, cluster or filter pass, and diff, computed off the + /// main thread against an immutable spatial index. + private struct ViewportRefreshRequest { + let generation: Int + let index: MarkerSpatialIndex + let clustering: Bool + let parameters: RefreshParameters + } + + /// Coalesces refresh requests between the main thread (producer) and the + /// compute queue (consumer). At most one compute block is queued at a time; a + /// request posted while one is queued replaces the pending request instead of + /// adding another block, so a long gesture cannot build a backlog of stale + /// work. The latest generations are mirrored here so queued work can bail + /// out before computing. + private final class RefreshInbox { + private let lock = NSLock() + private var pending: ViewportRefreshRequest? + private var isComputeQueued = false + private var latestDatasetGeneration = 0 + + func record(datasetGeneration: Int) { + lock.lock() + latestDatasetGeneration = datasetGeneration + lock.unlock() + } + + func isCurrent(datasetGeneration: Int) -> Bool { + lock.lock() + defer { lock.unlock() } + return datasetGeneration == latestDatasetGeneration + } + + /// Stores `request` as the latest one. Returns true when the caller must + /// enqueue a compute block, false when a queued block will pick it up. + func post(_ request: ViewportRefreshRequest) -> Bool { + lock.lock() + defer { lock.unlock() } + pending = request + if isComputeQueued { + return false + } + isComputeQueued = true + return true + } + + /// Hands the latest request to the compute block and frees the slot. + func take() -> ViewportRefreshRequest? { + lock.lock() + defer { lock.unlock() } + isComputeQueued = false + let request = pending + pending = nil + return request + } + + func discardPending() { + lock.lock() + pending = nil + lock.unlock() + } + } + private let clusterCellPoints: Double private var allMarkerDescriptors: [MarkerDescriptor] = [] private var spatialIndex: MarkerSpatialIndex? private var viewportRefreshWorkItem: DispatchWorkItem? private var markersFingerprint = 0 + /// Invalidates in-flight refresh results (viewport diffs). private var refreshGeneration = 0 + /// Invalidates in-flight index builds. Kept apart from `refreshGeneration` + /// so a burst of refreshes during a gesture cannot keep discarding the index + /// build for a dataset that has not changed. + private var datasetGeneration = 0 + private var latestRefreshParameters: RefreshParameters? private var clusteringEnabled = false + private let refreshInbox = RefreshInbox() private let computeQueue = DispatchQueue( label: "com.nitromaps.markerCompute", qos: .userInitiated @@ -380,6 +469,9 @@ final class MarkerRenderPipeline { viewportRefreshWorkItem?.cancel() viewportRefreshWorkItem = nil refreshGeneration += 1 + advanceDatasetGeneration() + refreshInbox.discardPending() + latestRefreshParameters = nil allMarkerDescriptors.removeAll() spatialIndex = nil markersFingerprint = 0 @@ -405,6 +497,7 @@ final class MarkerRenderPipeline { markersFingerprint = fingerprint allMarkerDescriptors = next spatialIndex = nil + advanceDatasetGeneration() return true } @@ -414,17 +507,20 @@ final class MarkerRenderPipeline { viewSize: CGSize, apply: @escaping (MarkerRenderDiff) -> Void ) { + let parameters = RefreshParameters( + displayedVersions: displayedVersions, + region: region, + viewSize: viewSize, + apply: apply + ) if usesViewportPipeline { - rebuildIndexAndRefresh( - displayedVersions: displayedVersions, - region: region, - viewSize: viewSize, - apply: apply - ) + rebuildIndexAndRefresh(parameters) } else { viewportRefreshWorkItem?.cancel() viewportRefreshWorkItem = nil refreshGeneration += 1 + refreshInbox.discardPending() + latestRefreshParameters = parameters apply(Self.computeDiff( target: allMarkerDescriptors.map { .single($0) }, displayed: displayedVersions @@ -476,74 +572,106 @@ final class MarkerRenderPipeline { return } + let parameters = RefreshParameters( + displayedVersions: displayedVersions, + region: region, + viewSize: viewSize, + apply: apply + ) guard let index = spatialIndex else { - rebuildIndexAndRefresh( - displayedVersions: displayedVersions, - region: region, - viewSize: viewSize, - apply: apply - ) + rebuildIndexAndRefresh(parameters) return } - let clustering = clusteringEnabled + refreshNow(parameters, index: index) + } + + private func refreshNow(_ parameters: RefreshParameters, index: MarkerSpatialIndex) { + latestRefreshParameters = parameters refreshGeneration += 1 - let generation = refreshGeneration - let clusterCellPoints = self.clusterCellPoints + let request = ViewportRefreshRequest( + generation: refreshGeneration, + index: index, + clustering: clusteringEnabled, + parameters: parameters + ) + guard refreshInbox.post(request) else { + // A compute block is already queued and will pick this request up. + return + } + let clusterCellPoints = self.clusterCellPoints computeQueue.async { [weak self] in - let candidates = index.candidates(in: region) - let elements: [MarkerClusterEngine.Element] - if clustering { - elements = MarkerClusterEngine.clusters( - candidates: candidates, - region: region, - viewSize: viewSize, - cellPoints: clusterCellPoints - ) - } else { - elements = MarkerViewportFilter - .displaySubset(candidates: candidates, region: region) - .map { .single($0) } + guard let self, let request = self.refreshInbox.take() else { + return } - let diff = Self.computeDiff(target: elements, displayed: displayedVersions) - DispatchQueue.main.async { - guard let self, generation == self.refreshGeneration else { + let diff = Self.computeViewportDiff(request, clusterCellPoints: clusterCellPoints) + DispatchQueue.main.async { [weak self] in + guard let self, request.generation == self.refreshGeneration else { return } - apply(diff) + request.parameters.apply(diff) } } } - private func rebuildIndexAndRefresh( - displayedVersions: [String: Int], - region: MKCoordinateRegion, - viewSize: CGSize, - apply: @escaping (MarkerRenderDiff) -> Void - ) { - let descriptors = allMarkerDescriptors + private func rebuildIndexAndRefresh(_ parameters: RefreshParameters) { + latestRefreshParameters = parameters + // Diffs computed against the previous index are stale from here on. refreshGeneration += 1 - let generation = refreshGeneration + let descriptors = allMarkerDescriptors + let builtForDataset = datasetGeneration computeQueue.async { [weak self] in + guard let self, self.refreshInbox.isCurrent(datasetGeneration: builtForDataset) else { + // A newer dataset superseded this build before it started. + return + } + let index = MarkerSpatialIndex(markers: descriptors) - DispatchQueue.main.async { - guard let self, generation == self.refreshGeneration else { + DispatchQueue.main.async { [weak self] in + guard let self, builtForDataset == self.datasetGeneration else { return } self.spatialIndex = index - self.refreshNow( - displayedVersions: displayedVersions, - region: region, - viewSize: viewSize, - apply: apply - ) + // Refresh for the viewport that was requested most recently, not the + // one that was current when the build was queued. + if let latest = self.latestRefreshParameters, self.usesViewportPipeline { + self.refreshNow(latest, index: index) + } } } } + private func advanceDatasetGeneration() { + datasetGeneration += 1 + refreshInbox.record(datasetGeneration: datasetGeneration) + } + + private static func computeViewportDiff( + _ request: ViewportRefreshRequest, + clusterCellPoints: Double + ) -> MarkerRenderDiff { + let parameters = request.parameters + let candidates = request.index.candidates(in: parameters.region) + let elements: [MarkerClusterEngine.Element] + if request.clustering { + elements = MarkerClusterEngine.clusters( + candidates: candidates, + region: parameters.region, + viewSize: parameters.viewSize, + cellPoints: clusterCellPoints + ) + } else { + elements = MarkerViewportFilter + .displaySubset(candidates: candidates, region: parameters.region) + .map { .single($0) } + } + + return computeDiff(target: elements, displayed: parameters.displayedVersions) + } + private static func computeDiff( target: [MarkerClusterEngine.Element], displayed: [String: Int] diff --git a/package/ios/MarkerImageLoader.swift b/package/ios/MarkerImageLoader.swift index c1ca8fe..5d9bd8f 100644 --- a/package/ios/MarkerImageLoader.swift +++ b/package/ios/MarkerImageLoader.swift @@ -3,7 +3,14 @@ import UIKit /// Loads marker images from bundled assets or remote URLs with in-memory caching. /// Local images decode off-thread; completions always land on main. enum MarkerImageLoader { - private static let cache = NSCache() + private static let maximumCachedImages = 256 + private static let maximumCacheBytes = 32 * 1024 * 1024 + private static let cache: NSCache = { + let cache = NSCache() + cache.countLimit = maximumCachedImages + cache.totalCostLimit = maximumCacheBytes + return cache + }() private static let session = URLSession.shared private static let decodeQueue = DispatchQueue( label: "com.nitromaps.markerImageDecode", @@ -33,7 +40,7 @@ enum MarkerImageLoader { decodeQueue.async { let loaded = loadLocal(uri: uri, image: image) if let loaded { - cache.setObject(loaded, forKey: cacheKey) + cache.setObject(loaded, forKey: cacheKey, cost: byteCost(of: loaded)) } DispatchQueue.main.async { completion(loaded) @@ -88,7 +95,7 @@ enum MarkerImageLoader { if let data, let decoded = UIImage(data: data) { uiImage = resize(decoded, image: image) if let uiImage { - cache.setObject(uiImage, forKey: cacheKey) + cache.setObject(uiImage, forKey: cacheKey, cost: byteCost(of: uiImage)) } } else { uiImage = nil @@ -100,6 +107,13 @@ enum MarkerImageLoader { }.resume() } + /// Decoded size in bytes, so the cache evicts by memory rather than by count. + private static func byteCost(of image: UIImage) -> Int { + let pixelWidth = Int(image.size.width * image.scale) + let pixelHeight = Int(image.size.height * image.scale) + return max(1, pixelWidth * pixelHeight * 4) + } + private static func resize(_ uiImage: UIImage, image: MarkerImage) -> UIImage { guard let width = image.width, let height = image.height else { return uiImage diff --git a/package/ios/NitroPinAnnotationView.swift b/package/ios/NitroPinAnnotationView.swift index 3a32b1c..06e5ba8 100644 --- a/package/ios/NitroPinAnnotationView.swift +++ b/package/ios/NitroPinAnnotationView.swift @@ -30,7 +30,10 @@ final class NitroPinAnnotationView: MKMarkerAnnotationView { displayPriority = .required alpha = marker.opacity - layoutIfNeeded() + // No forced layout here: this runs inside MapKit's `viewFor` callback for + // every pin entering the viewport. The marker view keeps one size across + // reuse, so a zero size only happens before the first layout, and the + // default covers that. let pinSize = bounds.size == .zero ? Self.defaultPinSize : bounds.size centerOffset = marker.centerOffset(forImageSize: pinSize) diff --git a/package/ios/Region+ApproximateEquality.swift b/package/ios/Region+ApproximateEquality.swift new file mode 100644 index 0000000..9124d03 --- /dev/null +++ b/package/ios/Region+ApproximateEquality.swift @@ -0,0 +1,12 @@ +extension Region { + func approximatelyEquals( + _ other: Region, + coordinateEpsilon: Double = MapApproximateEquality.coordinateEpsilon, + spanEpsilon: Double = MapApproximateEquality.spanEpsilon + ) -> Bool { + abs(latitude - other.latitude) < coordinateEpsilon + && abs(longitude - other.longitude) < coordinateEpsilon + && abs(latitudeDelta - other.latitudeDelta) < spanEpsilon + && abs(longitudeDelta - other.longitudeDelta) < spanEpsilon + } +} diff --git a/package/ios/ShapeDescriptor+RenderVersion.swift b/package/ios/ShapeDescriptor+RenderVersion.swift new file mode 100644 index 0000000..2717b8c --- /dev/null +++ b/package/ios/ShapeDescriptor+RenderVersion.swift @@ -0,0 +1,91 @@ +/// Render versions for shape overlay descriptors. +/// +/// Overlay controllers keep the version of every shape they have shown so a +/// descriptor that is sent again unchanged costs one hash instead of a native +/// remove-and-add. Geometry and style are versioned separately: a geometry +/// change needs a new SDK overlay, a style-only change is applied in place. +extension Array where Element == Coordinate { + func hashGeometry(into hasher: inout Hasher) { + hasher.combine(count) + for coordinate in self { + hasher.combine(coordinate.latitude) + hasher.combine(coordinate.longitude) + } + } +} + +extension PolylineDescriptor { + func geometryVersion() -> Int { + var hasher = Hasher() + coordinates.hashGeometry(into: &hasher) + return hasher.finalize() + } + + func styleVersion() -> Int { + var hasher = Hasher() + hasher.combine(strokeColor) + hasher.combine(strokeWidth) + hasher.combine(tappable) + return hasher.finalize() + } +} + +extension PolygonDescriptor { + func geometryVersion() -> Int { + var hasher = Hasher() + coordinates.hashGeometry(into: &hasher) + return hasher.finalize() + } + + func styleVersion() -> Int { + var hasher = Hasher() + hasher.combine(fillColor) + hasher.combine(strokeColor) + hasher.combine(strokeWidth) + hasher.combine(tappable) + return hasher.finalize() + } +} + +extension CircleDescriptor { + func geometryVersion() -> Int { + var hasher = Hasher() + hasher.combine(center.latitude) + hasher.combine(center.longitude) + hasher.combine(radius) + return hasher.finalize() + } + + func styleVersion() -> Int { + var hasher = Hasher() + hasher.combine(fillColor) + hasher.combine(strokeColor) + hasher.combine(strokeWidth) + hasher.combine(tappable) + return hasher.finalize() + } +} + +/// Geometry and style versions of a shown shape overlay, kept per overlay id. +struct ShapeRenderVersion: Equatable { + let geometry: Int + let style: Int +} + +extension PolylineDescriptor { + func renderVersion() -> ShapeRenderVersion { + ShapeRenderVersion(geometry: geometryVersion(), style: styleVersion()) + } +} + +extension PolygonDescriptor { + func renderVersion() -> ShapeRenderVersion { + ShapeRenderVersion(geometry: geometryVersion(), style: styleVersion()) + } +} + +extension CircleDescriptor { + func renderVersion() -> ShapeRenderVersion { + ShapeRenderVersion(geometry: geometryVersion(), style: styleVersion()) + } +} diff --git a/package/react-native-better-maps.podspec b/package/react-native-better-maps.podspec index ecb5fa8..a7e01e8 100644 --- a/package/react-native-better-maps.podspec +++ b/package/react-native-better-maps.podspec @@ -2,12 +2,16 @@ require 'json' package = JSON.parse(File.read(File.join(__dir__, 'package.json'))) -def better_maps_podfile_properties +# Helpers are lambdas in local variables rather than top-level `def`s: CocoaPods +# evaluates a podspec with `eval`, and a method defined that way is not visible +# from inside the `Pod::Spec.new` block on every Ruby / CocoaPods combination +# (Ruby 4.0 + CocoaPods 1.17 raises `undefined method ... for module Pod`). +better_maps_podfile_properties = lambda do installation_root = Pod::Config.instance.installation_root - return {} if installation_root.nil? + next {} if installation_root.nil? podfile_properties_path = File.join(installation_root, 'Podfile.properties.json') - return {} unless File.exist?(podfile_properties_path) + next {} unless File.exist?(podfile_properties_path) JSON.parse(File.read(podfile_properties_path)) rescue StandardError => e @@ -15,10 +19,9 @@ rescue StandardError => e {} end -def better_maps_ios_google_provider_enabled? - # Must match IOS_GOOGLE_PROVIDER_PODFILE_PROPERTY in plugin/src/ios.ts - better_maps_podfile_properties['betterMaps.iosGoogleProvider'] == 'true' -end +# Must match IOS_GOOGLE_PROVIDER_PODFILE_PROPERTY in plugin/src/ios.ts +better_maps_ios_google_provider_enabled = + better_maps_podfile_properties.call['betterMaps.iosGoogleProvider'] == 'true' Pod::Spec.new do |s| s.name = 'react-native-better-maps' @@ -44,7 +47,7 @@ Pod::Spec.new do |s| s.dependency 'React-jsi' s.dependency 'React-callinvoker' - if better_maps_ios_google_provider_enabled? + if better_maps_ios_google_provider_enabled s.dependency 'GoogleMaps' end diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 132b08e..4d0ec7d 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -28,6 +28,11 @@ import type { Coordinate } from '../types/coordinate'; import type { MapViewProps, PoiPressEvent } from '../types/map'; import type { MapViewRef } from '../types/ref'; import { normalizeEnteringAnimation } from '../utils/enteringAnimation'; +import { + camerasEqual, + edgePaddingsEqual, + regionsEqual, +} from '../utils/mapValueEquality'; const MAP_VIEW_NOT_MOUNTED_ERROR = 'MapView is not mounted'; @@ -131,6 +136,13 @@ export function MapView({ enteringAnimationsEqual, ); + // `region`, `camera` and `mapPadding` are usually written inline in JSX. The + // Google providers answer a new `region` with a camera move, so an + // equal-but-new object must not reach native. + const stableRegion = useStableValue(region, regionsEqual); + const stableCamera = useStableValue(camera, camerasEqual); + const stableMapPadding = useStableValue(mapPadding, edgePaddingsEqual); + const hasMarkerPress = onMarkerPressProp != null || hasCollectedMarkerPress; const hasMarkerDragEnd = @@ -276,8 +288,8 @@ export function MapView({ provider={resolvedProvider} googleMapId={googleMapId} mapType={mapType} - region={region} - camera={camera} + region={stableRegion} + camera={stableCamera} scrollEnabled={scrollEnabled} zoomEnabled={zoomEnabled} rotateEnabled={rotateEnabled} @@ -288,7 +300,7 @@ export function MapView({ showsScale={showsScale} customMapStyle={customMapStyle} clusteringEnabled={clusteringEnabled} - mapPadding={mapPadding} + mapPadding={stableMapPadding} markerEnteringAnimation={markerEntering} clusterEnteringAnimation={clusterEntering} markers={markers} diff --git a/package/src/utils/__tests__/mapValueEquality.test.ts b/package/src/utils/__tests__/mapValueEquality.test.ts new file mode 100644 index 0000000..21e6d69 --- /dev/null +++ b/package/src/utils/__tests__/mapValueEquality.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, test } from 'bun:test'; +import type { Camera } from '../../types/camera'; +import type { EdgePadding, Region } from '../../types/region'; +import { + camerasEqual, + edgePaddingsEqual, + regionsEqual, +} from '../mapValueEquality'; + +type Change = [string, (base: Value) => Value]; + +function describeFieldCoverage( + name: string, + base: Value, + valuesEqual: (left: Value | undefined, right: Value | undefined) => boolean, + changes: Array>, +) { + describe(name, () => { + test('treats a structural clone as equal', () => { + expect(valuesEqual(base, structuredClone(base))).toBe(true); + }); + + test('treats the same reference as equal', () => { + expect(valuesEqual(base, base)).toBe(true); + }); + + test('treats two undefined values as equal', () => { + expect(valuesEqual(undefined, undefined)).toBe(true); + }); + + test('treats a value and undefined as different', () => { + expect(valuesEqual(base, undefined)).toBe(false); + expect(valuesEqual(undefined, base)).toBe(false); + }); + + for (const [field, change] of changes) { + test(`detects a change to ${field}`, () => { + expect(valuesEqual(base, change(base))).toBe(false); + }); + } + }); +} + +const baseRegion: Region = { + latitude: 52.2297, + longitude: 21.0122, + latitudeDelta: 0.1, + longitudeDelta: 0.2, +}; + +describeFieldCoverage('regionsEqual', baseRegion, regionsEqual, [ + ['latitude', (r) => ({ ...r, latitude: 0 })], + ['longitude', (r) => ({ ...r, longitude: 0 })], + ['latitudeDelta', (r) => ({ ...r, latitudeDelta: 1 })], + ['longitudeDelta', (r) => ({ ...r, longitudeDelta: 1 })], +]); + +const baseCamera: Camera = { + center: { latitude: 52.2297, longitude: 21.0122 }, + zoom: 12, + heading: 30, + pitch: 45, + altitude: 1000, +}; + +describeFieldCoverage('camerasEqual', baseCamera, camerasEqual, [ + ['center.latitude', (c) => ({ ...c, center: { ...c.center, latitude: 0 } })], + [ + 'center.longitude', + (c) => ({ ...c, center: { ...c.center, longitude: 0 } }), + ], + ['zoom', (c) => ({ ...c, zoom: 13 })], + ['a cleared zoom', (c) => ({ ...c, zoom: undefined })], + ['heading', (c) => ({ ...c, heading: 0 })], + ['pitch', (c) => ({ ...c, pitch: 0 })], + ['altitude', (c) => ({ ...c, altitude: 2000 })], +]); + +const basePadding: EdgePadding = { top: 1, right: 2, bottom: 3, left: 4 }; + +describeFieldCoverage('edgePaddingsEqual', basePadding, edgePaddingsEqual, [ + ['top', (p) => ({ ...p, top: 0 })], + ['right', (p) => ({ ...p, right: 0 })], + ['bottom', (p) => ({ ...p, bottom: 0 })], + ['left', (p) => ({ ...p, left: 0 })], +]); diff --git a/package/src/utils/mapValueEquality.ts b/package/src/utils/mapValueEquality.ts new file mode 100644 index 0000000..1a9e209 --- /dev/null +++ b/package/src/utils/mapValueEquality.ts @@ -0,0 +1,73 @@ +import type { Camera } from '../types/camera'; +import type { EdgePadding, Region } from '../types/region'; + +/** + * Structural comparisons for the camera-related `MapView` props. + * + * Nitro diffs view props by reference identity, and `region`, `camera` and + * `mapPadding` are usually written inline in JSX. Without these comparators an + * equal-but-new object reaches native on every render, and the Google + * providers answer a new `region` with a camera move. + */ + +export function regionsEqual( + left: Region | undefined, + right: Region | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return ( + left.latitude === right.latitude && + left.longitude === right.longitude && + left.latitudeDelta === right.latitudeDelta && + left.longitudeDelta === right.longitudeDelta + ); +} + +export function camerasEqual( + left: Camera | undefined, + right: Camera | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return ( + left.center.latitude === right.center.latitude && + left.center.longitude === right.center.longitude && + left.zoom === right.zoom && + left.heading === right.heading && + left.pitch === right.pitch && + left.altitude === right.altitude + ); +} + +export function edgePaddingsEqual( + left: EdgePadding | undefined, + right: EdgePadding | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return ( + left.top === right.top && + left.right === right.right && + left.bottom === right.bottom && + left.left === right.left + ); +}