From 0882b382aedf0f8a46b4b188c6f048ddca99bb20 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Fri, 25 Sep 2026 20:35:11 +0200 Subject: [PATCH 1/4] feat: report programmatic camera moves through events and promises A programmatic camera move was invisible from JS. onRegionChange and onRegionChangeComplete were gated on a user gesture, so setCamera, animateCamera, fitToCoordinates and the region / camera props emitted nothing, and the promises those methods return resolved as soon as the animation was handed to the SDK: `await animateCamera(...)` came back with the camera still at its old position. Both callbacks now fire for every move of the camera and carry a second RegionChangeDetails argument whose isGesture says who started it. One move emits one pair and nothing in between, and an update that leaves the camera where it is emits nothing - the Google Maps SDK reports a start and an idle even then, which would loop a handler that re-renders a map with an inline region prop. A gesture that interrupts the app's animation ends the app's move and starts its own. RegionChangeTracker holds these rules, in Kotlin and in Swift, with the same unit tests on both sides. Android and Google Maps on iOS also skip a region prop that matches what the map shows, as Apple already did, so echoing onRegionChangeComplete back into it is a no-op. The camera promises now settle when the camera stops: through the UIView.animate completion or regionDidChange on Apple, idleAt on Google Maps for iOS, and a real CancelableCallback on Android. A move cut short by a gesture, a later command or the view going away resolves; a call that never reached the map still rejects, as it has since #161. On MapKit a move is tracked only after it is handed over, because MapKit reports the end of the move it interrupts from inside that call. Verified with a scripted harness on an iPhone 17 simulator (Apple MapKit) and an Android 15 emulator (Google Maps): animateCamera(..., 1) resolves after ~1040 ms with the camera at its target; an animation interrupted after 500 ms by a second one resolves at ~520 ms and the second at ~1550 ms; both platforms emit the same events in all 16 scenarios, including an inline region prop re-rendered from the handler; and on Android a gesture taking over an animation emits the app's pair and then the gesture's. Google Maps on iOS is compile-checked only. Closes #136 Closes #137 --- README.md | 106 +++++++++- docs/architecture.md | 22 +- example/App.tsx | 35 ++-- .../nitro/nitromaps/CameraAnimations.kt | 60 ++++++ .../nitromaps/GoogleMapProviderAdapter.kt | 142 +++++++------ .../margelo/nitro/nitromaps/HybridMapView.kt | 4 +- .../nitro/nitromaps/MapApproximateEquality.kt | 1 + .../nitro/nitromaps/MapProviderAdapter.kt | 11 +- .../nitro/nitromaps/Region+LatLngBounds.kt | 12 ++ .../nitro/nitromaps/RegionChangeTracker.kt | 94 +++++++++ .../RegionApproximateEqualityTest.kt | 48 +++++ .../nitromaps/RegionChangeTrackerTest.kt | 188 +++++++++++++++++ package/ios/AppleMapProviderAdapter.swift | 147 ++++++++++---- package/ios/Camera+GMSCameraPosition.swift | 10 + package/ios/Camera+MKMapCamera.swift | 10 + package/ios/Camera/CameraPlacement.swift | 17 ++ package/ios/Camera/RegionChangeTracker.swift | 108 ++++++++++ package/ios/CameraMoveTracker.swift | 105 ++++++++++ package/ios/GoogleMapProviderAdapter.swift | 112 ++++++---- package/ios/HybridMapView.swift | 39 +--- package/ios/HybridMapViewDelegate.swift | 4 + package/ios/MapProviderAdapter.swift | 34 +++- package/ios/MapViewState.swift | 4 +- package/ios/Package.swift | 9 + .../Camera/RegionChangeTrackerTests.swift | 192 ++++++++++++++++++ package/src/index.ts | 1 + package/src/native/specs/MapView.nitro.ts | 31 ++- package/src/types/index.ts | 7 +- package/src/types/map.ts | 27 ++- package/src/types/ref.ts | 20 +- package/src/types/region.ts | 8 + package/type-tests/provider-props.ts | 13 ++ 32 files changed, 1380 insertions(+), 241 deletions(-) create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt create mode 100644 package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt create mode 100644 package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt create mode 100644 package/ios/Camera/CameraPlacement.swift create mode 100644 package/ios/Camera/RegionChangeTracker.swift create mode 100644 package/ios/CameraMoveTracker.swift create mode 100644 package/ios/Tests/Camera/RegionChangeTrackerTests.swift diff --git a/README.md b/README.md index 7c370095..79ef6883 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ Built with [Nitro Modules](https://nitro.margelo.com/) for high-performance nati - [Installation](#installation) - [Quick start](#quick-start) - [Map providers](#map-providers) +- [Region change events](#region-change-events) - [Native POI press events](#native-poi-press-events) - [Custom marker images](#custom-marker-images) - [GeoJSON overlays](#geojson-overlays) @@ -209,7 +210,9 @@ function MyMap() { console.log(region)} + onRegionChangeComplete={(region, details) => + console.log(region, details.isGesture) + } > (null); - const flyToWarsaw = () => { - mapRef.current?.animateCamera({ - center: { latitude: 52.2297, longitude: 21.0122 }, - zoom: 12, - }); + const flyToWarsaw = async () => { + const map = mapRef.current; + if (map == null) { + return; + } + + await map.animateCamera( + { center: { latitude: 52.2297, longitude: 21.0122 }, zoom: 12 }, + 1, + ); + + // The camera is there now, so this reads where it actually arrived. + const camera = await map.getCamera(); + console.log(camera.center); }; return ; @@ -292,6 +304,25 @@ builds wrapped in React's `` see this on every mount: React tears the effect down and sets it up again, the first call is rejected by that teardown, and the second one does the work. +#### When the camera promises settle + +`animateCamera` and `fitToCoordinates` resolve when the camera has arrived, not +when the animation is handed to the map. `setCamera` moves without animating, so +it resolves right away, as does `fitToCoordinates` with `animated: false`. + +An animation that is cut short - by a gesture, by a later camera command, or by +the map view unmounting mid-animation - resolves too rather than hanging. It does +not reject, and it does not report whether the requested position was reached: +read `getVisibleRegion()` or `getCamera()` after the `await` when that matters. +(On Apple Maps a gesture cannot cut `animateCamera` short: the map ignores touches +until the animation ends.) + +Pass `fitToCoordinates`' `animated` argument explicitly: left out, iOS animates +the fit and Android jumps to it. + +> **Changed in 1.3.0:** these promises used to resolve as soon as the animation +> started, so `await` returned with the camera still at its old position. + ## Map providers `MapView` accepts an optional `provider` prop: @@ -318,6 +349,68 @@ Changing `provider` remounts the native map view. Controlled props such as `regi Provider-specific TypeScript props are exposed through `MapViewPropsForProvider

