[aderiushev]

Description

Fixes #4414

On Android, AbstractEventEmitter.handleEvent dropped an event when one with the same "<key>-<type>" had been sent at most 10 ms earlier, and it never sent the dropped one later. During an animated camera move the map makes two camera updates per frame, so the second one was always dropped, and when the last frame's update was dropped, onCameraChanged (on MapView and on CameraGestureObserver) stopped on a stale camera.

The drop was there to protect the bridge, but canCoalesce() never reached React Native: components/AbstractEvent dispatches through the legacy dispatch(), which Fabric sends as an event that cannot be coalesced.

This PR:

• overrides dispatchModern in components/AbstractEvent, so Fabric gets canCoalesce() and merges queued camera changes of a view. The surface id stays -1, so Fabric finds the view by its tag, as it does today.
• removes the 10 ms drop.

Events that can't be coalesced (MapChangeEvent: idle, style loaded, region, frame; image missing; Android callbacks; map steady) are never merged, and now they are not dropped either. Fabric merges a new event only into the newest queued event of the same view, and only when that one is coalescable and has the same name (EventQueue.cpp), so camera events can't swallow the other topMapChange events. The counts below show onMapIdle and onDidFinishRenderingFrameFully arriving as before. iOS is not changed.

Other events keep canCoalesce() == true: inherited from events/AbstractEvent (press, feature press, point annotation select/drag, user tracking mode), or set by LocationEvent. Fabric can now merge two of the same kind that are queued for one view in the same frame, keeping the newer one; before, the second one within 10 ms was dropped. If presses should never merge, they can override canCoalesce() like AndroidCallbackEvent does — I left that out to keep this change small.

Things I tried first, for the review:

• Building the event with an explicit surface id (Event(surfaceId, viewTag) + getEventData()): UIManagerHelper.getSurfaceId(view) is -1 for the map view, because its context is the activity (getMapViewContext). The events then took the legacy path again (no merging), and every event logged a does not have SurfaceId soft exception. A surface id stored per manager would send events of a map on another surface to the wrong surface.
• Giving camera events their own event name (topCameraChanged, as on iOS): not needed. With this change, camera events merge although they share topMapChange with idle and frame events (same call counts as on main).

Checklist

• I've read CONTRIBUTING.md
• I updated the doc/other generated code with running yarn generate in the root folder (no changes)
• I have tested the new feature on /example app.
  • In V11 mode/ios
  • In New Architecture mode/ios
  • In V11 mode/android
  • In New Architecture mode/android
• I added/updated a sample - if a new feature was implemented (/example)

Also: ./gradlew :rnmapbox_maps:testDebugUnitTest passes (14 tests).

Screenshot OR Video

The reproducer below in the example app (main at b42e5ed, React Native 0.84.1, New Architecture, Mapbox 11.33.0), Android 13 emulator (arm64). Each build: a fresh start, then Slow x30 (30 moves, one every 3 s), then Fast x10 (10 bursts of 15 back-to-back moves 200 ms apart, each move interrupting the last, then 3 s to go idle).

	main	this PR
Slow: last MapView.onCameraChanged ≠ the camera in onMapIdle	30 of 31 idles	0 of 31
Slow: last CameraGestureObserver.onCameraChanged ≠ the camera in onMapIdle	30 of 31	0 of 31
Slow: onMapIdle / onDidFinishRenderingFrameFully / MapView.onCameraChanged calls	31 / 889 / 846	31 / 941 / 844
Fast: stale MapView / CameraGestureObserver onCameraChanged at the idle after a burst	10 of 10 / 10 of 10	0 / 0
Fast: onMapIdle / onDidFinishRenderingFrameFully / MapView.onCameraChanged calls	10 / 1161 / 1941	10 / 901 / 1856

An example of a stale idle on main: map 12.0000 | onCameraChanged 12.0060 | observer 12.0060. The frame counts depend on how fast the emulator renders (software GL on a busy host); they show the frame events still arrive.

