diff --git a/frontend/src/components/common/AuthenticatedImage.tsx b/frontend/src/components/common/AuthenticatedImage.tsx index 130da0be..484ce41e 100644 --- a/frontend/src/components/common/AuthenticatedImage.tsx +++ b/frontend/src/components/common/AuthenticatedImage.tsx @@ -98,14 +98,16 @@ export const AuthenticatedImage: React.FC = ({ const imageRef = useRef(null); // Draw image to canvas when canvas rendering is enabled + // Returns whether the pixels actually made it onto the canvas, so the + // caller knows if the source image is still needed (#1287). const drawToCanvas = useCallback(() => { - if (!useCanvasRendering || !canvasRef.current || !imageRef.current) return; + if (!useCanvasRendering || !canvasRef.current || !imageRef.current) return false; const canvas = canvasRef.current; const img = imageRef.current; const ctx = canvas.getContext('2d'); - if (!ctx || !img.complete || img.naturalWidth === 0) return; + if (!ctx || !img.complete || img.naturalWidth === 0) return false; // Set canvas dimensions to match image canvas.width = img.naturalWidth; @@ -115,6 +117,7 @@ export const AuthenticatedImage: React.FC = ({ ctx.drawImage(img, 0, 0); setCanvasReady(true); + return true; }, [useCanvasRendering]); useEffect(() => { @@ -271,7 +274,18 @@ export const AuthenticatedImage: React.FC = ({ img.onload = () => { imageRef.current = img; - drawToCanvas(); + const drawn = drawToCanvas(); + // Once drawImage has copied the pixels into the canvas the source + // decode is dead weight, so drop it here rather than at unmount. The + // grid is not virtualised — a 546-photo event mounts 546 of these and + // none of them unmount while the gallery is open — so a cleanup-only + // release never actually runs for the case it was meant to fix + // (#1287). Nothing redraws from `imageRef` afterwards: drawToCanvas + // has this one caller. + if (drawn) { + imageRef.current = null; + img.removeAttribute('src'); + } onLoad?.(); }; @@ -286,16 +300,13 @@ export const AuthenticatedImage: React.FC = ({ return () => { img.onload = null; img.onerror = null; - // Release the decoded bitmap (#1287). `imageRef` is what drawToCanvas - // reads, and it was never cleared — so a detached Image, and the - // decode behind it, stayed pinned by a live JS reference for the - // lifetime of the component. A decoded in the document is - // evictable under memory pressure; one held by a ref is not. - // - // This matters at gallery scale because the grid is not virtualised: - // a 546-photo event mounts 546 of these and none of them ever unmount, - // so nothing was ever released. Dropping the src first lets the - // browser reclaim the decode without waiting for GC to notice. + // Fallback release for the paths the onload handler above cannot + // cover: the draw failed, or the source changed / the component + // unmounted before onload ever fired. `imageRef` is what drawToCanvas + // reads and it was never cleared, so a detached Image — and the decode + // behind it — stayed pinned by a live JS reference. A decoded in + // the document is evictable under memory pressure; one held by a ref + // is not. if (imageRef.current === img) { imageRef.current = null; } diff --git a/frontend/src/components/common/__tests__/AuthenticatedImage.canvasRelease.test.tsx b/frontend/src/components/common/__tests__/AuthenticatedImage.canvasRelease.test.tsx index a8b6ffd0..52b21f0f 100644 --- a/frontend/src/components/common/__tests__/AuthenticatedImage.canvasRelease.test.tsx +++ b/frontend/src/components/common/__tests__/AuthenticatedImage.canvasRelease.test.tsx @@ -50,6 +50,12 @@ beforeEach(() => { constructor() { super(); created.push(this as unknown as HTMLImageElement); + // jsdom leaves these at 0/false for a blob: src, which makes + // drawToCanvas bail before it draws. Present a decoded image so the + // draw path is reachable. + Object.defineProperty(this, 'complete', { get: () => true }); + Object.defineProperty(this, 'naturalWidth', { get: () => 10 }); + Object.defineProperty(this, 'naturalHeight', { get: () => 10 }); // jsdom never fires load for a blob: src, so drive it manually. setTimeout(() => this.onload?.(new Event('load')), 0); } @@ -59,6 +65,31 @@ beforeEach(() => { afterEach(() => vi.unstubAllGlobals()); describe('AuthenticatedImage canvas mode', () => { + it('releases the decoded image as soon as it is drawn, without waiting for unmount', async () => { + // The case this whole change exists for. Every other test here asserts + // release on unmount or src change — neither of which happens to a grid + // tile, because the grid is not virtualised and the tiles stay mounted + // for as long as the gallery is open. Once drawImage has copied the + // pixels the source decode is dead weight and must go immediately. + const ctx = { drawImage: vi.fn() }; + const getContext = vi + .spyOn(HTMLCanvasElement.prototype, 'getContext') + .mockReturnValue(ctx as unknown as CanvasRenderingContext2D); + + render( + + ); + + await waitFor(() => expect(created.length).toBeGreaterThan(0)); + const img = created[0]; + + await waitFor(() => expect(ctx.drawImage).toHaveBeenCalled()); + // Still mounted, still the same src — and already released. + await waitFor(() => expect(img.getAttribute('src')).toBeNull()); + + getContext.mockRestore(); + }); + it('releases the decoded image on unmount', async () => { const { unmount } = render(