[Bug]: Android: onCameraChanged can stop on a stale camera — an event within 10 ms of the previous one is dropped and never sent #4414

Mapbox Version

11.23.1

React Native Version

0.86.2 (New Architecture)

Platform

Android

@rnmapbox/maps version

10.3.5, with a local patch equal to #4332. The code involved is unchanged in 10.3.12 and on main (f17bcfa).

Standalone component to reproduce

import React, { useRef, useState } from 'react';
import { Button, Text, View } from 'react-native';
import { Camera, MapView } from '@rnmapbox/maps';

// The reproducer flies between these two stops every 3 s.
const A = { centerCoordinate: [-74.00597, 40.71427], zoomLevel: 12 };
const B = { centerCoordinate: [-73.98, 40.75], zoomLevel: 14 };

const sameCamera = (a, b) =>
  a != null &&
  Math.abs(a.zoom - b.zoom) < 1e-6 &&
  Math.abs(a.center[0] - b.center[0]) < 1e-9 &&
  Math.abs(a.center[1] - b.center[1]) < 1e-9;

const BugReportExample = () => {
  const camera = useRef(null);
  const timer = useRef(null);
  const target = useRef(A);
  // The camera of the last onCameraChanged, and whether one came since the last onMapIdle
  const lastChanged = useRef(null);
  const changedSinceIdle = useRef(false);
  const calls = useRef(0);
  const [stats, setStats] = useState({ idles: 0, stale: 0, calls: 0, first: '-', last: '-' });

  const onCameraChanged = (state) => {
    calls.current += 1;
    lastChanged.current = { zoom: state.properties.zoom, center: state.properties.center };
    changedSinceIdle.current = true;
  };

  // onMapIdle carries the camera the map really stopped at
  const onMapIdle = (state) => {
    if (!changedSinceIdle.current) return;
    changedSinceIdle.current = false;

    const map = { zoom: state.properties.zoom, center: state.properties.center };
    const seen = lastChanged.current;
    const text = `last onCameraChanged zoom ${seen?.zoom.toFixed(4)} | map zoom ${map.zoom.toFixed(4)}`;

    setStats((s) => ({
      idles: s.idles + 1,
      stale: s.stale + (sameCamera(seen, map) ? 0 : 1),
      calls: calls.current,
      first: s.idles === 0 ? text : s.first,
      last: text,
    }));
  };

  const toggle = () => {
    if (timer.current) {
      clearInterval(timer.current);
      timer.current = null;
      return;
    }
    timer.current = setInterval(() => {
      target.current = target.current === A ? B : A;
      camera.current?.setCamera({ ...target.current, animationDuration: 500 });
    }, 3000);
  };

  return (
    <View style={{ flex: 1 }}>
      <MapView style={{ flex: 1 }} onCameraChanged={onCameraChanged} onMapIdle={onMapIdle}>
        <Camera ref={camera} defaultSettings={A} />
      </MapView>
      <Text testID="stats">
        {`idle after a change: ${stats.idles} | onCameraChanged stale: ${stats.stale} | onCameraChanged calls: ${stats.calls}`}
      </Text>
      <Text testID="first">{`first (mount): ${stats.first}`}</Text>
      <Text testID="last">{`last: ${stats.last}`}</Text>
      <Button title="Start / Stop" onPress={toggle} />
    </View>
  );
};

export default BugReportExample;

Observed behavior and steps to reproduce

1. Open the screen on Android and wait for the map. The first (mount) line compares the last onCameraChanged with the first onMapIdle.
2. Tap Start / Stop. Every 3 s the screen calls setCamera({ centerCoordinate, zoomLevel, animationDuration: 500 }), between two stops. At each onMapIdle it checks whether the camera of the last onCameraChanged is the camera the map stopped at.
3. After about 30 moves, tap Start / Stop again and read the counters.

Result on an Android 13 emulator (arm64, 60 Hz), 30 moves:

idle after a change: 32 | onCameraChanged stale: 31 | onCameraChanged calls: 882
first (mount): last onCameraChanged zoom 0.5622 | map zoom 12.0000
last: last onCameraChanged zoom 12.0060 | map zoom 12.0000

The mount and every one of the 30 moves end with a stale onCameraChanged. The map itself is right; only the events are wrong. An app that keeps the camera from onCameraChanged (scale bar, zoom-based logic, a viewport fetch) keeps the stale value until the user moves the map again.

A temporary Log.d in AbstractEventEmitter.handleEvent (our app, same versions) shows the pattern. Mount: the map's first camera event is sent, the defaultSettings camera 1 ms later is dropped:

SEND key=topMapChange-camerachanged view=582 ts=1793036653943 sinceLast=-1 zoom=0.5545888516776374 center=[-0.0, 0.0]
DROP key=topMapChange-camerachanged view=582 ts=1793036653944 sinceLast=0 zoom=10.0 center=[-157.8581, 21.309899999999985]

