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); }}>