From f5b4f01c4a2117cd2331461be18362bdaa9d8b5c Mon Sep 17 00:00:00 2001 From: Dennis Falling Date: Sat, 29 Aug 2026 17:59:31 +0100 Subject: [PATCH] Stop map pins swapping and truncating each other's images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit react-native-view-shot's Android module compresses every snapshot through a single static byte buffer and dispatches captures on an unbounded thread pool, with no synchronisation anywhere between the two. We started a capture for every distinct pin emoji in one animation frame, so they raced through that one array: the data URI that came back for a pin held another pin's PNG bytes with its own spliced through the tail. On the map that decoded as the wrong emoji, ending in a hard horizontal edge partway down where the bytes stopped making sense — a bed showing a tree with no teardrop tip, which reads as a pin that is also too big and off its spot. Capture through a serial queue instead: at most one snapshot in flight, so no two ever share the buffer. Pins now rasterise one per frame, and until one lands its element keeps the plain teardrop that already stands in for an unrasterised icon. Co-Authored-By: Claude Opus 5 (1M context) --- __tests__/usePinImages.test.tsx | 125 ++++++++++++++++++++++++++++++++ src/map/usePinImages.tsx | 81 ++++++++++++++++----- 2 files changed, 188 insertions(+), 18 deletions(-) create mode 100644 __tests__/usePinImages.test.tsx diff --git a/__tests__/usePinImages.test.tsx b/__tests__/usePinImages.test.tsx new file mode 100644 index 0000000..33f25b5 --- /dev/null +++ b/__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; + +// Under react-test-renderer a `` 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 = {}; + +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(); + }); + 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,🍽', + }); +}); diff --git a/src/map/usePinImages.tsx b/src/map/usePinImages.tsx index 287368c..643d6bf 100644 --- a/src/map/usePinImages.tsx +++ b/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(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()); - const hosts = useRef(new Map()); + const hosts = useRef(new Map()); + // Names waiting to be captured, and whether the drain loop below is running. + const queue = useRef([]); + 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,18 +85,41 @@ 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, @@ -95,16 +127,28 @@ export function usePinImages( // 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 = ( {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); }}>