`. For example, `showsScale` is accepted for `apple` but rejected for `google` because Google Maps SDK has no native scale control. +## Region change events + +`onRegionChange` fires when the camera starts moving and `onRegionChangeComplete` when it +stops. Both fire for every move, whoever started it: a pinch or pan, `setCamera`, +`animateCamera`, `fitToCoordinates`, or an updated `region` / `camera` prop. The second +argument says which it was. + +```tsx + { + if (details.isGesture) { + cancelAutoFollow(); + } + }} + onRegionChangeComplete={(region, details) => { + console.log( + details.isGesture ? 'user moved the map' : 'the app moved the map', + ); + }} +/> +``` + +One move emits exactly one `onRegionChange` and one `onRegionChangeComplete`, with the +same `details` on both, and nothing fires while the camera is on its way. An update that +leaves the camera where it is - a `region` or `camera` prop set to what the map already +shows, or a repeated `fitToCoordinates` - emits nothing, and neither does the map settling +into its first position as it appears. Read `getVisibleRegion()` from `onMapReady` for +that one. + +A gesture that interrupts the app's own animation ends that move and starts the user's: +`onRegionChangeComplete` with `isGesture: false` where the finger caught the camera, then +`onRegionChange` with `isGesture: true`. The opposite does not split: a camera command +issued while the map is still moving from a gesture stays part of the gesture's move. On +Apple Maps an `animateCamera` animation cannot be interrupted this way, because the map +ignores touches until it ends. + +`isGesture` means a touch gesture on the map itself. Android's own controls (the zoom +buttons and the my-location button) report as `isGesture: false`, because the Google Maps +SDK classifies them as an API animation rather than a gesture. + +### react-native-maps migration (region events) + +| react-native-maps | react-native-better-maps | +| ------------------------------------------------- | ---------------------------------------------------- | +| `onRegionChangeStart(region, details)` | `onRegionChange(region, details)` | +| `onRegionChange(region, details)`, on every frame | No equivalent - nothing fires while the camera moves | +| `onRegionChangeComplete(region, details)` | Same | +| `details.isGesture`, on Google Maps only | `details.isGesture`, on Apple and Google Maps alike | + +Note the first two rows: `onRegionChange` here fires once, when a move begins, which is +what `react-native-maps` calls `onRegionChangeStart`. + +> **Changed in 1.3.0:** before this release both callbacks fired only for user gestures, +> and took the region alone. Code that treated every event as user input should now check +> `details.isGesture`. + +Feeding `onRegionChangeComplete` back into a controlled `region` prop is safe: the map is +already showing that region, so the update moves nothing and emits nothing. Feeding back +`onRegionChange` is not. It hands the map the region the camera is leaving, and for a move +the app started, going back there cancels it. + ## Native POI press events Provider-owned points of interest are base-map features supplied by Apple Maps or Google Maps, such as restaurants, parks, schools, hotels, and stores. They are separate from app-owned `` elements and bulk `markers`; marker presses still use `Marker.onPress` and `MapView.onMarkerPress`. @@ -698,6 +791,7 @@ An optional overlay field set to `null` - the way JSON data usually says "no val | ---------------------------- | ----------------------------------------------------- | | `Coordinate` | `{ latitude, longitude }` | | `Region` | Center + span | +| `RegionChangeDetails` | `{ isGesture }` context for a region change | | `Camera` | Position, zoom, heading, pitch | | `MapType` | `'standard' \| 'satellite' \| 'hybrid' \| 'terrain'` | | `MapProvider` | `'apple' \| 'google' \| 'openstreetmap' \| 'mapbox'` | diff --git a/docs/architecture.md b/docs/architecture.md index 5aad11cc..09856d67 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -63,17 +63,17 @@ Provider adapters own SDK-specific view creation, destruction, lifecycle, camera ### Events -Map and overlay callbacks are wired through Nitro listeners on the HybridView. Callbacks receive payloads directly (e.g. `onPress(coordinate)`, `onRegionChange(region)`). - -| Callback | Payload | Notes | -| ------------------------------------------- | ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `onRegionChange` / `onRegionChangeComplete` | `Region` | iOS uses `MKCoordinateRegion` (center + span); Android derives center + deltas from visible `LatLngBounds`. Values agree without rotation/pitch but may diverge when the map is tilted or rotated. | -| `onPress` / `onLongPress` | `Coordinate` | Map background only; marker taps do not also fire map `onPress`. | -| `onPoiPress` | `PoiPressEvent` | Provider-owned base-map POIs only. Apple Maps emits category data; Google Maps emits place ID. POI taps do not also fire map `onPress`. | -| `onMapReady` | none | Fires once after the map finishes loading tiles. | -| `Marker.onPress` / `onDragEnd` | none / `Coordinate` | Dispatched by overlay `id` from native to JS registry. | -| Overlay `onPress` | none | Polyline/polygon/circle with `onPress` default to `tappable` on native. | -| `onClusterPress` | `string[]`, `Coordinate` | Fires when a marker cluster is tapped; IDs are member marker overlay ids. | +Map and overlay callbacks are wired through Nitro listeners on the HybridView. Callbacks receive payloads directly (e.g. `onPress(coordinate)`, `onRegionChange(region, details)`). + +| Callback | Payload | Notes | +| ------------------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `onRegionChange` / `onRegionChangeComplete` | `Region`, `RegionChangeDetails` | iOS uses `MKCoordinateRegion` (center + span); Android derives center + deltas from visible `LatLngBounds`. Values agree without rotation/pitch but may diverge when the map is tilted or rotated. Both fire for gestures and for programmatic moves; `details.isGesture` tells them apart. | +| `onPress` / `onLongPress` | `Coordinate` | Map background only; marker taps do not also fire map `onPress`. | +| `onPoiPress` | `PoiPressEvent` | Provider-owned base-map POIs only. Apple Maps emits category data; Google Maps emits place ID. POI taps do not also fire map `onPress`. | +| `onMapReady` | none | Fires once after the map finishes loading tiles. | +| `Marker.onPress` / `onDragEnd` | none / `Coordinate` | Dispatched by overlay `id` from native to JS registry. | +| Overlay `onPress` | none | Polyline/polygon/circle with `onPress` default to `tappable` on native. | +| `onClusterPress` | `string[]`, `Coordinate` | Fires when a marker cluster is tapped; IDs are member marker overlay ids. | ### Advanced MapView props diff --git a/example/App.tsx b/example/App.tsx index 6c3c3403..0f5312be 100644 --- a/example/App.tsx +++ b/example/App.tsx @@ -48,6 +48,7 @@ import { type OverlayEnteringAnimation, type PoiPressEvent, Region, + type RegionChangeDetails, } from 'react-native-better-maps'; import { APPLE_POI_DETAILS_DEFAULT_PRESENTATION, @@ -127,6 +128,11 @@ const springSoft = { damping: 20, stiffness: 240 }; const AnimatedPressable = Animated.createAnimatedComponent(Pressable); +function regionLabel(region: Region, details: RegionChangeDetails): string { + const source = details.isGesture ? 'gesture' : 'app'; + return `${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)} (${source})`; +} + function mergeMapPadding( padding: EdgePadding | undefined, showsScale: boolean, @@ -599,8 +605,11 @@ type MapSceneProps = { onPress: (coordinate: Coordinate) => void; onPoiPress: (event: PoiPressEvent) => void; onLongPress: (coordinate: Coordinate) => void; - onRegionChange: (region: Region) => void; - onRegionChangeComplete: (region: Region) => void; + onRegionChange: (region: Region, details: RegionChangeDetails) => void; + onRegionChangeComplete: ( + region: Region, + details: RegionChangeDetails, + ) => void; }; const MapScene = memo(function MapScene({ @@ -959,17 +968,19 @@ export default function App() { ); }, []); - const handleRegionChange = useCallback((region: Region) => { - setStatus( - `Region start · ${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)}`, - ); - }, []); + const handleRegionChange = useCallback( + (region: Region, details: RegionChangeDetails) => { + setStatus(`Region start · ${regionLabel(region, details)}`); + }, + [], + ); - const handleRegionChangeComplete = useCallback((region: Region) => { - setStatus( - `Region complete · ${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)}`, - ); - }, []); + const handleRegionChangeComplete = useCallback( + (region: Region, details: RegionChangeDetails) => { + setStatus(`Region complete · ${regionLabel(region, details)}`); + }, + [], + ); return ( diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt new file mode 100644 index 00000000..79a73be4 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt @@ -0,0 +1,60 @@ +package com.margelo.nitro.nitromaps + +import com.google.android.gms.maps.GoogleMap + +/** + * Hands out the [GoogleMap.CancelableCallback] for each camera animation, and ends + * whichever are still running when the map goes away. + * + * The SDK reports how an animation ended - `onFinish` once the camera arrives, + * `onCancel` when a gesture or a later camera update cuts it short - but never that + * the map it ran on was destroyed, and a promise waiting on that report would hang + * for good. + * + * Main thread only, like the map it serves. + */ +internal class CameraAnimations { + private val running = mutableListOf() + private var isReleased = false + + /** A callback that runs [onEnd] exactly once, however the animation ends. */ + fun callback(onEnd: () -> Unit): GoogleMap.CancelableCallback { + val animation = Animation(onEnd) + if (isReleased) { + animation.end() + } else { + running += animation + } + return animation + } + + /** Ends every animation still running, and any started from now on. */ + fun release() { + isReleased = true + val ended = running.toList() + running.clear() + for (animation in ended) { + animation.end() + } + } + + private inner class Animation( + private val onEnd: () -> Unit, + ) : GoogleMap.CancelableCallback { + private var hasEnded = false + + override fun onFinish() = end() + + override fun onCancel() = end() + + fun end() { + if (hasEnded) { + return + } + + hasEnded = true + running -= this + onEnd() + } + } +} 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 d73c4d0f..6666202b 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 @@ -19,6 +19,8 @@ import com.margelo.nitro.core.Promise private const val MAP_RELEASED_BEFORE_LAYOUT_MESSAGE = "MapView was released before it was laid out" +private const val DEFAULT_ANIMATION_DURATION_SECONDS = 0.25 + @Keep @DoNotStrip class GoogleMapProviderAdapter( @@ -27,7 +29,13 @@ class GoogleMapProviderAdapter( ) : MapProviderAdapter, LifecycleEventListener { private var googleMap: GoogleMap? = null - private var isUserGesture = false + private val regionChanges = + RegionChangeTracker( + position = { googleMap?.cameraPosition }, + region = { currentRegion() }, + onBegin = { region, details -> onRegionChange?.invoke(region, details) }, + onComplete = { region, details -> onRegionChangeComplete?.invoke(region, details) }, + ) private var hasFiredMapReady = false private val overlayController = MapOverlayController(context) private val locationSource = FusedLocationSource(context) @@ -38,6 +46,7 @@ class GoogleMapProviderAdapter( private var pendingCircles: Array? = null private val density: Float = context.resources.displayMetrics.density private val deferredMap = DeferredGoogleMap() + private val cameraAnimations = CameraAnimations() private val googleMapIdAtCreation: String? = normalizeGoogleMapId(initialGoogleMapId) @@ -107,7 +116,7 @@ class GoogleMapProviderAdapter( get() = _region set(value) { _region = value - if (value != null && !isUserGesture && _camera == null) { + if (value != null && !regionChanges.isGesture && _camera == null) { applyRegion(value) } } @@ -117,7 +126,7 @@ class GoogleMapProviderAdapter( get() = _camera set(value) { _camera = value - if (value != null && !isUserGesture) { + if (value != null && !regionChanges.isGesture) { applyCameraProp(value) } } @@ -250,8 +259,8 @@ class GoogleMapProviderAdapter( overlayController.clusterEnteringAnimation = value } - override var onRegionChange: ((region: Region) -> Unit)? = null - override var onRegionChangeComplete: ((region: Region) -> Unit)? = null + override var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? = null + override var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? = null override var onMapReady: (() -> Unit)? = null override var onPress: ((coordinate: Coordinate) -> Unit)? = null override var onPoiPress: ((event: NativePoiPressEvent) -> Unit)? = null @@ -334,9 +343,11 @@ class GoogleMapProviderAdapter( camera: Camera, duration: Double?, ): Promise { - val animationDuration = duration ?: 0.25 - return deferredMap.promise { map -> - updateMapCamera(map, camera, animated = true, durationMs = (animationDuration * 1000).toInt()) + val durationMs = ((duration ?: DEFAULT_ANIMATION_DURATION_SECONDS) * 1000).toInt() + return deferredMap.promiseCompletion { map, complete -> + updateMapCamera(map, camera, animated = true, durationMs = durationMs) { + complete(Result.success(Unit)) + } } } @@ -373,24 +384,24 @@ class GoogleMapProviderAdapter( complete(Result.failure(IllegalStateException(MAP_RELEASED_BEFORE_LAYOUT_MESSAGE))) }, ) { - complete( - runCatching { - // Inside the callback: converting the insets needs the size the map was laid out with. - val target = - bounds.expandedForEdgePadding( - padding?.toPixels(density), - _mapPadding?.toPixels(density), - view.width, - view.height, - ) ?: bounds - val update = CameraUpdateFactory.newLatLngBounds(target, 0) - if (animated == true) { - map.animateCamera(update) - } else { - map.moveCamera(update) - } - }, - ) + runCatching { + // Inside the callback: converting the insets needs the size the map was laid out with. + val target = + bounds.expandedForEdgePadding( + padding?.toPixels(density), + _mapPadding?.toPixels(density), + view.width, + view.height, + ) ?: bounds + val update = CameraUpdateFactory.newLatLngBounds(target, 0) + if (animated == true) { + // Settled by the SDK once the camera has arrived, not here. + map.animateCamera(update, cameraAnimations.callback { complete(Result.success(Unit)) }) + } else { + map.moveCamera(update) + complete(Result.success(Unit)) + } + }.onFailure { error -> complete(Result.failure(error)) } } } } @@ -441,16 +452,17 @@ class GoogleMapProviderAdapter( syncMarkerPressHandlers() map.setOnCameraMoveStartedListener { reason -> - handleRegionWillChange( - userInteracting = reason == GoogleMap.OnCameraMoveStartedListener.REASON_GESTURE, + regionChanges.moveStarted( + isGesture = reason == GoogleMap.OnCameraMoveStartedListener.REASON_GESTURE, ) } map.setOnCameraMoveListener { overlayController.onCameraMove() + regionChanges.cameraMoved() } map.setOnCameraIdleListener { overlayController.onCameraIdle() - handleRegionDidChange() + regionChanges.cameraStopped() } map.setOnMapClickListener { latLng -> onPress?.invoke(latLng.toCoordinate()) @@ -616,24 +628,35 @@ class GoogleMapProviderAdapter( val bounds = region.toLatLngBounds() val runUpdate = { - // No padding argument: Google Maps already fits bounds inside the region `setPadding` - // leaves over, so passing `mapPadding` here as well would inset the region twice. - val update = CameraUpdateFactory.newLatLngBounds(bounds, 0) - if (animated) { - map.animateCamera(update) - } else { - map.moveCamera(update) + // What the map already shows is exactly what an `onRegionChangeComplete` consumer + // hands back as the next `region` prop. Fitting it again would move nothing, yet + // still report a move - and the echo would answer that one too, for ever. + if (!currentRegion().approximatelyEquals(region)) { + // No padding argument: Google Maps already fits bounds inside the region `setPadding` + // leaves over, so passing `mapPadding` here as well would inset the region twice. + val update = CameraUpdateFactory.newLatLngBounds(bounds, 0) + if (animated) { + map.animateCamera(update) + } else { + map.moveCamera(update) + } } } runWhenMapViewLaidOut(block = runUpdate) } + /** + * Moves the camera to [camera], and calls [onEnd] once it has stopped: straight away + * after a jump, or when nothing had to move, and otherwise when the animation finishes + * or is cut short. + */ private fun updateMapCamera( map: GoogleMap, camera: Camera, animated: Boolean, durationMs: Int = 0, + onEnd: () -> Unit = {}, ) { // Every camera path ends here - the `camera` prop, its replay in `configureMap`, and // `applyCamera`/`animateCamera` - so this one check covers them all. An invalid camera is @@ -643,23 +666,30 @@ class GoogleMapProviderAdapter( // before the call is queued. if (!camera.isValid()) { Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid camera: $camera.") + onEnd() return } + // Nothing is handed to the SDK for a camera the map is already at, so no `onFinish` + // is coming either. val target = camera.toCameraPosition(map.cameraPosition) if (map.cameraPosition.approximatelyEquals(target)) { + onEnd() return } val update = CameraUpdateFactory.newCameraPosition(target) - if (animated) { - if (durationMs > 0) { - map.animateCamera(update, durationMs, null) - } else { - map.animateCamera(update) - } - } else { + if (!animated) { map.moveCamera(update) + onEnd() + return + } + + val callback = cameraAnimations.callback(onEnd) + if (durationMs > 0) { + map.animateCamera(update, durationMs, callback) + } else { + map.animateCamera(update, callback) } } @@ -694,29 +724,6 @@ class GoogleMapProviderAdapter( overlayController.setViewportSize(view.width, view.height) } - private fun handleRegionWillChange(userInteracting: Boolean) { - if (userInteracting && !isUserGesture) { - isUserGesture = true - emitRegionChange(complete = false) - } - } - - private fun handleRegionDidChange() { - if (isUserGesture) { - emitRegionChange(complete = true) - isUserGesture = false - } - } - - private fun emitRegionChange(complete: Boolean) { - val region = currentRegion() - if (complete) { - onRegionChangeComplete?.invoke(region) - } else { - onRegionChange?.invoke(region) - } - } - private fun currentRegion(): Region { val bounds = googleMap?.projection?.visibleRegion?.latLngBounds if (bounds != null) { @@ -767,6 +774,9 @@ class GoogleMapProviderAdapter( private fun destroyMapView() { deferredMap.release() deferredLayout.release() + // A call that never reached the SDK rejects above. An animation already under way + // ends here instead, like any other move cut short, and resolves. + cameraAnimations.release() if (lifecycle.isDestroyed) { return diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt index 6efb96fb..360dd758 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt @@ -185,13 +185,13 @@ class HybridMapView( adapter?.clusterEnteringAnimation = value } - override var onRegionChange: ((region: Region) -> Unit)? = null + override var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? = null set(value) { field = value adapter?.onRegionChange = value } - override var onRegionChangeComplete: ((region: Region) -> Unit)? = null + override var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? = null set(value) { field = value adapter?.onRegionChangeComplete = value 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 bd70dc75..adbca3c7 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/MapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt index 8432683a..581639ed 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt @@ -24,8 +24,8 @@ interface MapProviderAdapter { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((region: Region) -> Unit)? - var onRegionChangeComplete: ((region: Region) -> Unit)? + var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? + var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? var onMapReady: (() -> Unit)? var onPress: ((coordinate: Coordinate) -> Unit)? var onPoiPress: ((event: NativePoiPressEvent) -> Unit)? @@ -45,8 +45,14 @@ interface MapProviderAdapter { fun fetchCamera(): Promise + /** + * Resolves once the camera has arrived: a non-animated move right away, an animated + * one when it finishes or when a gesture, a later command or [release] cuts it short. + * A call made before the map exists waits for it, and rejects if [release] comes first. + */ fun applyCamera(camera: Camera): Promise + /** @see applyCamera for when the promise settles. */ fun animateCamera( camera: Camera, duration: Double?, @@ -54,6 +60,7 @@ interface MapProviderAdapter { fun getVisibleRegion(): Promise + /** @see applyCamera for when the promise settles. */ fun fitToCoordinates( coordinates: Array, padding: EdgePadding?, diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+LatLngBounds.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+LatLngBounds.kt index 471091f9..41dce789 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+LatLngBounds.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Region+LatLngBounds.kt @@ -2,6 +2,7 @@ package com.margelo.nitro.nitromaps import com.google.android.gms.maps.model.LatLng import com.google.android.gms.maps.model.LatLngBounds +import kotlin.math.abs fun Region.toLatLngBounds(): LatLngBounds { val halfLat = latitudeDelta / 2.0 @@ -12,6 +13,17 @@ fun Region.toLatLngBounds(): LatLngBounds { ) } +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 +} + fun LatLngBounds.toRegion(): Region { val center = center return Region( diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt new file mode 100644 index 00000000..f753d865 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt @@ -0,0 +1,94 @@ +package com.margelo.nitro.nitromaps + +/** + * Turns the Google Maps camera callbacks into one `onRegionChange` / + * `onRegionChangeComplete` pair per move of the camera. + * + * The SDK is noisier than that. It reports a move starting again whenever the + * reason for it changes, and it reports a start and an idle for a camera update + * that leaves the camera exactly where it was - a repeated fit, or a `region` + * prop sent again with the same values, which an inline object literal does on + * every render. So a move is announced only once the camera has left the place + * it last came to rest, and a move that never did announces nothing. Otherwise a + * handler that re-renders the map would be answered by a move that re-renders it + * again, for ever. Until the camera has come to rest once there is nothing to + * measure against - the map is still settling into its first position - so that + * is not announced either. + * + * `package/ios/Camera/RegionChangeTracker.swift` is the same class for the iOS + * adapters. Main thread only, like the map callbacks that drive it. + */ +internal class RegionChangeTracker( + private val position: () -> Position, + private val region: () -> Region, + private val onBegin: (Region, RegionChangeDetails) -> Unit, + private val onComplete: (Region, RegionChangeDetails) -> Unit, +) { + private class Move( + val details: RegionChangeDetails, + /** `null` while the map is still settling into its first position. */ + val start: Resting?, + ) { + var hasBegun = false + } + + private class Resting( + val position: Position, + val region: Region, + ) + + private var move: Move? = null + private var rest: Resting? = null + + /** True while the finger, rather than the app, is driving the camera. */ + val isGesture: Boolean + get() = move?.details?.isGesture == true + + /** The SDK says the camera is about to move: `onCameraMoveStarted`. */ + fun moveStarted(isGesture: Boolean) { + val current = move + if (current == null) { + move = Move(RegionChangeDetails(isGesture = isGesture), rest) + return + } + + // Still the same move - unless a gesture takes over from the app's own, which + // ends the app's move where the finger caught the camera and starts the user's there. + if (!isGesture || current.details.isGesture) { + return + } + + finish(current) + move = Move(RegionChangeDetails(isGesture = true), Resting(position(), region())) + } + + /** The SDK says the camera has moved on a step: `onCameraMove`. */ + fun cameraMoved() { + move?.let(::beginIfMoved) + } + + /** The SDK says the camera has come to rest: `onCameraIdle`. */ + fun cameraStopped() { + move?.let(::finish) + rest = Resting(position(), region()) + } + + private fun finish(current: Move) { + move = null + // A jump can come to rest without a single step reported in between. + beginIfMoved(current) + if (current.hasBegun) { + onComplete(region(), current.details) + } + } + + private fun beginIfMoved(current: Move) { + val start = current.start ?: return + if (current.hasBegun || position() == start.position) { + return + } + + current.hasBegun = true + onBegin(start.region, current.details) + } +} diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt new file mode 100644 index 00000000..62454f69 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt @@ -0,0 +1,48 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * `applyRegion` compares the requested region against the one the map already shows, so + * a consumer that echoes `onRegionChangeComplete` back into the `region` prop does not + * start a re-fit loop. These pin down what "already shows it" means. + */ +class RegionApproximateEqualityTest { + @Test + fun acceptsTheSameRegion() { + assertTrue(region().approximatelyEquals(region())) + } + + @Test + fun acceptsADifferenceBelowTheEpsilon() { + assertTrue(region().approximatelyEquals(region(latitude = 52.23 + 1e-9))) + assertTrue(region().approximatelyEquals(region(longitudeDelta = 0.1 + 1e-9))) + } + + @Test + fun rejectsAMovedCenter() { + assertFalse(region().approximatelyEquals(region(latitude = 52.24))) + assertFalse(region().approximatelyEquals(region(longitude = 21.02))) + } + + @Test + fun rejectsAResizedSpan() { + assertFalse(region().approximatelyEquals(region(latitudeDelta = 0.2))) + assertFalse(region().approximatelyEquals(region(longitudeDelta = 0.2))) + } + + private fun region( + latitude: Double = 52.23, + longitude: Double = 21.01, + latitudeDelta: Double = 0.1, + longitudeDelta: Double = 0.1, + ): Region = + Region( + latitude = latitude, + longitude = longitude, + latitudeDelta = latitudeDelta, + longitudeDelta = longitudeDelta, + ) +} diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt new file mode 100644 index 00000000..97bacd03 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt @@ -0,0 +1,188 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Drives [RegionChangeTracker] with the callback sequences the Google Maps SDK was + * seen to produce on an emulator, with an integer standing in for the camera position. + * `package/ios/Tests/Camera/RegionChangeTrackerTests.swift` runs the same cases. + */ +class RegionChangeTrackerTest { + private var position = 0 + private val events = mutableListOf() + + private val tracker = + RegionChangeTracker( + position = { position }, + region = { regionAt(position) }, + onBegin = { region, details -> events += event("begin", region, details) }, + onComplete = { region, details -> events += event("complete", region, details) }, + ) + + /** A map that has settled into its first position, as every case but one assumes. */ + private fun settle() { + tracker.cameraStopped() + } + + @Test + fun anAnimationEmitsOneBeginAndOneComplete() { + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @2"), events) + } + + @Test + fun aJumpWithNoMoveCallbackStillEmitsBothEvents() { + settle() + tracker.moveStarted(isGesture = false) + position = 5 + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @5"), events) + } + + @Test + fun measuresAMoveFromWhereTheCameraLastCameToRest() { + // MapKit already reports the destination when the app sets the camera. + settle() + position = 4 + tracker.moveStarted(isGesture = false) + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @4"), events) + } + + @Test + fun anUpdateThatLeavesTheCameraInPlaceEmitsNothing() { + // A repeated fit, or a `region` prop sent again with the same values. + settle() + tracker.moveStarted(isGesture = false) + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(emptyList(), events) + } + + @Test + fun theMapSettlingIntoItsFirstPositionEmitsNothing() { + tracker.moveStarted(isGesture = false) + position = 7 + tracker.cameraMoved() + tracker.cameraStopped() + assertEquals(emptyList(), events) + + // From there on, moves are measured against it. + tracker.moveStarted(isGesture = false) + position = 8 + tracker.cameraStopped() + assertEquals(listOf("begin app @7", "complete app @8"), events) + } + + @Test + fun aSecondStartForTheSameMoveIsIgnored() { + // One animation interrupting another: the SDK does not go idle in between. + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = false) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @2"), events) + } + + @Test + fun aGestureTakingOverEndsTheAppsMoveAndStartsItsOwn() { + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = true) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals( + listOf("begin app @0", "complete app @1", "begin gesture @1", "complete gesture @2"), + events, + ) + } + + @Test + fun aGestureTakingOverBeforeTheCameraMovedLeavesOnlyTheGesture() { + settle() + tracker.moveStarted(isGesture = false) + tracker.moveStarted(isGesture = true) + position = 3 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin gesture @0", "complete gesture @3"), events) + } + + @Test + fun theAppTakingOverFromAGestureStaysOneGestureMove() { + settle() + tracker.moveStarted(isGesture = true) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = false) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin gesture @0", "complete gesture @2"), events) + } + + @Test + fun reportsAGestureOnlyWhileOneIsUnderWay() { + settle() + assertFalse(tracker.isGesture) + + tracker.moveStarted(isGesture = true) + assertTrue(tracker.isGesture) + + tracker.cameraStopped() + assertFalse(tracker.isGesture) + + tracker.moveStarted(isGesture = false) + assertFalse(tracker.isGesture) + } + + @Test + fun ignoresStepsOutsideAMove() { + settle() + position = 1 + tracker.cameraMoved() + + assertEquals(emptyList(), events) + } + + private fun event( + kind: String, + region: Region, + details: RegionChangeDetails, + ): String { + val source = if (details.isGesture) "gesture" else "app" + return "$kind $source @${region.latitude.toInt()}" + } + + private fun regionAt(position: Int): Region = + Region( + latitude = position.toDouble(), + longitude = 0.0, + latitudeDelta = 1.0, + longitudeDelta = 1.0, + ) +} diff --git a/package/ios/AppleMapProviderAdapter.swift b/package/ios/AppleMapProviderAdapter.swift index ac0b1276..819904d8 100644 --- a/package/ios/AppleMapProviderAdapter.swift +++ b/package/ios/AppleMapProviderAdapter.swift @@ -3,8 +3,27 @@ import NitroModules import UIKit final class AppleMapProviderAdapter: MapProviderAdapter { + /// MapKit animates `setVisibleMapRect` over a duration it does not publish. This is + /// only the watchdog's guess at it, not a duration handed to MapKit. + private static let fitAnimationDuration: TimeInterval = 0.3 + private static let defaultAnimationDuration: TimeInterval = 0.25 + private let mapViewDelegate = HybridMapViewDelegate() - private var isUserRegionChange = false + private let cameraMoves = CameraMoveTracker() + private lazy var regionChanges = RegionChangeTracker( + position: { [unowned self] in view.camera.placement }, + region: { [unowned self] in currentRegion() }, + onBegin: { [unowned self] region, isGesture in + onRegionChange?(region, RegionChangeDetails(isGesture: isGesture)) + }, + onComplete: { [unowned self] region, isGesture in + onRegionChangeComplete?(region, RegionChangeDetails(isGesture: isGesture)) + } + ) + /// Whether MapKit is between a `regionWillChange` and its `regionDidChange`. + private var isRegionChanging = false + /// The end of a move, held back for a turn of the run loop - see `handleRegionDidChange()`. + private var pendingRegionChangeEnd: DispatchWorkItem? private var isMapReady = false private var hasDeliveredMapReady = false private var liveClusterTimer: Timer? @@ -149,8 +168,8 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } } - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? { didSet { deliverMapReadyIfPossible() @@ -208,13 +227,35 @@ final class AppleMapProviderAdapter: MapProviderAdapter { Promise.resolved(withResult: view.camera.toCamera()) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { updateMapCamera(camera, animated: false) + return Promise.resolved() } - func animateCamera(camera: Camera, duration: Double?) throws { - let animationDuration = duration ?? 0.25 - updateMapCamera(camera, animated: true, duration: animationDuration) + func animateCamera(camera: Camera, duration: Double?) throws -> Promise { + let animationDuration = duration ?? Self.defaultAnimationDuration + let promise = Promise() + let move = CameraMoveCompletion(promise: promise) + + // UIKit calls the completion when the animation runs out, and straight away when a + // later camera animation replaces it. + let didMove = updateMapCamera(camera, animated: true, duration: animationDuration) { + move.settle() + } + + // Nothing handed over - an invalid camera, or the one the map is already at - or a + // camera MapKit applied without animating, as it does for a map that is not on + // screen yet: either way there is nothing left to wait for. + guard didMove, isRegionChanging else { + move.settle() + return promise + } + + // Also settled when MapKit reports the camera at rest: a fit or a `region` update + // that cuts this animation short does not end it for UIKit, whose completion then + // waits out the full duration. + cameraMoves.track(move, duration: animationDuration) + return promise } func getVisibleRegion() throws -> Promise { @@ -225,10 +266,10 @@ final class AppleMapProviderAdapter: MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { let validCoordinates = coordinates.filter { $0.isValid } guard !validCoordinates.isEmpty else { - return + return Promise.resolved() } var mapRect = MKMapRect.null @@ -244,12 +285,22 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } let edgePadding = padding?.toUIEdgeInsets() ?? .zero - let shouldAnimate = animated ?? true - view.setVisibleMapRect( - mapRect, - edgePadding: edgePadding, - animated: shouldAnimate - ) + view.setVisibleMapRect(mapRect, edgePadding: edgePadding, animated: animated ?? true) + + // MapKit reports from inside that call: the end of any move this one cut short, the + // start of this one, and its end too when it jumps rather than animates - which it + // does for a rect far from the one on screen, `animated` or not. A rect it already + // shows it ignores without a word. Only a fit still under way has anything left to + // wait for. + guard isRegionChanging else { + return Promise.resolved() + } + + // `setVisibleMapRect` takes no completion handler: the `regionDidChange` that ends + // the move settles it. + let promise = Promise() + cameraMoves.track(promise, duration: Self.fitAnimationDuration) + return promise } func applyRegion(_ region: Region, animated: Bool = false) { @@ -270,19 +321,27 @@ final class AppleMapProviderAdapter: MapProviderAdapter { view.setRegion(targetRegion, animated: animated) } - func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double = 0) { + /// Moves the camera, and reports whether anything was handed to MapKit: nothing is for + /// an invalid camera, or for the one the map is already at. + @discardableResult + func updateMapCamera( + _ camera: Camera, + animated: Bool, + duration: Double = 0, + completion: (() -> Void)? = nil + ) -> Bool { // `setCamera` raises an Objective-C NSException - `Invalid camera // centerCoordinate` - from `-[MKMapCamera _validate]` for a center MapKit // cannot place, and Swift cannot catch that. The framing values do not // raise, but a non-finite one collapses the altitude or leaves // `view.region` reading back as `NaN`. guard camera.isValid else { - return + return false } let mapCamera = camera.toMKMapCamera() guard !view.camera.approximatelyEquals(mapCamera) else { - return + return false } if animated { @@ -290,11 +349,16 @@ final class AppleMapProviderAdapter: MapProviderAdapter { withDuration: duration, animations: { self.view.camera = mapCamera + }, + completion: { _ in + completion?() } ) } else { view.camera = mapCamera } + + return true } // Derived from MKCoordinateRegion (center + span). May differ from Android @@ -313,35 +377,40 @@ final class AppleMapProviderAdapter: MapProviderAdapter { func handleRegionWillChange(userInteracting: Bool) { startLiveClustering() - guard userInteracting, !isUserRegionChange else { - return - } - isUserRegionChange = true - emitRegionChange(complete: false) + isRegionChanging = true + pendingRegionChangeEnd?.cancel() + pendingRegionChangeEnd = nil + regionChanges.moveStarted(isGesture: userInteracting) + } + + func handleVisibleRegionChange() { + regionChanges.cameraMoved() } func handleRegionDidChange() { stopLiveClustering() + isRegionChanging = false - guard isUserRegionChange else { - return - } + // The camera has stopped, so every move still under way is over - finished, + // superseded, or cut short. + cameraMoves.settleAll() + // MapKit reports the end of each leg of a gesture, including the ones the + // finger is still driving. The move is only over once it lets go. guard !view.isUserInteracting else { return } - emitRegionChange(complete: true) - isUserRegionChange = false - } - - private func emitRegionChange(complete: Bool) { - let region = currentRegion() - if complete { - onRegionChangeComplete?(region) - } else { - onRegionChange?(region) + // A camera command that cuts an animation short makes MapKit end that move and + // start the next back to back, from inside the command. Google Maps reports the + // same thing as one move, so the end waits a turn of the run loop, and a move that + // starts in the meantime carries on the one before. + let end = DispatchWorkItem { [weak self] in + self?.pendingRegionChangeEnd = nil + self?.regionChanges.cameraStopped() } + pendingRegionChangeEnd = end + DispatchQueue.main.async(execute: end) } func startLiveClustering() { @@ -431,7 +500,11 @@ final class AppleMapProviderAdapter: MapProviderAdapter { func prepareForRecycle() { liveClusterTimer?.invalidate() liveClusterTimer = nil - isUserRegionChange = false + cameraMoves.settleAll() + pendingRegionChangeEnd?.cancel() + pendingRegionChangeEnd = nil + isRegionChanging = false + regionChanges.reset() isMapReady = false hasDeliveredMapReady = false onRegionChange = nil diff --git a/package/ios/Camera+GMSCameraPosition.swift b/package/ios/Camera+GMSCameraPosition.swift index 06f25295..f2a9d5d4 100644 --- a/package/ios/Camera+GMSCameraPosition.swift +++ b/package/ios/Camera+GMSCameraPosition.swift @@ -28,6 +28,16 @@ extension GMSCameraPosition { && abs(viewingAngle - other.viewingAngle) < angleEpsilon } + var placement: CameraPlacement { + CameraPlacement( + latitude: target.latitude, + longitude: target.longitude, + scale: Double(zoom), + heading: bearing, + pitch: viewingAngle + ) + } + func toCamera() -> Camera { Camera( center: Coordinate(latitude: target.latitude, longitude: target.longitude), diff --git a/package/ios/Camera+MKMapCamera.swift b/package/ios/Camera+MKMapCamera.swift index 78ef4fb6..013b418d 100644 --- a/package/ios/Camera+MKMapCamera.swift +++ b/package/ios/Camera+MKMapCamera.swift @@ -39,6 +39,16 @@ extension MKMapCamera { && abs(pitch - other.pitch) < angleEpsilon } + var placement: CameraPlacement { + CameraPlacement( + latitude: centerCoordinate.latitude, + longitude: centerCoordinate.longitude, + scale: centerCoordinateDistance, + heading: heading, + pitch: Double(pitch) + ) + } + func toCamera() -> Camera { let centerCoordinate = Coordinate( latitude: centerCoordinate.latitude, diff --git a/package/ios/Camera/CameraPlacement.swift b/package/ios/Camera/CameraPlacement.swift new file mode 100644 index 00000000..e02b403c --- /dev/null +++ b/package/ios/Camera/CameraPlacement.swift @@ -0,0 +1,17 @@ +import Foundation + +/// Where a map camera is, as a plain value that both SDKs' cameras reduce to. +/// +/// `MKMapCamera` and `GMSCameraPosition` are classes, and `MKMapView.camera` hands out a +/// fresh copy on every read, so two reads of a camera that has not moved are never `==` +/// as objects. Compared exactly on purpose: the question is whether the camera moved at +/// all, not whether it moved far. +struct CameraPlacement: Equatable { + let latitude: Double + let longitude: Double + /// The distance to the ground for MapKit, the zoom level for Google Maps. Only ever + /// compared with a placement from the same SDK. + let scale: Double + let heading: Double + let pitch: Double +} diff --git a/package/ios/Camera/RegionChangeTracker.swift b/package/ios/Camera/RegionChangeTracker.swift new file mode 100644 index 00000000..225694e9 --- /dev/null +++ b/package/ios/Camera/RegionChangeTracker.swift @@ -0,0 +1,108 @@ +import Foundation + +/// Turns a map SDK's camera callbacks into one `onRegionChange` / +/// `onRegionChangeComplete` pair per move of the camera. +/// +/// The SDKs are noisier than that. Google Maps reports a start and an idle for a +/// camera update that leaves the camera exactly where it was - a repeated fit, or a +/// `region` prop sent again with the same values, which an inline object literal does +/// on every render. So a move is announced only once the camera has left the place it +/// last came to rest, and a move that never did announces nothing. Otherwise a handler +/// that re-renders the map would be answered by a move that re-renders it again, for +/// ever. +/// +/// The place it last came to rest, rather than wherever the SDK says the camera is when +/// a move starts: MapKit already reports the destination by then when the app sets the +/// camera. Until the camera has come to rest once there is nothing to measure against - +/// the map is still settling into its first position - so that is not announced either. +/// +/// Generic over the position and the region so it builds without MapKit, Google Maps +/// or the Nitro-generated types. The Android adapter has the same class in Kotlin. +/// Main thread only, like the delegate callbacks that drive it. +final class RegionChangeTracker { + private struct Move { + let isGesture: Bool + /// `nil` while the map is still settling into its first position. + let start: (position: Position, region: Region)? + var hasBegun = false + } + + private let position: () -> Position + private let region: () -> Region + private let onBegin: (Region, _ isGesture: Bool) -> Void + private let onComplete: (Region, _ isGesture: Bool) -> Void + private var move: Move? + private var rest: (position: Position, region: Region)? + + init( + position: @escaping () -> Position, + region: @escaping () -> Region, + onBegin: @escaping (Region, _ isGesture: Bool) -> Void, + onComplete: @escaping (Region, _ isGesture: Bool) -> Void + ) { + self.position = position + self.region = region + self.onBegin = onBegin + self.onComplete = onComplete + } + + /// True while the finger, rather than the app, is driving the camera. + var isGesture: Bool { + move?.isGesture == true + } + + /// The SDK says the camera is about to move. + func moveStarted(isGesture: Bool) { + guard let current = move else { + move = Move(isGesture: isGesture, start: rest) + return + } + + // Still the same move - unless a gesture takes over from the app's own, which ends + // the app's move where the finger caught the camera and starts the user's there. + guard isGesture, !current.isGesture else { + return + } + + finish() + move = Move(isGesture: true, start: (position(), region())) + } + + /// The SDK says the camera has moved on a step. + func cameraMoved() { + guard let current = move, !current.hasBegun, let start = current.start, + position() != start.position + else { + return + } + + move?.hasBegun = true + onBegin(start.region, current.isGesture) + } + + /// The SDK says the camera has come to rest. + func cameraStopped() { + finish() + rest = (position(), region()) + } + + /// Forgets the move under way without reporting it, and where the camera last came to + /// rest: the map view is about to show a different map. + func reset() { + move = nil + rest = nil + } + + private func finish() { + // A jump can come to rest without a single step reported in between. + cameraMoved() + guard let current = move else { + return + } + + move = nil + if current.hasBegun { + onComplete(region(), current.isGesture) + } + } +} diff --git a/package/ios/CameraMoveTracker.swift b/package/ios/CameraMoveTracker.swift new file mode 100644 index 00000000..f20225b5 --- /dev/null +++ b/package/ios/CameraMoveTracker.swift @@ -0,0 +1,105 @@ +import Foundation +import NitroModules + +/// The Promise of one camera move, settled exactly once. +/// +/// A move ends in more ways than it begins: its animation runs out, the map reports the +/// camera has come to rest, a gesture or a later command cuts it short, or the view goes +/// away first. Whichever signal arrives first wins, and the rest are ignored - resolving +/// a Nitro Promise twice traps. +final class CameraMoveCompletion { + private let promise: Promise + fileprivate var onSettled: ((CameraMoveCompletion) -> Void)? + fileprivate(set) var isSettled = false + + init(promise: Promise) { + self.promise = promise + } + + func settle() { + guard !isSettled else { + return + } + + isSettled = true + onSettled?(self) + onSettled = nil + promise.resolve() + } +} + +/// Holds the camera moves still under way, so the one callback that says the camera has +/// come to rest settles all of them, and none outlives the adapter. +/// +/// Register a move only after handing it to the map. Both SDKs report the end of the move +/// it cuts short from inside that hand-over, and a move registered before it would be +/// settled by that report - before its own animation had even started. +/// +/// Main thread only, like the map views it serves. +final class CameraMoveTracker { + /// Grace period on top of the requested duration before a move is assumed over. The + /// map is expected to report the end of every move, and an unsettled Promise would + /// hang its `await` for the lifetime of the view if it ever did not. + private static let watchdogSlack: TimeInterval = 2 + + private var pending: [CameraMoveCompletion] = [] + private var watchdog: Timer? + + deinit { + settleAll() + } + + /// Tracks `promise` until the camera comes to rest. + func track(_ promise: Promise, duration: TimeInterval) { + track(CameraMoveCompletion(promise: promise), duration: duration) + } + + /// Tracks `move` until the camera comes to rest, or until the move settles itself. + func track(_ move: CameraMoveCompletion, duration: TimeInterval) { + guard !move.isSettled else { + return + } + + move.onSettled = { [weak self] settled in + self?.remove(settled) + } + pending.append(move) + scheduleWatchdog(after: duration + Self.watchdogSlack) + } + + /// Settles every move still under way. The camera has come to rest, so each of them + /// is over - finished, superseded, or cut short - or the view is going away. + func settleAll() { + guard !pending.isEmpty else { + return + } + + let moves = pending + pending.removeAll() + stopWatchdog() + for move in moves { + move.settle() + } + } + + private func remove(_ move: CameraMoveCompletion) { + pending.removeAll { $0 === move } + if pending.isEmpty { + stopWatchdog() + } + } + + private func scheduleWatchdog(after interval: TimeInterval) { + watchdog?.invalidate() + let timer = Timer(timeInterval: interval, repeats: false) { [weak self] _ in + self?.settleAll() + } + RunLoop.main.add(timer, forMode: .common) + watchdog = timer + } + + private func stopWatchdog() { + watchdog?.invalidate() + watchdog = nil + } +} diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index b33a9679..6bc793c1 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -9,10 +9,22 @@ import UIKit final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { private static let liveGestureRefreshInterval: CFTimeInterval = 0.18 private static let liveGestureAnimationBudget = 24 + private static let defaultAnimationDuration: TimeInterval = 0.25 + + private let cameraMoves = CameraMoveTracker() + private lazy var regionChanges = RegionChangeTracker( + position: { [unowned self] in view.camera.placement }, + region: { [unowned self] in view.currentNitroRegion() }, + onBegin: { [unowned self] region, isGesture in + onRegionChange?(region, RegionChangeDetails(isGesture: isGesture)) + }, + onComplete: { [unowned self] region, isGesture in + onRegionChangeComplete?(region, RegionChangeDetails(isGesture: isGesture)) + } + ) private var isMapReady = false private var hasDeliveredMapReady = false - private var isUserRegionChange = false private var isUserGestureMoving = false private var lastLiveMarkerRefreshTime: CFTimeInterval = 0 private var myLocationObservation: NSKeyValueObservation? @@ -176,8 +188,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } } - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? { didSet { deliverMapReadyIfPossible() @@ -234,12 +246,18 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { Promise.resolved(withResult: view.camera.toCamera()) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { updateMapCamera(camera, animated: false) + return Promise.resolved() } - func animateCamera(camera: Camera, duration: Double?) throws { - updateMapCamera(camera, animated: true, duration: duration ?? 0.25) + func animateCamera(camera: Camera, duration: Double?) throws -> Promise { + let animationDuration = duration ?? Self.defaultAnimationDuration + guard updateMapCamera(camera, animated: true, duration: animationDuration) else { + return Promise.resolved() + } + + return trackCameraMove(duration: animationDuration) } func getVisibleRegion() throws -> Promise { @@ -250,10 +268,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { let validCoordinates = coordinates.filter { $0.isValid } guard !validCoordinates.isEmpty else { - return + return Promise.resolved() } var bounds = GMSCoordinateBounds() @@ -262,11 +280,19 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } let edgePadding = padding?.toUIEdgeInsets() ?? .zero let update = GMSCameraUpdate.fit(bounds, with: edgePadding) - applyCameraUpdate(update, animated: animated ?? true, duration: nil) + let shouldAnimate = animated ?? true + applyCameraUpdate(update, animated: shouldAnimate, duration: nil) + + guard shouldAnimate else { + return Promise.resolved() + } + + return trackCameraMove(duration: Self.defaultAnimationDuration) } func prepareForRecycle() { - isUserRegionChange = false + cameraMoves.settleAll() + regionChanges.reset() isUserGestureMoving = false lastLiveMarkerRefreshTime = 0 isMapReady = false @@ -314,6 +340,16 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { return } + // What the map already shows is exactly what an `onRegionChangeComplete` consumer + // hands back as the next `region` prop. Fitting it again moves the camera - with + // `mapPadding`, a little further out each round - and the echo answers every move. + guard + !view.currentNitroRegion().toMKCoordinateRegion() + .approximatelyEquals(region.toMKCoordinateRegion()) + else { + return + } + applyCameraUpdate( GMSCameraUpdate.fit(region.toGMSCoordinateBounds(), with: mapPadding?.toUIEdgeInsets() ?? .zero), animated: animated, @@ -321,18 +357,35 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { ) } - private func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double? = nil) { + /// Moves the camera, and reports whether anything was handed to the Google SDK: + /// nothing is for an invalid camera, or for the one the map is already at. + @discardableResult + private func updateMapCamera( + _ camera: Camera, + animated: Bool, + duration: Double? = nil + ) -> Bool { guard camera.isValid else { - return + return false } let target = camera.toGMSCameraPosition(current: view.camera) guard !view.camera.approximatelyEquals(target) else { - return + return false } let update = GMSCameraUpdate.setCamera(target) applyCameraUpdate(update, animated: animated, duration: duration) + return true + } + + /// `GMSMapView.animate(with:)` takes no completion handler, so an animated move is + /// settled by `mapView(_:idleAt:)`. Called after the hand-over, as + /// `CameraMoveTracker` requires. + private func trackCameraMove(duration: TimeInterval) -> Promise { + let promise = Promise() + cameraMoves.track(promise, duration: duration) + return promise } private func applyCameraUpdate( @@ -354,31 +407,6 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } } - private func handleRegionWillChange(userInteracting: Bool) { - guard userInteracting, !isUserRegionChange else { - return - } - isUserRegionChange = true - emitRegionChange(complete: false) - } - - private func handleRegionDidChange() { - guard isUserRegionChange else { - return - } - emitRegionChange(complete: true) - isUserRegionChange = false - } - - private func emitRegionChange(complete: Bool) { - let region = view.currentNitroRegion() - if complete { - onRegionChangeComplete?(region) - } else { - onRegionChange?(region) - } - } - private func refreshVisibleMarkers() { overlayController.scheduleViewportRefresh(immediate: true) } @@ -521,7 +549,7 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { extension GoogleMapProviderAdapter: GMSMapViewDelegate { func mapView(_ mapView: GMSMapView, willMove gesture: Bool) { - handleRegionWillChange(userInteracting: gesture) + regionChanges.moveStarted(isGesture: gesture) if gesture { startGestureMarkerRefresh() } @@ -529,12 +557,16 @@ extension GoogleMapProviderAdapter: GMSMapViewDelegate { func mapView(_ mapView: GMSMapView, didChange position: GMSCameraPosition) { refreshGestureMarkersIfNeeded() + regionChanges.cameraMoved() } func mapView(_ mapView: GMSMapView, idleAt position: GMSCameraPosition) { refreshVisibleMarkers() stopGestureMarkerRefresh() - handleRegionDidChange() + // The camera has stopped, so every move still under way is over - finished, + // superseded, or cut short. + cameraMoves.settleAll() + regionChanges.cameraStopped() notifyMapReadyIfNeeded() } diff --git a/package/ios/HybridMapView.swift b/package/ios/HybridMapView.swift index 198e686b..e407c2c5 100644 --- a/package/ios/HybridMapView.swift +++ b/package/ios/HybridMapView.swift @@ -171,12 +171,12 @@ final class HybridMapView: HybridMapViewSpec { } } - var onRegionChange: ((Region) -> Void)? { + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? { get { getBacked(\.onRegionChange) } set { setBackedOnMain(newValue, store: \.onRegionChange) { $0.onRegionChange = $1 } } } - var onRegionChangeComplete: ((Region) -> Void)? { + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? { get { getBacked(\.onRegionChangeComplete) } set { setBackedOnMain(newValue, store: \.onRegionChangeComplete) { @@ -262,11 +262,11 @@ final class HybridMapView: HybridMapViewSpec { } func applyCamera(camera: Camera) throws -> Promise { - promiseOnMainVoid { try $0.applyCamera(camera: camera) } + promiseOnMain { try $0.applyCamera(camera: camera) } } func animateCamera(camera: Camera, duration: Double?) throws -> Promise { - promiseOnMainVoid { + promiseOnMain { try $0.animateCamera(camera: camera, duration: duration) } } @@ -280,7 +280,7 @@ final class HybridMapView: HybridMapViewSpec { padding: EdgePadding?, animated: Bool? ) throws -> Promise { - promiseOnMainVoid { + promiseOnMain { try $0.fitToCoordinates( coordinates: coordinates, padding: padding, @@ -453,35 +453,6 @@ final class HybridMapView: HybridMapViewSpec { } } - private func promiseOnMainVoid( - _ work: @escaping (MapProviderAdapter) throws -> Void - ) -> Promise { - let promise = Promise() - let lifecycle = currentLifecycleSnapshot() - let run = { [weak self] in - guard let self else { - promise.reject(withError: Self.mapViewNotMountedError()) - return - } - - do { - let adapter = try self.currentAdapter(matching: lifecycle) - try work(adapter) - promise.resolve(withResult: ()) - } catch { - promise.reject(withError: error) - } - } - - if Thread.isMainThread { - run() - } else { - DispatchQueue.main.async(execute: run) - } - - return promise - } - private func promiseOnMain( _ work: @escaping (MapProviderAdapter) throws -> Promise ) -> Promise { diff --git a/package/ios/HybridMapViewDelegate.swift b/package/ios/HybridMapViewDelegate.swift index bca7bf49..7e68e233 100644 --- a/package/ios/HybridMapViewDelegate.swift +++ b/package/ios/HybridMapViewDelegate.swift @@ -87,6 +87,10 @@ final class HybridMapViewDelegate: NSObject, MKMapViewDelegate, UIGestureRecogni parent?.handleRegionDidChange() } + func mapViewDidChangeVisibleRegion(_ mapView: MKMapView) { + parent?.handleVisibleRegionChange() + } + func mapViewDidFinishLoadingMap(_ mapView: MKMapView) { parent?.notifyMapReadyIfNeeded() } diff --git a/package/ios/MapProviderAdapter.swift b/package/ios/MapProviderAdapter.swift index 8ab3d6cb..d186456e 100644 --- a/package/ios/MapProviderAdapter.swift +++ b/package/ios/MapProviderAdapter.swift @@ -23,8 +23,8 @@ protocol MapProviderAdapter: AnyObject { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? { get set } var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? { get set } - var onRegionChange: ((Region) -> Void)? { get set } - var onRegionChangeComplete: ((Region) -> Void)? { get set } + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? { get set } + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? { get set } var onMapReady: (() -> Void)? { get set } var onPress: ((Coordinate) -> Void)? { get set } var onPoiPress: ((NativePoiPressEvent) -> Void)? { get set } @@ -43,10 +43,24 @@ protocol MapProviderAdapter: AnyObject { var onClusterPress: (([String], Coordinate) -> Void)? { get set } func fetchCamera() throws -> Promise - func applyCamera(camera: Camera) throws - func animateCamera(camera: Camera, duration: Double?) throws + + /// Resolves once the camera has arrived: a non-animated move right away, an animated + /// one when it finishes or when a gesture, a later command or ``prepareForRecycle()`` + /// cuts it short. + func applyCamera(camera: Camera) throws -> Promise + + /// - SeeAlso: ``applyCamera(camera:)`` for when the Promise settles. + func animateCamera(camera: Camera, duration: Double?) throws -> Promise + func getVisibleRegion() throws -> Promise - func fitToCoordinates(coordinates: [Coordinate], padding: EdgePadding?, animated: Bool?) throws + + /// - SeeAlso: ``applyCamera(camera:)`` for when the Promise settles. + func fitToCoordinates( + coordinates: [Coordinate], + padding: EdgePadding?, + animated: Bool? + ) throws -> Promise + func prepareForRecycle() } @@ -73,8 +87,8 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? var onPress: ((Coordinate) -> Void)? var onPoiPress: ((NativePoiPressEvent) -> Void)? @@ -120,11 +134,11 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { Promise.rejected(withError: error) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { throw error } - func animateCamera(camera: Camera, duration: Double?) throws { + func animateCamera(camera: Camera, duration: Double?) throws -> Promise { throw error } @@ -136,7 +150,7 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { throw error } diff --git a/package/ios/MapViewState.swift b/package/ios/MapViewState.swift index 5e28f3b4..919cfbf4 100644 --- a/package/ios/MapViewState.swift +++ b/package/ios/MapViewState.swift @@ -20,8 +20,8 @@ struct MapViewState { var mapPadding: EdgePadding? var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? var onPress: ((Coordinate) -> Void)? var onPoiPress: ((NativePoiPressEvent) -> Void)? diff --git a/package/ios/Package.swift b/package/ios/Package.swift index 3aaea218..ee5f321b 100644 --- a/package/ios/Package.swift +++ b/package/ios/Package.swift @@ -8,6 +8,10 @@ let package = Package( name: "NitroMapsSupport", platforms: [.macOS(.v13)], targets: [ + .target( + name: "NitroMapsCamera", + path: "Camera" + ), .target( name: "NitroMapsColorParser", path: "ColorParser" @@ -20,6 +24,11 @@ let package = Package( name: "NitroMapsClusterBadge", path: "ClusterBadge" ), + .testTarget( + name: "NitroMapsCameraTests", + dependencies: ["NitroMapsCamera"], + path: "Tests/Camera" + ), .testTarget( name: "NitroMapsColorParserTests", dependencies: ["NitroMapsColorParser"], diff --git a/package/ios/Tests/Camera/RegionChangeTrackerTests.swift b/package/ios/Tests/Camera/RegionChangeTrackerTests.swift new file mode 100644 index 00000000..0ff8175e --- /dev/null +++ b/package/ios/Tests/Camera/RegionChangeTrackerTests.swift @@ -0,0 +1,192 @@ +import Testing + +@testable import NitroMapsCamera + +/// Drives `RegionChangeTracker` with the callback sequences MapKit and Google Maps were +/// seen to produce, with an integer standing in for the camera position and for the +/// region derived from it. The Kotlin `RegionChangeTrackerTest` runs the same cases. +@MainActor +private final class Harness { + var position = 0 + var events: [String] = [] + + lazy var tracker = RegionChangeTracker( + position: { [unowned self] in position }, + region: { [unowned self] in position }, + onBegin: { [unowned self] region, isGesture in + events.append("begin \(isGesture ? "gesture" : "app") @\(region)") + }, + onComplete: { [unowned self] region, isGesture in + events.append("complete \(isGesture ? "gesture" : "app") @\(region)") + } + ) + + /// A map that has settled into its first position, as every case but one assumes. + static func settled() -> Harness { + let harness = Harness() + harness.tracker.cameraStopped() + return harness + } +} + +@Test @MainActor +func anAnimationEmitsOneBeginAndOneComplete() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @2"]) +} + +@Test @MainActor +func aJumpWithNoStepReportedStillEmitsBothEvents() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 5 + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @5"]) +} + +@Test @MainActor +func measuresAMoveFromWhereTheCameraLastCameToRest() { + // MapKit already reports the destination when the app sets the camera. + let harness = Harness.settled() + harness.position = 4 + harness.tracker.moveStarted(isGesture: false) + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @4"]) +} + +@Test @MainActor +func anUpdateThatLeavesTheCameraInPlaceEmitsNothing() { + // A repeated fit, or a `region` prop sent again with the same values. + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events.isEmpty) +} + +@Test @MainActor +func theMapSettlingIntoItsFirstPositionEmitsNothing() { + let harness = Harness() + harness.tracker.moveStarted(isGesture: false) + harness.position = 7 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + #expect(harness.events.isEmpty) + + // From there on, moves are measured against it. + harness.tracker.moveStarted(isGesture: false) + harness.position = 8 + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @7", "complete app @8"]) +} + +@Test @MainActor +func aSecondStartForTheSameMoveIsIgnored() { + // One animation interrupting another, reported as one continuous move. + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @2"]) +} + +@Test @MainActor +func aGestureTakingOverEndsTheAppsMoveAndStartsItsOwn() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: true) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect( + harness.events == [ + "begin app @0", "complete app @1", "begin gesture @1", "complete gesture @2", + ] + ) +} + +@Test @MainActor +func aGestureTakingOverBeforeTheCameraMovedLeavesOnlyTheGesture() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.tracker.moveStarted(isGesture: true) + harness.position = 3 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin gesture @0", "complete gesture @3"]) +} + +@Test @MainActor +func theAppTakingOverFromAGestureStaysOneGestureMove() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: true) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin gesture @0", "complete gesture @2"]) +} + +@Test @MainActor +func reportsAGestureOnlyWhileOneIsUnderWay() { + let harness = Harness.settled() + #expect(!harness.tracker.isGesture) + + harness.tracker.moveStarted(isGesture: true) + #expect(harness.tracker.isGesture) + + harness.tracker.cameraStopped() + #expect(!harness.tracker.isGesture) + + harness.tracker.moveStarted(isGesture: false) + #expect(!harness.tracker.isGesture) +} + +@Test @MainActor +func ignoresStepsOutsideAMove() { + let harness = Harness.settled() + harness.position = 1 + harness.tracker.cameraMoved() + + #expect(harness.events.isEmpty) +} + +@Test @MainActor +func resetForgetsTheMoveAndWhereTheCameraRested() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.reset() + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @0"]) + + // A new map settles into its first position without a word. + harness.tracker.reset() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @0"]) +} diff --git a/package/src/index.ts b/package/src/index.ts index b7dadb13..f96f26b9 100644 --- a/package/src/index.ts +++ b/package/src/index.ts @@ -11,6 +11,7 @@ export { geojsonToOverlayDescriptors } from './geojson/geojsonToDescriptors'; export type { Coordinate, Region, + RegionChangeDetails, Camera, EdgePadding, VisibleRegion, diff --git a/package/src/native/specs/MapView.nitro.ts b/package/src/native/specs/MapView.nitro.ts index 90b80ee4..50bc1486 100644 --- a/package/src/native/specs/MapView.nitro.ts +++ b/package/src/native/specs/MapView.nitro.ts @@ -6,7 +6,12 @@ import type { import type { Camera } from '../../types/camera'; import type { Coordinate } from '../../types/coordinate'; import type { MapProvider, MapType } from '../../types/map'; -import type { EdgePadding, Region, VisibleRegion } from '../../types/region'; +import type { + EdgePadding, + Region, + RegionChangeDetails, + VisibleRegion, +} from '../../types/region'; import type { CircleDescriptor, MarkerDescriptor, @@ -180,11 +185,14 @@ export interface MapViewProps extends HybridViewProps { /** Entering animation for marker clusters. */ clusterEnteringAnimation?: OverlayEnteringAnimationDescriptor; - /** Called once when a user-initiated region change begins. */ - onRegionChange?: (region: Region) => void; + /** Called once when a region change begins, whoever started it. */ + onRegionChange?: (region: Region, details: RegionChangeDetails) => void; - /** Called once when a user-initiated region change ends. */ - onRegionChangeComplete?: (region: Region) => void; + /** Called once when a region change ends, whoever started it. */ + onRegionChangeComplete?: ( + region: Region, + details: RegionChangeDetails, + ) => void; /** Called when the map is ready to use. */ onMapReady?: () => void; @@ -245,20 +253,27 @@ export interface MapViewMethods extends HybridViewMethods { fetchCamera(): Promise; /** - * Sets the camera position immediately. + * Sets the camera position immediately. Resolves once the move is done, which + * for a non-animated move is right away. * * Named `applyCamera` in the Nitro spec to avoid colliding with the `camera` * prop accessor (`getCamera`/`setCamera`) in generated C++ bindings. */ applyCamera(camera: Camera): Promise; - /** Animates the camera to the given position. */ + /** + * Animates the camera to the given position. Resolves once the camera has + * arrived, or once the animation is cut short. + */ animateCamera(camera: Camera, duration?: number): Promise; /** Returns the currently visible geographic region. */ getVisibleRegion(): Promise; - /** Fits the camera to show all given coordinates with optional edge padding. */ + /** + * Fits the camera to show all given coordinates with optional edge padding. + * Resolves once the camera has arrived, or once the animation is cut short. + */ fitToCoordinates( coordinates: Coordinate[], padding?: EdgePadding, diff --git a/package/src/types/index.ts b/package/src/types/index.ts index 5a7e0bb5..032d85d7 100644 --- a/package/src/types/index.ts +++ b/package/src/types/index.ts @@ -1,6 +1,11 @@ export type { Coordinate } from './coordinate'; export type { Camera } from './camera'; -export type { Region, EdgePadding, VisibleRegion } from './region'; +export type { + Region, + EdgePadding, + RegionChangeDetails, + VisibleRegion, +} from './region'; export type { ApplePoiCategory, ApplePoiDetailPresentation, diff --git a/package/src/types/map.ts b/package/src/types/map.ts index d9e4f8ca..257ca83d 100644 --- a/package/src/types/map.ts +++ b/package/src/types/map.ts @@ -12,7 +12,7 @@ import type { ApplePoiDetailPresentation, } from '../native/specs/MapView.nitro'; import type { MarkerDescriptor, OverlayEnteringAnimation } from './overlays'; -import type { EdgePadding, Region } from './region'; +import type { EdgePadding, Region, RegionChangeDetails } from './region'; /** * Available map display styles. @@ -111,11 +111,28 @@ interface BaseMapViewProps { /** Called when any circle is pressed. */ onCirclePress?: (id: string) => void; - /** Called once when a user-initiated region change begins. */ - onRegionChange?: (region: Region) => void; + /** + * Called once when the camera starts to move, with the region it is leaving - + * not on every frame while it moves. + * + * Fires for gestures and for programmatic moves alike - `setCamera`, + * `animateCamera`, `fitToCoordinates` and the `region` / `camera` props. + * Use `details.isGesture` to tell the two apart. An update that leaves the + * camera where it is fires nothing, and neither does the map settling into + * its first position as it appears. + */ + onRegionChange?: (region: Region, details: RegionChangeDetails) => void; - /** Called once when a user-initiated region change ends. */ - onRegionChangeComplete?: (region: Region) => void; + /** + * Called once when the camera comes to rest, with the region it arrived at + * and the same `details` as the `onRegionChange` that began the move. + * + * @see {@linkcode BaseMapViewProps.onRegionChange} + */ + onRegionChangeComplete?: ( + region: Region, + details: RegionChangeDetails, + ) => void; /** Called when the map is ready to use. */ onMapReady?: () => void; diff --git a/package/src/types/ref.ts b/package/src/types/ref.ts index 42e419e4..d073623a 100644 --- a/package/src/types/ref.ts +++ b/package/src/types/ref.ts @@ -13,6 +13,13 @@ import type { EdgePadding, VisibleRegion } from './region'; * A call that is still waiting when the map view unmounts rejects, as does any * call made afterwards. Nothing silently does nothing. * + * The camera methods resolve when the move is over, never before. An animated + * move resolves when the camera arrives, or as soon as something cuts it short: + * a gesture, a later camera command, or the map view unmounting mid-animation. + * It resolves rather than rejects in those cases, and does not say whether the + * requested position was reached - read {@linkcode MapViewRef.getCamera} or + * {@linkcode MapViewRef.getVisibleRegion} after the `await` when that matters. + * * @example * ```tsx * const mapRef = useRef(null); @@ -29,7 +36,8 @@ export interface MapViewRef { getCamera(): Promise; /** - * Sets the camera position immediately. + * Sets the camera position immediately. Resolves once the camera is there, + * which for this un-animated move is right away. * * Rejects straight away, and leaves the map where it is, for a camera the * map cannot use: a center outside the world, or a zoom, heading, pitch or @@ -39,9 +47,10 @@ export interface MapViewRef { setCamera(camera: Camera): Promise; /** - * Animates the camera to the given position. Resolves once the animation has - * been handed to the native map, not when it finishes, and rejects for a - * camera the map cannot use, as {@linkcode MapViewRef.setCamera} does. + * Animates the camera to the given position. Resolves when the animation + * ends - after roughly `duration`, or earlier if something cuts it short - + * and rejects for a camera the map cannot use, as + * {@linkcode MapViewRef.setCamera} does. * * @param duration Animation duration in seconds. Defaults to `0.25`. */ @@ -52,7 +61,8 @@ export interface MapViewRef { /** * Fits the camera to show all given coordinates with optional edge padding. - * An empty {@linkcode coordinates} list is a no-op. + * Resolves once the camera has arrived, which for an un-animated fit is + * right away. An empty {@linkcode coordinates} list is a no-op. * * @param animated Pass it explicitly: omitted, iOS animates and Android jumps. */ diff --git a/package/src/types/region.ts b/package/src/types/region.ts index 445965f2..88214b99 100644 --- a/package/src/types/region.ts +++ b/package/src/types/region.ts @@ -29,3 +29,11 @@ export interface VisibleRegion { farLeft: Coordinate; farRight: Coordinate; } + +/** + * Context delivered alongside a region change. + */ +export interface RegionChangeDetails { + /** Whether the change was started by a user gesture rather than by the app. */ + isGesture: boolean; +} diff --git a/package/type-tests/provider-props.ts b/package/type-tests/provider-props.ts index 843a4ac0..e2443b22 100644 --- a/package/type-tests/provider-props.ts +++ b/package/type-tests/provider-props.ts @@ -2,6 +2,7 @@ import type { ApplePoiCategory, MapViewProps, MapViewPropsForProvider, + Region, } from '../src'; export const appleProps: MapViewPropsForProvider<'apple'> = { @@ -57,6 +58,18 @@ export const defaultProviderGoogleMapIdProps: MapViewProps = { googleMapId: 'google-map-id', }; +export const regionChangeProps: MapViewProps = { + onRegionChange: (region, details) => { + region satisfies Region; + details.isGesture satisfies boolean; + }, + onRegionChangeComplete: (region, details) => { + region satisfies Region; + // @ts-expect-error The details carry the gesture flag, not the region. + details.latitude satisfies number; + }, +}; + export const plannedProviderProps: MapViewProps = { provider: 'mapbox', }; From 75e0ba055228ed72dfbe8d9baf279b24eb5b56f2 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Fri, 25 Sep 2026 20:42:45 +0200 Subject: [PATCH 2/4] refactor: drop dead guards and restated comments from the camera trackers CameraAnimations.callback and CameraMoveTracker.track guarded against a released tracker and an already-settled move, neither of which any caller can produce. Also drops comments that repeated the KDoc or the settleAll doc, and restates the applyRegion echo check without the claims that no longer held once RegionChangeTracker suppressed no-op moves. --- .../com/margelo/nitro/nitromaps/CameraAnimations.kt | 10 ++-------- .../nitro/nitromaps/GoogleMapProviderAdapter.kt | 7 ++----- package/ios/AppleMapProviderAdapter.swift | 3 --- package/ios/CameraMoveTracker.swift | 6 +----- package/ios/GoogleMapProviderAdapter.swift | 6 ++---- 5 files changed, 7 insertions(+), 25 deletions(-) diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt index 79a73be4..ed271a7d 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt @@ -15,22 +15,16 @@ import com.google.android.gms.maps.GoogleMap */ internal class CameraAnimations { private val running = mutableListOf() - private var isReleased = false /** A callback that runs [onEnd] exactly once, however the animation ends. */ fun callback(onEnd: () -> Unit): GoogleMap.CancelableCallback { val animation = Animation(onEnd) - if (isReleased) { - animation.end() - } else { - running += animation - } + running += animation return animation } - /** Ends every animation still running, and any started from now on. */ + /** Ends every animation still running. */ fun release() { - isReleased = true val ended = running.toList() running.clear() for (animation in ended) { 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 6666202b..bd91504e 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 @@ -395,7 +395,6 @@ class GoogleMapProviderAdapter( ) ?: bounds val update = CameraUpdateFactory.newLatLngBounds(target, 0) if (animated == true) { - // Settled by the SDK once the camera has arrived, not here. map.animateCamera(update, cameraAnimations.callback { complete(Result.success(Unit)) }) } else { map.moveCamera(update) @@ -629,8 +628,8 @@ class GoogleMapProviderAdapter( val runUpdate = { // What the map already shows is exactly what an `onRegionChangeComplete` consumer - // hands back as the next `region` prop. Fitting it again would move nothing, yet - // still report a move - and the echo would answer that one too, for ever. + // hands back as the next `region` prop. Fitting it again must not move the camera, + // or the echo would answer every move with another one. if (!currentRegion().approximatelyEquals(region)) { // No padding argument: Google Maps already fits bounds inside the region `setPadding` // leaves over, so passing `mapPadding` here as well would inset the region twice. @@ -670,8 +669,6 @@ class GoogleMapProviderAdapter( return } - // Nothing is handed to the SDK for a camera the map is already at, so no `onFinish` - // is coming either. val target = camera.toCameraPosition(map.cameraPosition) if (map.cameraPosition.approximatelyEquals(target)) { onEnd() diff --git a/package/ios/AppleMapProviderAdapter.swift b/package/ios/AppleMapProviderAdapter.swift index 819904d8..2fe5bbcc 100644 --- a/package/ios/AppleMapProviderAdapter.swift +++ b/package/ios/AppleMapProviderAdapter.swift @@ -390,9 +390,6 @@ final class AppleMapProviderAdapter: MapProviderAdapter { func handleRegionDidChange() { stopLiveClustering() isRegionChanging = false - - // The camera has stopped, so every move still under way is over - finished, - // superseded, or cut short. cameraMoves.settleAll() // MapKit reports the end of each leg of a gesture, including the ones the diff --git a/package/ios/CameraMoveTracker.swift b/package/ios/CameraMoveTracker.swift index f20225b5..2bcd941f 100644 --- a/package/ios/CameraMoveTracker.swift +++ b/package/ios/CameraMoveTracker.swift @@ -10,7 +10,7 @@ import NitroModules final class CameraMoveCompletion { private let promise: Promise fileprivate var onSettled: ((CameraMoveCompletion) -> Void)? - fileprivate(set) var isSettled = false + private var isSettled = false init(promise: Promise) { self.promise = promise @@ -56,10 +56,6 @@ final class CameraMoveTracker { /// Tracks `move` until the camera comes to rest, or until the move settles itself. func track(_ move: CameraMoveCompletion, duration: TimeInterval) { - guard !move.isSettled else { - return - } - move.onSettled = { [weak self] settled in self?.remove(settled) } diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index 6bc793c1..bcfd4624 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -341,8 +341,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } // What the map already shows is exactly what an `onRegionChangeComplete` consumer - // hands back as the next `region` prop. Fitting it again moves the camera - with - // `mapPadding`, a little further out each round - and the echo answers every move. + // hands back as the next `region` prop. Fitting it again must not move the camera, + // or the echo would answer every move with another one. guard !view.currentNitroRegion().toMKCoordinateRegion() .approximatelyEquals(region.toMKCoordinateRegion()) @@ -563,8 +563,6 @@ extension GoogleMapProviderAdapter: GMSMapViewDelegate { func mapView(_ mapView: GMSMapView, idleAt position: GMSCameraPosition) { refreshVisibleMarkers() stopGestureMarkerRefresh() - // The camera has stopped, so every move still under way is over - finished, - // superseded, or cut short. cameraMoves.settleAll() regionChanges.cameraStopped() notifyMapReadyIfNeeded() From 5d2be216658e87d45117d1d3a8cebd03224e8927 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Mon, 28 Sep 2026 17:11:49 +0200 Subject: [PATCH 3/4] docs: list region change events in the capability matrix --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 60b80553..f0776487 100644 --- a/README.md +++ b/README.md @@ -900,6 +900,7 @@ An optional overlay field set to `null` - the way JSON data usually says "no val | Visible region | Supported | Supported | Supported | | Fit to coordinates | Supported | Supported | Supported | | Screen point conversion | Supported | Supported | Supported | +| Region change events | Supported, with `isGesture` | Supported, with `isGesture` | Supported, with `isGesture` | | Map types | Standard, satellite, hybrid; terrain falls back to standard | Standard, satellite, hybrid, terrain | Standard, satellite, hybrid, terrain | | Gestures | Supported | Supported | Supported | | User location | Supported; host app owns permission prompt | Supported; host app owns permission prompt | Supported; host app owns permission prompt | From 1d8b384029a518ed75647f1e309374a1b0d74bf9 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Mon, 28 Sep 2026 17:27:32 +0200 Subject: [PATCH 4/4] docs: handle a rejected ref call in the imperative camera example getCamera rejects when the map view unmounted during the animation, and animateCamera does when it unmounted before the call ran, so the example catches both as the README asks. --- README.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index f0776487..3d8dcb68 100644 --- a/README.md +++ b/README.md @@ -252,14 +252,19 @@ function ControlledMap() { return; } - await map.animateCamera( - { center: { latitude: 52.2297, longitude: 21.0122 }, zoom: 12 }, - 1000, - ); - - // The camera is there now, so this reads where it actually arrived. - const camera = await map.getCamera(); - console.log(camera.center); + try { + await map.animateCamera( + { center: { latitude: 52.2297, longitude: 21.0122 }, zoom: 12 }, + 1000, + ); + + // The camera is there now, so this reads where it actually arrived. + const camera = await map.getCamera(); + console.log(camera.center); + } catch (error) { + // The map view unmounted, so there is no camera left to move or read. + console.warn(error); + } }; return ;