The map's start camera from #4414 did not reproduce in the example app on main (the first onCameraChanged already had the start camera); in our app on 10.3.5 it did. The per-frame drop reproduces on main every time.

Component to reproduce the issue you're fixing

BugReportExample.js — Slow x30 / Fast x10, counts stale onCameraChanged at each onMapIdle

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

// The camera moves between these two stops.
const A = { centerCoordinate: [-74.00597, 40.71427], zoomLevel: 12 };
const B = { centerCoordinate: [-73.98, 40.75], zoomLevel: 14 };

const cameraOf = (state) => ({ zoom: state.properties.zoom, center: state.properties.center });

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 MapView.onCameraChanged and CameraGestureObserver.onCameraChanged
  const lastMap = useRef(null);
  const lastObserver = useRef(null);
  const changedSinceIdle = useRef(false);
  const counts = useRef({ moves: 0, cameraCalls: 0, observerCalls: 0, frames: 0, idles: 0 });
  const [stats, setStats] = useState({ checked: 0, staleMap: 0, staleObserver: 0, first: '-', last: '-' });
  const [shown, setShown] = useState(counts.current);

  const onCameraChanged = (state) => {
    counts.current.cameraCalls += 1;
    lastMap.current = cameraOf(state);
    changedSinceIdle.current = true;
  };

  const onObserverCameraChanged = (event) => {
    counts.current.observerCalls += 1;
    lastObserver.current = cameraOf(event.nativeEvent);
  };

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

    const map = cameraOf(state);
    const text =
      `map ${map.zoom.toFixed(4)} | onCameraChanged ${lastMap.current?.zoom.toFixed(4)}` +
      ` | observer ${lastObserver.current?.zoom.toFixed(4)}`;

    setStats((s) => ({
      checked: s.checked + 1,
      staleMap: s.staleMap + (sameCamera(lastMap.current, map) ? 0 : 1),
      staleObserver: s.staleObserver + (sameCamera(lastObserver.current, map) ? 0 : 1),
      first: s.checked === 0 ? text : s.first,
      last: text,
    }));
  };

  const move = () => {
    counts.current.moves += 1;
    target.current = target.current === A ? B : A;
    camera.current?.setCamera({ ...target.current, animationDuration: 500 });
  };

  const stop = () => {
    clearInterval(timer.current);
    timer.current = null;
  };

  // 30 moves, one every 3 s
  const runSlow = () => {
    stop();
    let left = 30;
    timer.current = setInterval(() => {
      move();
      left -= 1;
      if (left === 0) stop();
    }, 3000);
  };

  // 10 bursts of 15 back-to-back moves (each interrupts the last), then 3 s for the map to go idle
  const runFast = () => {
    stop();
    let tick = 0;
    timer.current = setInterval(() => {
      if (tick % 30 < 15) move();
      tick += 1;
      if (tick === 300) stop();
    }, 200);
  };

  return (
    <View style={{ flex: 1 }}>
      <MapView
        style={{ flex: 1 }}
        onCameraChanged={onCameraChanged}
        onMapIdle={onMapIdle}
        onDidFinishRenderingFrameFully={() => {
          counts.current.frames += 1;
        }}
      >
        <Camera ref={camera} defaultSettings={A} />
        <CameraGestureObserver onCameraChanged={onObserverCameraChanged} />
      </MapView>
      <Text testID="stale">
        {`idle checks: ${stats.checked} | stale MapView.onCameraChanged: ${stats.staleMap} | stale CameraGestureObserver.onCameraChanged: ${stats.staleObserver}`}
      </Text>
      <Text testID="counts">
        {`moves: ${shown.moves} | onMapIdle: ${shown.idles} | onDidFinishRenderingFrameFully: ${shown.frames} | MapView.onCameraChanged: ${shown.cameraCalls} | observer onCameraChanged: ${shown.observerCalls}`}
      </Text>
      <Text testID="first">{`first idle: ${stats.first}`}</Text>
      <Text testID="last">{`last idle: ${stats.last}`}</Text>
      <View style={{ flexDirection: 'row', justifyContent: 'space-around' }}>
        <Button title="Slow x30" onPress={runSlow} />
        <Button title="Fast x10" onPress={runFast} />
      </View>
    </View>
  );
};