An animated move: every frame has two camera updates, usually 0–2 ms apart, and the second one (here the zoom) is always dropped (30 of 56 events in one 0.5 s move):

SEND ... ts=1793036658287 sinceLast=16 zoom=10.009 center=[-157.4863921, 21.501893999999982]
DROP ... ts=1793036658288 sinceLast=0  zoom=10.032 center=[-157.4863921, 21.501893999999982]
SEND ... ts=1793036658305 sinceLast=17 zoom=10.032 center=[-157.0087325, 21.748613900000027]
DROP ... ts=1793036658306 sinceLast=0  zoom=10.073 center=[-157.0087325, 21.748613900000066]

When the last frame's second update is dropped, JS never gets the final camera.

Expected behavior

The last onCameraChanged describes the camera the map stopped at, as onMapIdle does. Merging intermediate events is fine, but the newest one should never be lost.

Notes / preliminary analysis

1. AbstractEventEmitter.handleEvent drops an event when one with the same "<key>-<type>" was sent at most 10 ms (BRIDGE_TIMEOUT_MS) earlier (shouldDropEvent). The dropped event is never sent later.
2. The key is per view manager, not per view, and it does not include the payload. So two MapViews share it, and two onImageMissing events for different images within 10 ms would lose the second image. (This point is from reading the code; the reproducer does not cover it.)
3. CameraChangeEvent.canCoalesce() is true, and Fabric can merge such events: EventQueue::enqueueEvent replaces a queued unique event of the same type for the same view. But components/AbstractEvent uses the deprecated Event(viewId) constructor, so its surfaceId is -1 (Event.init). dispatchModern then falls back to the legacy receiveEvent(viewTag, …), which React Native sends as "cannot be coalesced" (RCTModernEventEmitter). So today canCoalesce() has no effect, and the 10 ms drop is the only limit on camera events.

Possible fix (run with the reproducer above)

Create the event with the surface id and return its payload from getEventData(), so Fabric sees canCoalesce() and merges camera events itself; then remove the 10 ms drop:

--- a/android/src/main/java/com/rnmapbox/rnmbx/components/AbstractEvent.kt
+++ b/android/src/main/java/com/rnmapbox/rnmbx/components/AbstractEvent.kt
 class AbstractEvent(
+    surfaceId: Int,
     viewId: Int,
     private val mEventName: String,
     private val mCanCoalesce: Boolean,
     private val mEvent: WritableMap?
-) : Event<AbstractEvent>(viewId) {
+) : Event<AbstractEvent>(surfaceId, viewId) {
     override fun getEventName(): String {
         return mEventName
     }
 
-    override fun dispatch(rctEventEmitter: RCTEventEmitter) {
-        rctEventEmitter.receiveEvent(viewTag, eventName, mEvent)
-    }
+    override fun getEventData(): WritableMap? {
+        return mEvent
+    }
 
--- a/android/src/main/java/com/rnmapbox/rnmbx/components/AbstractEventEmitter.kt
+++ b/android/src/main/java/com/rnmapbox/rnmbx/components/AbstractEventEmitter.kt
     fun handleEvent(event: IEvent) {
-        val eventCacheKey = getEventCacheKey(event)
-
-        // fail safe to protect bridge from being spammed
-        if (shouldDropEvent(eventCacheKey, event)) {
-            return
-        }
-        mRateLimitedEvents[eventCacheKey] = System.currentTimeMillis()
-
         try {
             mEventDispatcher!!.dispatchEvent(
                 AbstractEvent(
+                    mSurfaceId,
                     event.iD,
 ...
     override fun addEventEmitters(context: ThemedReactContext, view: T) {
         mEventDispatcher = UIManagerHelper.getEventDispatcherForReactTag(context, view.id)
+        mSurfaceId = context.surfaceId
     }

(plus private var mSurfaceId = -1, and removing the now unused mRateLimitedEvents, shouldDropEvent, getEventCacheKey and BRIDGE_TIMEOUT_MS).

The same reproducer, 30 moves each, on the same emulator:

Removing the drop alone fixes the stale camera but doubles the calls into JS, because nothing merges the events. With the surface id, Fabric merges them: the calls stay at today's level, and every move ends on the right camera.

Happy to open a PR with this change.

Additional links and references

• [Bug]: Android: onCameraChanged fires with [0,0] on launch #4213 — "Android: onCameraChanged fires with [0,0] on launch". The log above shows the same first event at [0, 0], followed by a dropped start camera; it may be this bug.
• [Bug]: dense onCameraChanged traffic during gestures can overwhelm JS without a native throttle #4192 / feat: add native onCameraChanged throttling to MapView #4193 — the request for a native throttle for onCameraChanged.