Stop map pins swapping and truncating each other's images

__tests__/usePinImages.test.tsx

@@ -0,0 +1,125 @@
+/**
+ * @format
+ */
+
+import type {ReactElement} from 'react';
+import {View} from 'react-native';
+import {captureRef} from 'react-native-view-shot';
+import ReactTestRenderer from 'react-test-renderer';
+import {usePinImages} from '../src/map/usePinImages';
+import {lightTheme} from '../src/theme/colors';
+
+jest.mock('react-native-view-shot', () => ({
+  __esModule: true,
+  captureRef: jest.fn(),
+}));
+
+const mockCaptureRef = captureRef as jest.MockedFunction<typeof captureRef>;
+
+// Under react-test-renderer a `<View>` ref is the component instance, so a
+// capture host still carries the props it was rendered with. That's what lets
+// a capture be traced back to the pin it was for — the thing that goes wrong
+// when two of them overlap.
+const uriOf = (host: unknown) => {
+  const {children} = (host as {props: {children: ReactElement}}).props;
+  const {icon} = children.props as {icon: string | null};
+  return `data:image/png;base64,${icon ?? 'plain'}`;
+};
+
+/** The data URI the hook last registered for each icon it was handed. */
+const drawn: Record<string, string | undefined> = {};
+
+function Harness({icons}: {icons: readonly string[]}) {
+  const {images, rasterizer, imageNameFor} = usePinImages(lightTheme, icons);
+  for (const icon of icons) {
+    const entry = images[imageNameFor(icon)];
+    drawn[icon] =
+      typeof entry === 'object' && 'source' in entry
+        ? // Every entry this hook builds carries a `{uri}` source.
+          (entry.source as {uri?: string}).uri
+        : undefined;
+  }
+  return rasterizer;
+}
+
+/** Renders the rasterizer and lays out every capture host in one tick. */
+async function layOutPins(icons: readonly string[]) {
+  let tree: ReactTestRenderer.ReactTestRenderer | undefined;
+  await ReactTestRenderer.act(() => {
+    tree = ReactTestRenderer.create(<Harness icons={icons} />);
+  });
+  const renderer = tree as ReactTestRenderer.ReactTestRenderer;
+  await ReactTestRenderer.act(async () => {
+    for (const host of renderer.root.findAllByType(View)) {
+      host.props.onLayout?.();
+    }
+  });
+}
+
+/** Runs the frame the capture queue waits on, then settles React. */
+async function flushFrame() {
+  await ReactTestRenderer.act(async () => {
+    jest.runOnlyPendingTimers();
+  });
+}
+
+beforeEach(() => {
+  jest.useFakeTimers();
+  jest.clearAllMocks();
+  for (const icon of Object.keys(drawn)) delete drawn[icon];
+});
+
+afterEach(() => {
+  jest.useRealTimers();
+});
+
+test('captures pins one at a time', async () => {
+  // Captures that stay in flight until released. The bug this guards against
+  // is invisible to a mock that resolves immediately: react-native-view-shot's
+  // Android module compresses every snapshot through one static byte buffer, so
+  // two captures running at once come back holding each other's bytes.
+  const releases: (() => void)[] = [];
+  let inFlight = 0;
+  let maxInFlight = 0;
+  mockCaptureRef.mockImplementation(host => {
+    inFlight += 1;
+    maxInFlight = Math.max(maxInFlight, inFlight);
+    return new Promise(resolve => {
+      releases.push(() => {
+        inFlight -= 1;
+        resolve(uriOf(host));
+      });
+    });
+  });
+
+  // The plain pin plus three icons: four hosts, all laid out in the same tick.
+  await layOutPins(['🛏️', '🌲', '🍽']);
+  await flushFrame();
+
+  expect(mockCaptureRef).toHaveBeenCalledTimes(1);
+  expect(maxInFlight).toBe(1);
+
+  // Each finished capture lets exactly one more start, never two.
+  for (let done = 1; done < 4; done += 1) {
+    await ReactTestRenderer.act(async () => {
+      releases[done - 1]();
+    });
+    await flushFrame();
+    expect(mockCaptureRef).toHaveBeenCalledTimes(done + 1);
+    expect(maxInFlight).toBe(1);
+  }
+});
+
+test('registers each pin under the name of the icon it drew', async () => {
+  mockCaptureRef.mockImplementation(host => Promise.resolve(uriOf(host)));
+
+  await layOutPins(['🛏️', '🌲', '🍽']);
+  // One capture per frame, and each one re-renders with the next pin to draw.
+  for (let i = 0; i < 8; i += 1) await flushFrame();
+
+  expect(drawn).toEqual({
+    '🛏️': 'data:image/png;base64,🛏️',
+    '🌲': 'data:image/png;base64,🌲',
+    '🍽': 'data:image/png;base64,🍽',
+  });
+});