export default BugReportExample;

🤖 Generated with Claude Code

[mfazekas]

Thanks, this is a good find. I could reproduce the stale onCameraChanged on main (25 of 33 idles in the slow run, 10 of 13 in the fast one), and with this PR it's 0 in both, at about the same event volume. Removing the drop also fixes lost onImageMissing events (only 1–3 of 20 arrived on main) and one map's events dropping another map's. The drop dates back to 2017 and was only a precaution for the old bridge, so I'm happy to see it go.

A few changes before merging:

1. Presses shouldn't be coalesced. Every event that inherits canCoalesce() == true from events/AbstractEvent.kt can now be merged by Fabric. With JS busy for a few seconds, 4 taps on a ShapeSource arrived as a single onPress (main delivers all 4); vector and raster source presses work the same way. MapView presses survived in my tests, but only because other touch events happened to be queued between them. Please make canCoalesce() default to false and opt in only CameraChangeEvent, CameraChangedEvent and the intermediate point-annotation drag event.

2. React Native 0.79 is still supported (peerDependencies: >=0.79, Expo SDK 53). On 0.79, EventQueue::enqueueUniqueEvent replaces the newest queued event of the same name for the view without checking whether that one was coalescable; that check came in 0.80 (react/react-native#50988). Since topMapChange carries both coalescable camera changes and non-coalescable idle / style-loaded / frame events, a camera change could overwrite a queued onMapIdle or onDidFinishLoadingStyle there. Please give camera changes their own event name on Android (e.g. topCameraChanged, as on iOS), so topMapChange only carries events that are never merged.

3. The comment on dispatchModern is longer than it needs to be and the reason given for surface id -1 isn't quite right: it's -1 because the event is created with Event(viewId), and it applies to every component event, not just the map view. One sentence is enough, e.g. // The default dispatchModern() uses dispatch() for surfaceId -1, which Fabric never coalesces.

Optional cleanup: IEvent.timestamp and events/AbstractEvent.equals look unused once the drop is gone.

[aderiushev]

Thanks for the review and for measuring it. Pushed b4386d1:

1. canCoalesce() now defaults to false. Only CameraChangeEvent, CameraChangedEvent and the point annotation drag moves (not drag start or end) opt in. LocationEvent also returned true. It goes through the location module's emitter, not a view event, so it never mattered, but it now returns false too, so only the three you listed say true.
2. Camera changes on Android now have their own event name, topCameraChanged (mapped to onCameraChanged, as on iOS), so topMapChange only carries events that are never merged. Thanks for the 0.79 detail: EventQueue.cpp in 0.79.6 matches by name only, and 0.80.0 checks isUnique.
3. The dispatchModern comment is now the one sentence you suggested.

I also removed IEvent.timestamp and equals, with LocationEvent's overrides of them.

Checked in the example app (Android, New Architecture) with the reproducer from the description, plus a ShapeSource and a button that keeps JS busy for 6 s:

• 4 taps on the ShapeSource while JS is busy, 0.7 s apart (closer taps count as a double-tap zoom): 1 of 4 reached JS on the previous commit, 4 of 4 with this one, 3 trials each.
• Slow x30 and Fast x10: still 0 stale onCameraChanged (MapView and CameraGestureObserver), and onMapIdle once per move and once per burst (33 and 10).
• ./gradlew :rnmapbox_maps:testDebugUnitTest passes, and yarn generate leaves no diff.

The iOS Detox failure in the last run (should finish loading a map timed out after 1 min) can't come from this PR, which only changes Android files, and the same job passed on main that day. Could you re-run it? I can't from the fork.