From 9e634c6bb037f037c5c14b3d2a5e8cac76b187ce Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Wed, 23 Sep 2026 17:57:24 +0200 Subject: [PATCH 1/2] fix: skip bulk markers whose coordinate cannot be placed Descriptors passed through the bulk `markers` prop were forwarded verbatim, and with clustering or above 500 markers they reach the background viewport pipeline, where a NaN coordinate took the app down on both platforms. On iOS `Int(_:)` traps on NaN and infinity while the spatial index and the cluster grid are built. On Android the NaN lands in a cluster whose `LatLngBounds` throws "southern latitude exceeds northern latitude (NaN > NaN)" on the compute thread. `normalizeMarkerDescriptors` now drops a descriptor whose coordinate cannot be placed, with the development warning a `` child already gets. `hybridRef` reaches the native setter directly, so `MarkerRenderPipeline.setMarkers` on iOS and `MarkerRenderState.setMarkers` on Android filter the same way where the dataset enters the pipeline - which covers the synchronous path as well. Android reports each skip to logcat. The README no longer lists bulk `markers` as a gap, and names the one that remains: bulk `polylines` / `polygons` / `circles` are checked only natively on Android. Closes #171 --- README.md | 6 +- .../nitro/nitromaps/MapOverlayController.kt | 5 +- .../nitro/nitromaps/MarkerClusterEngine.kt | 3 + .../nitro/nitromaps/MarkerRenderState.kt | 17 +++- .../nitromaps/OverlayDescriptor+Validity.kt | 3 + .../nitromaps/MarkerDescriptorFixture.kt | 3 +- .../nitro/nitromaps/MarkerRenderStateTest.kt | 52 ++++++++++++ .../nitromaps/MarkerViewportPipelineTest.kt | 67 ++++++++++++++++ .../OverlayDescriptorValidityTest.kt | 8 ++ package/ios/MarkerClusterEngine.swift | 14 +++- package/ios/MarkerSpatialIndex.swift | 3 + .../normalizeMarkerDescriptors.test.ts | 79 ++++++++++++++++++- .../overlays/normalizeMarkerDescriptors.ts | 21 ++++- 13 files changed, 269 insertions(+), 12 deletions(-) create mode 100644 package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerViewportPipelineTest.kt diff --git a/README.md b/README.md index 4562e4d7..4a7e3ca1 100644 --- a/README.md +++ b/README.md @@ -576,11 +576,11 @@ A coordinate that arrives as `NaN` or out of range is dropped instead of being f - An invalid `region` is ignored, and the map keeps the region it already had. - An invalid `camera` is ignored the same way, and a pitch past the range the SDKs draw is pulled back to it rather than rejected. - An overlay whose coordinates, ring length or radius cannot be drawn is skipped; its neighbours still render. -- Anything supplied through `region`, `camera`, or a `` / `` / `` / `` child is reported through `console.warn` in development. +- Anything supplied through `region`, `camera`, the bulk `markers` prop, or a `` / `` / `` / `` child is reported through `console.warn` in development. -Where the check runs depends on the entry point. `region`, `camera` and `fitToCoordinates` are guarded natively on both platforms, so a `hybridRef` call - `setCamera` and `animateCamera` included - cannot reach the SDKs either. Overlay descriptors are additionally filtered natively on Android, where an undrawable overlay throws inside the Fabric mount transaction and would otherwise take the whole screen down; those skips are reported to logcat rather than `console.warn`. +Where the check runs depends on the entry point. `region`, `camera`, `fitToCoordinates` and marker descriptors are guarded natively on both platforms, so a `hybridRef` call - `setCamera` and `animateCamera` included - cannot reach the SDKs either. Polyline, polygon and circle descriptors are additionally filtered natively on Android, where an undrawable overlay throws inside the Fabric mount transaction and would otherwise take the whole screen down. Native skips are reported to logcat on Android rather than through `console.warn`. -One gap is worth knowing about: descriptors passed through the bulk `markers` prop are checked on neither side - only the `` child is. +One gap is worth knowing about: descriptors passed through the bulk `polylines` / `polygons` / `circles` props are checked only natively on Android - on iOS they reach MapKit and the Google Maps SDK unchecked, and neither platform warns about them in development. Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, camera `zoom` / `heading` / `pitch` / `altitude` finite when supplied (with `zoom` and `heading` also small enough for the 32-bit float the SDKs keep them in), two coordinates for a polyline, three per polygon ring, and a finite radius of at least 0 for a circle. A region whose span would run past a pole is pulled back to what the map can show rather than rejected. 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 59259fef..ed7b2730 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 @@ -33,7 +33,10 @@ internal class MapOverlayController( private val polygons = LinkedHashMap() private val circles = LinkedHashMap() private val markerEnterAnimators = HashMap() - private val renderState = MarkerRenderState() + private val renderState = + MarkerRenderState { descriptor -> + Log.w(NITRO_MAPS_LOG_TAG, "Skipped marker \"${descriptor.id}\": it cannot be drawn.") + } private var onMarkerPress: ((String) -> Unit)? = null private var onClusterPress: ((List, Coordinate) -> Unit)? = null private var spatialIndex: MarkerSpatialIndex? = null diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt index 0bf5d452..ff1eb273 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt @@ -49,6 +49,9 @@ internal sealed interface ClusterElement { * Pure function over descriptor data (no map projection), so it is safe to call * from a background thread. Output is bounded by the number of grid cells that * fit on screen, keeping per-frame Google Maps work small and constant. + * + * Every coordinate must be placeable: a `NaN` one joins a cluster whose + * `LatLngBounds` throws. [MarkerRenderState] drops those on the way in. */ internal object MarkerClusterEngine { private const val CELL_DP = 64.0 diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerRenderState.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerRenderState.kt index 0042146a..441b854e 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerRenderState.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerRenderState.kt @@ -10,8 +10,14 @@ package com.margelo.nitro.nitromaps * * [setMarkers], [setClusteringEnabled] and [attachMap] return whether the caller * has to redraw the markers. + * + * Markers whose coordinate cannot be placed are dropped on the way in and reported + * to [onSkippedMarker], so neither the synchronous path nor the viewport pipeline + * ever sees one. JS drops them too, but `hybridRef` reaches [setMarkers] directly. */ -internal class MarkerRenderState { +internal class MarkerRenderState( + private val onSkippedMarker: (MarkerDescriptor) -> Unit = {}, +) { var descriptors: Array = emptyArray() private set @@ -33,12 +39,13 @@ internal class MarkerRenderState { fun setMarkers(next: Array?): Boolean { val nextDescriptors = next ?: emptyArray() + // Fingerprinted as delivered, so redelivering drawn markers skips the filter and its reports. val nextFingerprint = nextDescriptors.markersFingerprint() if (nextFingerprint == fingerprint && isDrawn) { return false } - descriptors = nextDescriptors + descriptors = placeable(nextDescriptors) fingerprint = nextFingerprint if (!isMapAttached) { // Nothing reached the map, so nothing may be remembered as drawn. @@ -81,6 +88,12 @@ internal class MarkerRenderState { isDrawn = false } + private fun placeable(delivered: Array): Array { + val (placeable, skipped) = delivered.partition { it.isValid() } + skipped.forEach(onSkippedMarker) + return placeable.toTypedArray() + } + 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/OverlayDescriptor+Validity.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/OverlayDescriptor+Validity.kt index f1757907..ceadd0f1 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/OverlayDescriptor+Validity.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/OverlayDescriptor+Validity.kt @@ -1,5 +1,8 @@ package com.margelo.nitro.nitromaps +/** A marker is placed by its coordinate alone; clustering one that cannot be placed throws in `LatLngBounds`. */ +internal fun MarkerDescriptor.isValid(): Boolean = coordinate.isValid() + /** A polyline needs two placeable points before `GoogleMap.addPolyline` accepts it. */ internal fun PolylineDescriptor.isValid(): Boolean = coordinates.isValidPath(MINIMUM_POLYLINE_SIZE) diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerDescriptorFixture.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerDescriptorFixture.kt index 63c8019c..f1545aa1 100644 --- a/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerDescriptorFixture.kt +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerDescriptorFixture.kt @@ -2,6 +2,7 @@ package com.margelo.nitro.nitromaps internal fun marker( id: String = "marker-1", + coordinate: Coordinate = Coordinate(37.77, -122.41), image: MarkerImage? = null, markerColor: String? = null, anchor: MarkerAnchor? = null, @@ -17,7 +18,7 @@ internal fun marker( // shifts every positional argument after it. return MarkerDescriptor( id = id, - coordinate = Coordinate(37.77, -122.41), + coordinate = coordinate, title = "Title", subtitle = "Subtitle", draggable = false, diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerRenderStateTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerRenderStateTest.kt index b2eecca2..5b05a724 100644 --- a/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerRenderStateTest.kt +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerRenderStateTest.kt @@ -150,4 +150,56 @@ class MarkerRenderStateTest { assertTrue(state.usesViewportPipeline) } + + @Test + fun `markers that cannot be placed are dropped and reported`() { + val skipped = ArrayList() + val state = MarkerRenderState { skipped.add(it.id) } + state.attachMap() + + val redraw = + state.setMarkers( + arrayOf( + marker(id = "first"), + marker(id = "nan", coordinate = Coordinate(Double.NaN, Double.NaN)), + marker(id = "infinite", coordinate = Coordinate(Double.POSITIVE_INFINITY, 0.0)), + marker(id = "past-the-pole", coordinate = Coordinate(90.0001, 0.0)), + marker(id = "last"), + ), + ) + + assertTrue(redraw) + assertEquals(listOf("first", "last"), state.descriptors.map { it.id }) + assertEquals(listOf("nan", "infinite", "past-the-pole"), skipped) + } + + @Test + fun `redelivering markers that cannot be placed does not report them again`() { + val skipped = ArrayList() + val state = MarkerRenderState { skipped.add(it.id) } + val markers = arrayOf(marker(id = "a"), marker(id = "nan", coordinate = Coordinate(Double.NaN, 0.0))) + state.attachMap() + state.setMarkers(markers) + + assertFalse(state.setMarkers(markers)) + assertEquals(listOf("nan"), skipped) + } + + @Test + fun `markers that cannot be placed leave nothing to draw on an attaching map`() { + val state = MarkerRenderState() + state.setMarkers(arrayOf(marker(id = "nan", coordinate = Coordinate(Double.NaN, Double.NaN)))) + + assertFalse(state.attachMap()) + assertTrue(state.descriptors.isEmpty()) + } + + @Test + fun `only markers that can be placed count towards the viewport pipeline`() { + val state = MarkerRenderState() + + state.setMarkers(Array(500) { marker(id = "m$it") } + marker(id = "nan", coordinate = Coordinate(Double.NaN, 0.0))) + + assertFalse(state.usesViewportPipeline) + } } diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerViewportPipelineTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerViewportPipelineTest.kt new file mode 100644 index 00000000..d591671c --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/MarkerViewportPipelineTest.kt @@ -0,0 +1,67 @@ +package com.margelo.nitro.nitromaps + +import com.google.android.gms.maps.model.LatLng +import com.google.android.gms.maps.model.LatLngBounds +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * [MarkerRenderState], [MarkerSpatialIndex] and [MarkerClusterEngine] in the order + * [MapOverlayController] runs them with clustering on; `LatLngBounds` is plain Java, so no map + * is needed. + * + * The viewport is 2 degrees square at 1080 x 1920 px and density 2.75, which makes the grid cells + * 0.25 degrees on a side: everything up to 0.25 degrees north-east of (0, 0) shares one. + */ +class MarkerViewportPipelineTest { + private val viewport = LatLngBounds(LatLng(-1.0, -1.0), LatLng(1.0, 1.0)) + private val unplaceable = Coordinate(latitude = Double.NaN, longitude = Double.NaN) + + @Test + fun `markers that share a grid cell form one cluster`() { + val cluster = clusters(marker(id = "a", coordinate = point(0.01)), marker(id = "b", coordinate = point(0.02))) + + assertEquals(listOf("a", "b"), (cluster.single() as ClusterElement.Cluster).memberIds) + } + + @Test + fun `markers that cannot be placed never reach a cluster`() { + // `floor(NaN).toInt()` is 0, so unfiltered they share the (0, 0) cell with the real markers + // and turn the cluster's bounds into NaN, which `LatLngBounds` throws on. + val cluster = + clusters( + marker(id = "a", coordinate = point(0.01)), + marker(id = "bad-1", coordinate = unplaceable), + marker(id = "b", coordinate = point(0.02)), + marker(id = "bad-2", coordinate = unplaceable), + ) + + assertEquals(listOf("a", "b"), (cluster.single() as ClusterElement.Cluster).memberIds) + } + + @Test + fun `a dataset with nothing that can be placed clusters to nothing`() { + val elements = clusters(marker(id = "bad-1", coordinate = unplaceable), marker(id = "bad-2", coordinate = unplaceable)) + + assertTrue(elements.isEmpty()) + } + + private fun clusters(vararg markers: MarkerDescriptor): List { + val state = MarkerRenderState() + state.attachMap() + state.setClusteringEnabled(true) + state.setMarkers(arrayOf(*markers)) + + val candidates = MarkerSpatialIndex(state.descriptors).candidates(viewport) + return MarkerClusterEngine.clusters(candidates, viewport, VIEW_WIDTH_PX, VIEW_HEIGHT_PX, DENSITY) + } + + private fun point(degrees: Double): Coordinate = Coordinate(latitude = degrees, longitude = degrees) + + private companion object { + const val VIEW_WIDTH_PX = 1080 + const val VIEW_HEIGHT_PX = 1920 + const val DENSITY = 2.75f + } +} diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/OverlayDescriptorValidityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/OverlayDescriptorValidityTest.kt index 6192749e..7dc802d5 100644 --- a/package/android/src/test/java/com/margelo/nitro/nitromaps/OverlayDescriptorValidityTest.kt +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/OverlayDescriptorValidityTest.kt @@ -5,6 +5,14 @@ import org.junit.Assert.assertTrue import org.junit.Test class OverlayDescriptorValidityTest { + @Test + fun requiresAPlaceableCoordinateForAMarker() { + assertTrue(marker(coordinate = point()).isValid()) + assertFalse(marker(coordinate = point(latitude = Double.NaN)).isValid()) + assertFalse(marker(coordinate = point(longitude = Double.NEGATIVE_INFINITY)).isValid()) + assertFalse(marker(coordinate = point(latitude = 90.0001)).isValid()) + } + @Test fun requiresTwoPlaceablePointsForAPolyline() { assertFalse(polyline().isValid()) diff --git a/package/ios/MarkerClusterEngine.swift b/package/ios/MarkerClusterEngine.swift index 4dc7287b..e830d0f1 100644 --- a/package/ios/MarkerClusterEngine.swift +++ b/package/ios/MarkerClusterEngine.swift @@ -5,6 +5,9 @@ import MapKit /// Runs entirely off descriptor data (no `MKMapView` projection), so it is safe /// to call from a background queue. Output is bounded by the number of grid /// cells that fit on screen, keeping per-frame MapKit work small and constant. +/// +/// Every coordinate must be placeable: the grid cell comes from `Int(_:)`, which +/// traps on NaN and infinity. `MarkerRenderPipeline.setMarkers` drops those. enum MarkerClusterEngine { /// A single display element: an individual marker or a cluster badge. enum Element { @@ -407,11 +410,20 @@ final class MarkerRenderPipeline { } markersFingerprint = fingerprint - allMarkerDescriptors = next + allMarkerDescriptors = Self.placeable(next) spatialIndex = nil return true } + /// JS drops these already, but `hybridRef` reaches the native setter directly. + private static func placeable(_ descriptors: [MarkerDescriptor]) -> [MarkerDescriptor] { + // Filtering copies every C++-backed descriptor, so skip it when nothing is dropped. + guard !descriptors.allSatisfy({ $0.coordinate.isValid }) else { + return descriptors + } + return descriptors.filter { $0.coordinate.isValid } + } + func reapply( displayedVersions: [String: Int], region: MKCoordinateRegion, diff --git a/package/ios/MarkerSpatialIndex.swift b/package/ios/MarkerSpatialIndex.swift index b25f3f65..392f42e9 100644 --- a/package/ios/MarkerSpatialIndex.swift +++ b/package/ios/MarkerSpatialIndex.swift @@ -5,6 +5,9 @@ import MapKit /// Built once per dataset so viewport queries cost O(cells in view + markers in /// those cells) instead of O(all markers). Immutable after init, so instances /// are safe to query from a background queue. +/// +/// Every coordinate must be placeable: cells come from `Int(_:)`, which traps on +/// NaN and infinity. `MarkerRenderPipeline.setMarkers` drops those. final class MarkerSpatialIndex { let count: Int private let cellsPerSide: Int diff --git a/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts b/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts index 62bf6bc1..1befb304 100644 --- a/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts +++ b/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts @@ -1,6 +1,27 @@ -import { beforeEach, describe, expect, mock, test } from 'bun:test'; +import { + afterEach, + beforeEach, + describe, + expect, + mock, + spyOn, + test, +} from 'bun:test'; import type { MarkerDescriptor } from '../../types/overlays'; +const warnSpy = spyOn(console, 'warn'); +const previousDev = (globalThis as { __DEV__?: boolean }).__DEV__; + +function restoreDevFlag(): void { + const globalDev = globalThis as { __DEV__?: boolean }; + if (previousDev === undefined) { + delete globalDev.__DEV__; + return; + } + + globalDev.__DEV__ = previousDev; +} + const resolveAssetSourceMock = mock( ( source: @@ -36,10 +57,66 @@ const baseDescriptor = { zIndex: 3, } satisfies MarkerDescriptor; +function withCoordinate( + id: string, + latitude: number, + longitude: number, +): MarkerDescriptor { + return { ...baseDescriptor, id, coordinate: { latitude, longitude } }; +} + describe('normalizeMarkerDescriptors', () => { beforeEach(() => { resolveAssetSourceMock.mockClear(); clearResolvedMarkerImageCacheForTests(); + warnSpy.mockClear(); + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + }); + + afterEach(() => { + warnSpy.mockClear(); + restoreDevFlag(); + }); + + test('skips a descriptor whose coordinate cannot be placed and keeps its neighbours in order', () => { + const normalized = normalizeMarkerDescriptors([ + withCoordinate('first', 52.23, 21.01), + withCoordinate('nan', Number.NaN, 21.01), + withCoordinate('infinite', 52.23, Number.POSITIVE_INFINITY), + withCoordinate('out-of-range', 1000, 0), + withCoordinate('last', 50.06, 19.94), + ]); + + expect(normalized.map((descriptor) => descriptor.id)).toEqual([ + 'first', + 'last', + ]); + }); + + test('warns once per skipped descriptor, naming its id', () => { + normalizeMarkerDescriptors([ + withCoordinate('bad-1', Number.NaN, Number.NaN), + withCoordinate('good', 52.23, 21.01), + withCoordinate('bad-2', Number.POSITIVE_INFINITY, 0), + ]); + + expect(warnSpy).toHaveBeenCalledTimes(2); + expect(warnSpy.mock.calls[0]?.[0]).toContain( + 'marker "bad-1" skipped: invalid coordinate', + ); + expect(warnSpy.mock.calls[1]?.[0]).toContain( + 'marker "bad-2" skipped: invalid coordinate', + ); + }); + + test('skips a descriptor that has no coordinate at all', () => { + const normalized = normalizeMarkerDescriptors([ + { ...baseDescriptor, id: 'undefined', coordinate: undefined as never }, + { ...baseDescriptor, id: 'null', coordinate: null as never }, + ]); + + expect(normalized).toEqual([]); + expect(warnSpy).toHaveBeenCalledTimes(2); }); test('carries a descriptor without an image across unchanged', () => { diff --git a/package/src/overlays/normalizeMarkerDescriptors.ts b/package/src/overlays/normalizeMarkerDescriptors.ts index bffd57dd..d5bb5101 100644 --- a/package/src/overlays/normalizeMarkerDescriptors.ts +++ b/package/src/overlays/normalizeMarkerDescriptors.ts @@ -2,10 +2,15 @@ import type { MarkerDescriptor as PublicMarkerDescriptor } from '../types/overla import type { MarkerDescriptor } from '../native/specs/overlays'; import { buildMarkerDescriptor } from './collectMarkerOverlay'; import { resolveMarkerImage } from './resolveMarkerImage'; +import { warnOverlay } from './warnOverlay'; +import { isValidCoordinate } from '../utils/validateGeometry'; /** * Widens the public marker descriptors into the shape the native view expects: * `require()` image sources are resolved and `null` optional fields dropped. + * Descriptors whose coordinate cannot be placed are skipped with the development + * warning a `` child gets. Native filters them too, for `hybridRef`, but + * only JS can warn. * * Reference identity of the result does not matter here: `MapView` stabilizes * the array structurally before it reaches the native prop. @@ -13,7 +18,17 @@ import { resolveMarkerImage } from './resolveMarkerImage'; export function normalizeMarkerDescriptors( descriptors: PublicMarkerDescriptor[], ): MarkerDescriptor[] { - return descriptors.map((descriptor) => - buildMarkerDescriptor(descriptor.id, descriptor, resolveMarkerImage), - ); + const normalized: MarkerDescriptor[] = []; + for (const descriptor of descriptors) { + if (!isValidCoordinate(descriptor.coordinate)) { + warnOverlay(`marker "${descriptor.id}" skipped: invalid coordinate`); + continue; + } + + normalized.push( + buildMarkerDescriptor(descriptor.id, descriptor, resolveMarkerImage), + ); + } + + return normalized; } From 88ceb4f9cfa5f545d33ba5157c3839d025a6dc93 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Wed, 23 Sep 2026 17:57:33 +0200 Subject: [PATCH 2/2] fix(android): log a failed marker computation instead of crashing An exception that escaped a task on `computeExecutor` reached the worker thread's uncaught exception handler and killed the app - which is how the NaN marker of #171 became a crash rather than a missed refresh. Both tasks, the index build and the viewport refresh, now run through `executeCompute`, which logs the failure and leaves the markers on screen as they are. `clear()` shuts the executor down with `shutdownNow()`, so index builds and refreshes still queued when the map goes away are dropped instead of being computed only for the generation check to throw the result away. --- .../nitro/nitromaps/MapOverlayController.kt | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) 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 ed7b2730..dfc64b26 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 @@ -115,7 +115,8 @@ internal class MapOverlayController( renderState.reset() spatialIndex = null refreshGeneration += 1 - computeExecutor.shutdown() + // Queued work would only be discarded by the generation check, so drop it. + computeExecutor.shutdownNow() computeExecutor = Executors.newSingleThreadExecutor() } @@ -159,7 +160,7 @@ internal class MapOverlayController( refreshGeneration += 1 val generation = refreshGeneration - computeExecutor.execute { + executeCompute { val candidates = index.candidates(bounds) val elements: List = if (clustering) { @@ -186,7 +187,7 @@ internal class MapOverlayController( refreshGeneration += 1 val generation = refreshGeneration - computeExecutor.execute { + executeCompute { val index = MarkerSpatialIndex(descriptors) mainHandler.post { if (generation != refreshGeneration) { @@ -198,6 +199,20 @@ internal class MapOverlayController( } } + /** + * An exception escaping a [computeExecutor] task would reach the thread's uncaught exception + * handler and kill the app; a failed refresh just leaves the markers on screen as they are. + */ + private fun executeCompute(task: () -> Unit) { + computeExecutor.execute { + try { + task() + } catch (error: Exception) { + Log.e(NITRO_MAPS_LOG_TAG, "Marker computation failed; the markers on screen were left as they are.", error) + } + } + } + private fun applyDiff( diff: MarkerRenderDiff, animateEntering: Boolean = true,