src/map/usePinImages.tsx

@@ -9,6 +9,12 @@ import {PIN_SELECTED_SCALE, PinIcon} from './PinIcon';
 // capturing at the selected size means that only ever downsamples.
 const RASTER_SCALE = PIN_SELECTED_SCALE;
 
+/** Resolves once the views laid out in this commit have had a frame to draw. */
+const nextFrame = () =>
+  new Promise<void>(resolve => {
+    requestAnimationFrame(() => resolve());
+  });
+
 /**
  * Rasterises map pins into images a symbol layer can draw.
  *
@@ -45,7 +51,10 @@ export function usePinImages(
   // derived from `images` so a capture in flight isn't started twice and a
   // failed one isn't retried forever.
   const attempted = useRef(new Set<string>());
-  const hosts = useRef(new Map<string, ViewInstance | null>());
+  const hosts = useRef(new Map<string, ViewInstance>());
+  // Names waiting to be captured, and whether the drain loop below is running.
+  const queue = useRef<string[]>([]);
+  const capturing = useRef(false);
 
   // A rasterised pin bakes in the colours it was drawn with, so those are part
   // of its name: switching appearance changes both, and the new pins get new
@@ -76,35 +85,70 @@ export function usePinImages(
     );
   }, [icons, images, nameFor, plainPinName]);
 
-  const capture = useCallback((name: string) => {
-    if (attempted.current.has(name)) return;
-    const host = hosts.current.get(name);
-    if (!host) return;
-    attempted.current.add(name);
-
-    // Let the laid-out views actually draw before asking for their pixels:
-    // capture reads them via `view.draw()`, which needs the emoji's text run
-    // resolved and the shadow's drawable in place.
-    requestAnimationFrame(() => {
-      captureRef(host, {format: 'png', result: 'data-uri'})
-        .then(uri => {
+  /**
+   * Captures the queued pins, strictly one at a time.
+   *
+   * The serialisation is the point. react-native-view-shot's Android module
+   * encodes every snapshot through a single *static* byte buffer, and runs
+   * captures on an unbounded thread pool — so two in flight at once compress
+   * their PNGs into the same array. What comes back is one pin's bytes with
+   * another's spliced through them, which decodes to a different pin's emoji
+   * ending in a hard horizontal edge partway down where the bytes stopped
+   * making sense. Nothing in the JS API hints at this, and a whole screen of
+   * elements is exactly the case that starts every capture in one tick.
+   */
+  const drain = useCallback(async () => {
+    if (capturing.current) return;
+    capturing.current = true;
+    try {
+      for (;;) {
+        const name = queue.current.shift();
+        if (name === undefined) break;
+        const host = hosts.current.get(name);
+        if (!host) {
+          // Unmounted before its turn. Forget the attempt so a pin that comes
+          // back — a re-entered screen, a re-added element — is captured then.
+          attempted.current.delete(name);
+          continue;
+        }
+        // Let the laid-out views actually draw before asking for their pixels:
+        // capture reads them via `view.draw()`, which needs the emoji's text
+        // run resolved and the shadow's drawable in place.
+        await nextFrame();
+        try {
+          const uri = await captureRef(host, {
+            format: 'png',
+            result: 'data-uri',
+          });
           hosts.current.delete(name);
           setImages(prev => ({
             ...prev,
             // Declaring the scale the pin was captured at is what makes the
             // image's natural size on the map its unscaled point size.
             [name]: {source: {uri, scale: PixelRatio.get() * RASTER_SCALE}},
           }));
-        })
-        .catch((error: unknown) => {
+        } catch (error: unknown) {
           hosts.current.delete(name);
           // The name stays in `attempted` so we don't retry in a loop; the
           // element keeps the plain pin, which is a legible fallback.
           console.warn(`Failed to rasterize map pin ${name}:`, error);
-        });
-    });
+        }
+      }
+    } finally {
+      capturing.current = false;
+    }
   }, []);
 
+  const capture = useCallback(
+    (name: string) => {
+      if (attempted.current.has(name)) return;
+      attempted.current.add(name);
+      queue.current.push(name);
+      void drain();
+    },
+    [drain],
+  );
+
   const rasterizer = (
     <View pointerEvents="none" style={styles.offscreen}>
       {pending.map(({name, icon}) => (
@@ -113,7 +157,8 @@ export function usePinImages(
           collapsable={false}
           onLayout={() => capture(name)}
           ref={host => {
-            hosts.current.set(name, host);
+            if (host) hosts.current.set(name, host);
+            else hosts.current.delete(name);
           }}>
           <PinIcon theme={theme} icon={icon} scale={RASTER_SCALE} />
         </View>