diff --git a/apps/desktop/src/app/chat/composer/attachments.test.tsx b/apps/desktop/src/app/chat/composer/attachments.test.tsx index b635ab9ca9..e356098bf7 100644 --- a/apps/desktop/src/app/chat/composer/attachments.test.tsx +++ b/apps/desktop/src/app/chat/composer/attachments.test.tsx @@ -1,5 +1,5 @@ -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, it } from 'vitest' +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' import { I18nProvider } from '@/i18n/context' import type { ComposerAttachment } from '@/store/composer' @@ -30,6 +30,8 @@ async function renderWithI18n(ui: React.ReactNode) { describe('AttachmentList', () => { afterEach(() => { cleanup() + Reflect.deleteProperty(window, 'hermesDesktop') + vi.restoreAllMocks() }) it('renders valid attachments', async () => { @@ -98,6 +100,45 @@ describe('AttachmentList', () => { expect($previewTabs.get()).toHaveLength(0) }) + it('loads a path-backed full image only when opened and releases it when closed', async () => { + const readFileDataUrl = vi.fn(async () => DATA_URL) + + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl } + }) + + const image: ComposerAttachment = { + id: 'img-on-demand', + kind: 'image', + label: 'shot.png', + path: '/tmp/shot.png', + thumbnailUrl: THUMBNAIL_URL + } + + const { container } = await renderWithI18n() + + expect(readFileDataUrl).not.toHaveBeenCalled() + expect(screen.getByAltText('shot.png').getAttribute('src')).toBe(THUMBNAIL_URL) + expect(container.querySelector(`img[src="${DATA_URL}"]`)).toBeNull() + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /shot\.png/ })) + }) + + expect(readFileDataUrl).toHaveBeenCalledOnce() + const lightboxImage = (await screen.findByRole('dialog')).querySelector('img') + + expect(lightboxImage?.getAttribute('src')).toBe(DATA_URL) + + await act(async () => { + fireEvent.click(lightboxImage!) + }) + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + expect(container.querySelector(`img[src="${DATA_URL}"]`)).toBeNull() + }) + it('still routes a non-image attachment to the preview rail', async () => { $previewTabs.set([]) diff --git a/apps/desktop/src/app/chat/composer/attachments.tsx b/apps/desktop/src/app/chat/composer/attachments.tsx index bd062eec19..46ee988129 100644 --- a/apps/desktop/src/app/chat/composer/attachments.tsx +++ b/apps/desktop/src/app/chat/composer/attachments.tsx @@ -7,6 +7,7 @@ import { Codicon } from '@/components/ui/codicon' import { Tip } from '@/components/ui/tooltip' import { useImageDownload } from '@/hooks/use-image-download' import { useI18n } from '@/i18n' +import { readDesktopFileDataUrlLocalFirst } from '@/lib/desktop-fs' import { AlertCircle, FileText, FolderOpen, ImageIcon, Link, Loader2, MessageCode, Terminal } from '@/lib/icons' import { normalizeOrLocalPreviewTarget } from '@/lib/local-preview' import { cn } from '@/lib/utils' @@ -59,11 +60,10 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme ? attachment.detail : undefined - // An attached image already holds its full bytes as a data URL, so it belongs - // in the same lightbox the thread uses. The rail is for files you read or - // edit — not a picture you just want to look at. Images that never resolved a - // thumbnail still fall through to the rail rather than dead-clicking. - const lightboxSrc = attachment.kind === 'image' && !isUploading ? attachment.previewUrl : undefined + // Keep full image bytes out of composer state. New chips read their path only + // when clicked; previewUrl remains a compatibility fallback for older drafts. + const [loadedImageSrc, setLoadedImageSrc] = useState() + const lightboxSrc = attachment.kind === 'image' && !isUploading ? attachment.previewUrl || loadedImageSrc : undefined const [lightboxOpen, setLightboxOpen] = useState(false) const { download, saving } = useImageDownload(lightboxSrc) @@ -72,8 +72,19 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme return } - if (lightboxSrc) { - setLightboxOpen(true) + if (attachment.kind === 'image') { + try { + const source = lightboxSrc || (attachment.path ? await readDesktopFileDataUrlLocalFirst(attachment.path) : '') + + if (!source) { + throw new Error(c.couldNotPreview(attachment.label)) + } + + setLoadedImageSrc(source) + setLightboxOpen(true) + } catch (error) { + notifyError(error, c.previewUnavailable) + } return } @@ -122,7 +133,7 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme type="button" > - {attachment.previewUrl && attachment.kind === 'image' ? ( + {(attachment.thumbnailUrl || attachment.previewUrl) && attachment.kind === 'image' ? ( {attachment.label} { + setLightboxOpen(open) + + if (!open && !attachment.previewUrl) { + setLoadedImageSrc(undefined) + } + }} open={lightboxOpen} saving={saving} src={lightboxSrc} diff --git a/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts b/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts index 7436a296f8..8eac004725 100644 --- a/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts +++ b/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts @@ -1,4 +1,4 @@ -import { act, renderHook } from '@testing-library/react' +import { act, renderHook, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' import { $composerAttachments, type ComposerAttachment } from '@/store/composer' @@ -331,15 +331,77 @@ describe('attachImagePath thumbnail separation', () => { vi.unstubAllGlobals() delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop $composerAttachments.set([]) + $connection.set(null) }) - it('keeps the full-resolution previewUrl and stores a separate downscaled thumbnailUrl', async () => { + it('does not resurrect an attachment removed while its queued resize is in flight', async () => { const readFileDataUrl = vi.fn(async () => FULL_RES) - ;(window as unknown as { hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } }).hermesDesktop = { - readFileDataUrl + ;( + window as unknown as { + hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } + } + ).hermesDesktop = { readFileDataUrl } + + let resolveBitmap!: (bitmap: { close: () => void; height: number; width: number }) => void + + const createBitmap = vi.fn( + () => + new Promise<{ close: () => void; height: number; width: number }>(resolve => { + resolveBitmap = resolve + }) + ) + + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ blob: async () => new Blob([new Uint8Array([0])], { type: 'image/png' }) })) + ) + vi.stubGlobal('createImageBitmap', createBitmap) + + class MockOffscreenCanvas { + getContext = () => ({ drawImage: vi.fn() }) + convertToBlob = vi.fn(async () => new Blob(['x'], { type: 'image/png' })) + constructor(_width: number, _height: number) {} } + vi.stubGlobal('OffscreenCanvas', MockOffscreenCanvas) + + const { result } = renderHook(() => + useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() }) + ) + + let pending!: Promise + + act(() => { + pending = result.current.attachImagePath('/tmp/shot.png') + }) + + await waitFor(() => expect(createBitmap).toHaveBeenCalledOnce()) + expect($composerAttachments.get()).toHaveLength(1) + + await act(async () => { + await result.current.removeAttachment('image:/tmp/shot.png') + }) + expect($composerAttachments.get()).toEqual([]) + + resolveBitmap({ close: vi.fn(), height: 3000, width: 4000 }) + + await act(async () => { + await pending + }) + + expect($composerAttachments.get()).toEqual([]) + }) + + it('retains only the bounded thumbnail after creating a composer image preview', async () => { + const readFileDataUrl = vi.fn(async () => FULL_RES) + + ;( + window as unknown as { + hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } + } + ).hermesDesktop = { readFileDataUrl } + // Exercise the real downscale path: 4000×3000 bitmap → 512×384 canvas. const drawImage = vi.fn() const close = vi.fn() @@ -374,23 +436,78 @@ describe('attachImagePath thumbnail separation', () => { const attachment = $composerAttachments.get()[0] expect(attachment).toBeDefined() - // Full-resolution bytes stay on `previewUrl` — the same field main feeds - // to ImageLightbox and useImageDownload, so open/download keep full res. - expect(attachment?.previewUrl).toBe(FULL_RES) - // The pill thumbnail is a SEPARATE downscaled value, never the multi-MB - // original (which would re-introduce the main-thread decode freeze). + // The composer retains only the bounded value; full bytes are re-read from + // `path` on demand by the lightbox and independently by submit/upload. + expect(attachment?.previewUrl).toBeUndefined() expect(attachment?.thumbnailUrl).toBeDefined() expect(attachment?.thumbnailUrl).not.toBe(FULL_RES) + expect(JSON.stringify(attachment)).not.toContain(FULL_RES) expect(drawImage).toHaveBeenCalledWith(expect.anything(), 0, 0, 512, 384) expect(close).toHaveBeenCalled() }) + it('processes 72 image reads one at a time and retains only bounded thumbnails', async () => { + let activeReads = 0 + let maxActiveReads = 0 + + const readFileDataUrl = vi.fn(async () => { + activeReads += 1 + maxActiveReads = Math.max(maxActiveReads, activeReads) + await Promise.resolve() + activeReads -= 1 + + return FULL_RES + }) + + ;( + window as unknown as { + hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } + } + ).hermesDesktop = { readFileDataUrl } + + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ blob: async () => new Blob([new Uint8Array([0])], { type: 'image/png' }) })) + ) + vi.stubGlobal( + 'createImageBitmap', + vi.fn(async () => ({ close: vi.fn(), height: 6000, width: 6000 })) + ) + + class MockOffscreenCanvas { + getContext = () => ({ drawImage: vi.fn() }) + convertToBlob = vi.fn(async () => new Blob(['x'], { type: 'image/png' })) + constructor(_width: number, _height: number) {} + } + + vi.stubGlobal('OffscreenCanvas', MockOffscreenCanvas) + + const { result } = renderHook(() => + useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() }) + ) + + await act(async () => { + await Promise.all(Array.from({ length: 72 }, (_, index) => result.current.attachImagePath(`/tmp/${index}.png`))) + }) + + const attachments = $composerAttachments.get() + + expect(readFileDataUrl).toHaveBeenCalledTimes(72) + expect(maxActiveReads).toBe(1) + expect(attachments).toHaveLength(72) + expect(attachments.every(attachment => attachment.thumbnailUrl?.startsWith('data:image/'))).toBe(true) + expect(attachments.every(attachment => attachment.previewUrl === undefined)).toBe(true) + expect(JSON.stringify(attachments)).not.toContain(FULL_RES) + }) + it('leaves the thumbnail undefined when the preview is not an image', async () => { const readFileDataUrl = vi.fn(async () => 'data:text/plain;base64,aGVsbG8=') - ;(window as unknown as { hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } }).hermesDesktop = { - readFileDataUrl - } + ;( + window as unknown as { + hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } + } + ).hermesDesktop = { readFileDataUrl } const { result } = renderHook(() => useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() }) diff --git a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts index d3f1577a19..2173378a98 100644 --- a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts +++ b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts @@ -5,7 +5,7 @@ import { droppedFileInlineRef } from '@/app/chat/composer/inline-refs' import { formatRefValue } from '@/components/assistant-ui/directive-text' import { useI18n } from '@/i18n' import { attachmentId, contextPath, pathLabel } from '@/lib/chat-runtime' -import { readDesktopFileDataUrl, selectDesktopPaths } from '@/lib/desktop-fs' +import { readDesktopFileDataUrlLocalFirst, selectDesktopPaths } from '@/lib/desktop-fs' import { desktopGit } from '@/lib/desktop-git' import { downscaleDataUrlForPreview } from '@/lib/image-resize' import { normalize } from '@/lib/text' @@ -53,22 +53,25 @@ export function isImagePath(filePath: string): boolean { * In local mode the facade IS the local bridge, so this stays a single read. */ export async function attachmentPreviewDataUrl(filePath: string): Promise { - let dataUrl: string + return readDesktopFileDataUrlLocalFirst(filePath) +} - try { - const local = await window.hermesDesktop?.readFileDataUrl?.(filePath) +let attachmentPreviewQueue = Promise.resolve() - if (local) { - dataUrl = local - } else { - dataUrl = await readDesktopFileDataUrl(filePath) - } - } catch { - // Not on this machine (or unreadable locally) — try the gateway. - dataUrl = await readDesktopFileDataUrl(filePath) - } +async function queuedAttachmentPreview(filePath: string): Promise<{ previewUrl: string; thumbnailUrl?: string }> { + const task = attachmentPreviewQueue.then(async () => { + const previewUrl = await attachmentPreviewDataUrl(filePath) + const thumbnailUrl = previewUrl.startsWith('data:image/') ? await downscaleDataUrlForPreview(previewUrl) : undefined - return dataUrl + return { previewUrl, thumbnailUrl } + }) + + attachmentPreviewQueue = task.then( + () => undefined, + () => undefined + ) + + return task } export interface DroppedFile { @@ -473,20 +476,15 @@ export function useComposerActions({ attachToMain(baseAttachment) try { - const previewUrl = await attachmentPreviewDataUrl(filePath) + const { previewUrl, thumbnailUrl } = await queuedAttachmentPreview(filePath) if (previewUrl) { - // Downscale only the pill thumbnail. `previewUrl` must keep the - // full-resolution bytes: current main feeds it to ImageLightbox and - // useImageDownload (attachments.tsx), and the attached-image - // pipeline uploads the on-disk original to the model. The helper - // never rejects — on failure it returns a 1×1 placeholder so the - // pill never renders the multi-MB original. - const thumbnailUrl = previewUrl.startsWith('data:image/') - ? await downscaleDataUrlForPreview(previewUrl) - : undefined - - scope.add({ ...baseAttachment, previewUrl, thumbnailUrl }) + // Keep only the bounded thumbnail in composer state. The full source + // is read on demand for lightbox/download and separately at submit + // for the model, so retaining 72 multi-MB data URLs serves no purpose. + // The user may remove the optimistic chip while an expensive image is + // still decoding. A late result updates in place but never resurrects it. + scope.update(thumbnailUrl ? { ...baseAttachment, thumbnailUrl } : { ...baseAttachment, previewUrl }) } return true diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index 52976138e1..aa6d4a3210 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -134,8 +134,8 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // Refs are recomputed after sync (file.attach rewrites @file: refs to // workspace-relative paths the remote gateway can resolve). Seed the // optimistic message with the pre-sync refs, then rewrite once synced. - // Images use their base64 preview so the thumbnail renders inline without - // a (remote-mode 403-prone) /api/media fetch — see optimisticAttachmentRef. + // Images use their bounded base64 thumbnail so the optimistic bubble + // renders inline without embedding the full source — see optimisticAttachmentRef. let attachmentRefs = attachments.map(optimisticAttachmentRef).filter((r): r is string => Boolean(r)) const buildContextText = (atts: ComposerAttachment[]): string => { @@ -646,7 +646,7 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // Rewrite the optimistic message + prompt text with the synced refs so // the gateway receives @file: paths that resolve in its workspace. - // (Images keep their inline base64 preview — see optimisticAttachmentRef.) + // Images keep their inline bounded thumbnail — see optimisticAttachmentRef. attachmentRefs = syncedAttachments.map(optimisticAttachmentRef).filter((r): r is string => Boolean(r)) rewriteOptimistic(liveSessionId) const text = buildContextText(syncedAttachments) diff --git a/apps/desktop/src/lib/chat-runtime.test.ts b/apps/desktop/src/lib/chat-runtime.test.ts index b9038396d3..09c079e688 100644 --- a/apps/desktop/src/lib/chat-runtime.test.ts +++ b/apps/desktop/src/lib/chat-runtime.test.ts @@ -33,21 +33,20 @@ describe('optimisticAttachmentRef', () => { attachment({ kind: 'image', detail: '/tmp/shot.png', previewUrl: DATA_URL, thumbnailUrl: THUMB_URL }) ) - // The bubble is a display-only thumbnail; full-res previewUrl stays for - // lightbox/download and the model gets bytes via the upload pipeline. + // The bubble is display-only; full bytes are read on demand and for upload. expect(ref).toBe(THUMB_URL) }) - it('falls back to an @image: path ref when no preview is available', () => { - expect(optimisticAttachmentRef(attachment({ kind: 'image', detail: '/tmp/shot.png' }))).toBe('@image:/tmp/shot.png') + it('does not render a full path-backed image while its bounded thumbnail is pending', () => { + expect(optimisticAttachmentRef(attachment({ kind: 'image', detail: '/tmp/shot.png' }))).toBeNull() }) - it('ignores a non-data preview url and uses the path ref', () => { + it('does not use a path fallback for a non-data preview url', () => { const ref = optimisticAttachmentRef( attachment({ kind: 'image', detail: '/tmp/shot.png', previewUrl: 'https://example.com/x.png' }) ) - expect(ref).toBe('@image:/tmp/shot.png') + expect(ref).toBeNull() }) it('passes non-image attachments straight through to attachmentDisplayText', () => { diff --git a/apps/desktop/src/lib/chat-runtime.ts b/apps/desktop/src/lib/chat-runtime.ts index 30dc625119..c976e98fff 100644 --- a/apps/desktop/src/lib/chat-runtime.ts +++ b/apps/desktop/src/lib/chat-runtime.ts @@ -241,14 +241,11 @@ export function attachmentDisplayText(attachment: ComposerAttachment): string | /** * Display ref for the optimistic (in-flight) user bubble. * - * Images prefer their in-hand base64 preview (a `data:` URL) over a file path. - * `DirectiveContent` runs `extractEmbeddedImages` first, so a raw `data:` URL - * renders as an inline thumbnail with zero network. An `@image:` ref - * would instead route through `/api/media`, which in remote mode 403s ("Path - * outside media roots") on a local path the gateway can't read yet — flashing a - * fallback chip until submit uploads the bytes. The preview also survives the - * post-sync rewrite (bytes go to the agent via the attached-image pipeline, not - * this display ref), so the thumbnail stays stable instead of remounting. + * Images prefer their bounded base64 thumbnail over a file path. A raw `data:` + * URL renders inline with zero network, while an `@image:` ref would + * route through `/api/media` and can 403 in remote mode. Full-resolution bytes + * are loaded separately for the model and on-demand lightbox, not retained in + * the optimistic message. * * Everything else (files, folders, terminals, post-sync `@file:` refs) falls * through to `attachmentDisplayText`. @@ -258,11 +255,24 @@ export function optimisticAttachmentRef(attachment: ComposerAttachment): string return null } - if (attachment.kind === 'image' && attachment.previewUrl?.startsWith('data:')) { - // The pill and the in-flight bubble render the downscaled thumbnail; the - // full-resolution `previewUrl` stays for lightbox/download, and the model - // receives bytes via the attached-image upload pipeline, not this ref. - return attachment.thumbnailUrl ?? attachment.previewUrl + if (attachment.kind === 'image') { + if (attachment.thumbnailUrl?.startsWith('data:')) { + // The pill and the in-flight bubble render the bounded thumbnail. Full + // bytes are read separately for lightbox/download and model upload. + return attachment.thumbnailUrl + } + + if (attachment.previewUrl?.startsWith('data:')) { + // Backward compatibility for drafts created by older shells without a + // separate thumbnail. + return attachment.previewUrl + } + + // A newly attached image has no thumbnail while its queued resize is still + // pending. Do not fall through to @image:: the optimistic bubble would + // fetch and paint the full source, recreating the freeze if Send wins the + // race. The model upload remains path/byte based and is unaffected. + return null } return attachmentDisplayText(attachment) diff --git a/apps/desktop/src/lib/desktop-fs.ts b/apps/desktop/src/lib/desktop-fs.ts index a193fabfd9..1775911b1b 100644 --- a/apps/desktop/src/lib/desktop-fs.ts +++ b/apps/desktop/src/lib/desktop-fs.ts @@ -109,6 +109,25 @@ export async function readDesktopFileDataUrl(path: string): Promise { return typeof result === 'string' ? result : result.dataUrl || '' } +/** + * Read a composer image local-shell first, even when the active agent is + * remote. Picker, clipboard, and OS-drop paths belong to this machine; in-app + * project-tree paths may belong only to the gateway and fall back there. + */ +export async function readDesktopFileDataUrlLocalFirst(path: string): Promise { + try { + const local = await window.hermesDesktop?.readFileDataUrl?.(path) + + if (local) { + return local + } + } catch { + // Not on this machine (or unreadable locally) — try the active gateway. + } + + return readDesktopFileDataUrl(path) +} + export async function desktopGitRoot(path: string): Promise { const desktop = bridge() diff --git a/apps/desktop/src/lib/image-resize.test.ts b/apps/desktop/src/lib/image-resize.test.ts index af9d9d8e72..cc23de1ff2 100644 --- a/apps/desktop/src/lib/image-resize.test.ts +++ b/apps/desktop/src/lib/image-resize.test.ts @@ -18,15 +18,15 @@ describe('downscaleDataUrlForPreview', () => { }) describe('without createImageBitmap (jsdom default)', () => { - it('returns the original when createImageBitmap is unavailable', async () => { - await expect(downscaleDataUrlForPreview(TINY_PNG_DATA_URL)).resolves.toBe(TINY_PNG_DATA_URL) + it('fails closed when createImageBitmap is unavailable', async () => { + await expect(downscaleDataUrlForPreview('data:image/png;base64,ZmFrZQ==')).resolves.toBe(FALLBACK_PLACEHOLDER) }) - it('returns the original for a non-data-URL string', async () => { + it('preserves an external source when resize APIs are unavailable', async () => { await expect(downscaleDataUrlForPreview('not-a-data-url')).resolves.toBe('not-a-data-url') }) - it('returns the original for a data URL without a comma', async () => { + it('preserves a non-image data URL when resize APIs are unavailable', async () => { await expect(downscaleDataUrlForPreview('data:text/plain')).resolves.toBe('data:text/plain') }) }) @@ -37,7 +37,13 @@ describe('downscaleDataUrlForPreview', () => { * Mocks a bitmap of the given dimensions so the downscaling logic is exercised. */ function setupMocks(bitmapWidth: number, bitmapHeight: number) { - const close = vi.fn() + let activeBitmaps = 0 + let maxActiveBitmaps = 0 + + const close = vi.fn(() => { + activeBitmaps -= 1 + }) + const drawImage = vi.fn() const convertToBlob = vi.fn(async () => new Blob(['x'], { type: 'image/png' })) const canvasSizes: [number, number][] = [] @@ -62,11 +68,16 @@ describe('downscaleDataUrlForPreview', () => { ) vi.stubGlobal( 'createImageBitmap', - vi.fn(async () => bitmap) + vi.fn(async () => { + activeBitmaps += 1 + maxActiveBitmaps = Math.max(maxActiveBitmaps, activeBitmaps) + + return bitmap + }) ) vi.stubGlobal('OffscreenCanvas', MockOffscreenCanvas) - return { bitmap, close, drawImage, convertToBlob, ctx, canvasSizes } + return { bitmap, close, drawImage, convertToBlob, ctx, canvasSizes, maxActiveBitmaps: () => maxActiveBitmaps } } it('returns the original when image is smaller than maxLongEdge', async () => { @@ -98,13 +109,14 @@ describe('downscaleDataUrlForPreview', () => { }) it('keeps every thumbnail bounded for a 72-image composer prompt', async () => { - const { canvasSizes, drawImage } = setupMocks(6000, 6000) + const { canvasSizes, drawImage, maxActiveBitmaps } = setupMocks(6000, 6000) await Promise.all(Array.from({ length: 72 }, () => downscaleDataUrlForPreview(TINY_PNG_DATA_URL))) expect(canvasSizes).toHaveLength(72) expect(canvasSizes.every(([width, height]) => width === 512 && height === 512)).toBe(true) expect(drawImage).toHaveBeenCalledTimes(72) + expect(maxActiveBitmaps()).toBe(1) }) it('returns placeholder when createImageBitmap throws', async () => { diff --git a/apps/desktop/src/lib/image-resize.ts b/apps/desktop/src/lib/image-resize.ts index 9da24b951d..1639488363 100644 --- a/apps/desktop/src/lib/image-resize.ts +++ b/apps/desktop/src/lib/image-resize.ts @@ -1,54 +1,35 @@ /** * Downscale a data URL for preview rendering. * - * Large images (Retina screenshots, 6000×4000+) cause the Chromium main thread - * to block during decode because macOS ImageIO/vImage decodes synchronously. - * This utility uses createImageBitmap + OffscreenCanvas to resize *before* the - * data URL is assigned to an element, keeping the preview fast. + * Large images (Retina screenshots, 6000×4000+) cause Chromium to build very + * large paint operations when the original is assigned to an . This + * utility uses createImageBitmap + OffscreenCanvas to resize before display. * - * Full-resolution bytes are still saved to disk for the model — only the preview - * thumbnail is downscaled. + * Calls share a one-at-a-time queue. Multi-image attach flows must not keep + * dozens of decoded full-resolution bitmaps alive concurrently while their + * 512px thumbnails are produced. * * @param dataUrl The full-resolution data URL (data:image/...;base64,...) * @param maxLongEdge Maximum pixel dimension on the longest side (default 512) - * @returns A downscaled data URL (PNG), the original if already small enough, - * or a 1×1 transparent PNG placeholder if downscaling fails. + * @returns A downscaled PNG data URL, the original if already small enough, or + * a 1×1 transparent PNG placeholder if downscaling fails. */ -/** 1×1 transparent PNG — used when downscaling fails so the UI never receives a multi-MB data URL. */ +/** 1×1 transparent PNG — fail closed so the pill never receives the original. */ const FALLBACK_PLACEHOLDER = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+GkZcAAAAASUVORK5CYII=' // Composer pills render at 32 CSS pixels. A 512px source stays sharp on -// high-density displays while keeping a square RGBA decode to 1 MiB. The old -// 2048px default could decode to 16 MiB per thumbnail, so a large multi-image -// prompt still put substantial pressure on Chromium's renderer. +// high-density displays while keeping a square RGBA decode to 1 MiB. A 2048px +// thumbnail can decode to 16 MiB and reproduce Chromium's paint-op pressure. const DEFAULT_MAX_LONG_EDGE = 512 -export async function downscaleDataUrlForPreview( - dataUrl: string, - maxLongEdge = DEFAULT_MAX_LONG_EDGE -): Promise { - // Guard: createImageBitmap and OffscreenCanvas are not available in jsdom - // (test environment) or very old Chromium builds. Return the original if missing. - if (typeof createImageBitmap !== 'function' || typeof OffscreenCanvas !== 'function') { - return dataUrl - } - - const commaIndex = dataUrl.indexOf(',') - - if (commaIndex === -1) { - return dataUrl - } +let resizeQueue = Promise.resolve() +async function downscaleDataUrl(dataUrl: string, maxLongEdge: number): Promise { try { - // fetch(dataUrl) decodes base64 natively in C++ — avoids the O(n) atob() - // + charCodeAt loop that creates main-thread jank for multi-MB images. - const blob = await fetch(dataUrl).then(r => r.blob()) - - // createImageBitmap decodes off the main thread in Chromium, but the - // operation may still be costly for >5 MB retina screenshots; we then - // resize quickly with OffscreenCanvas. + // fetch(data:) decodes base64 natively in C++ and avoids an O(n) atob loop. + const blob = await fetch(dataUrl).then(response => response.blob()) const bitmap = await createImageBitmap(blob) try { @@ -60,9 +41,8 @@ export async function downscaleDataUrlForPreview( } const scale = maxLongEdge / longEdge - const newWidth = Math.round(width * scale) - const newHeight = Math.round(height * scale) - + const newWidth = Math.max(1, Math.round(width * scale)) + const newHeight = Math.max(1, Math.round(height * scale)) const canvas = new OffscreenCanvas(newWidth, newHeight) const ctx = canvas.getContext('2d') @@ -75,15 +55,9 @@ export async function downscaleDataUrlForPreview( const resultBlob = await canvas.convertToBlob({ type: 'image/png' }) const reader = new FileReader() - return new Promise(resolve => { - reader.onloadend = () => { - if (typeof reader.result === 'string') { - resolve(reader.result) - } else { - resolve(FALLBACK_PLACEHOLDER) - } - } - + return await new Promise(resolve => { + reader.onloadend = () => resolve(typeof reader.result === 'string' ? reader.result : FALLBACK_PLACEHOLDER) + reader.onabort = () => resolve(FALLBACK_PLACEHOLDER) reader.onerror = () => resolve(FALLBACK_PLACEHOLDER) reader.readAsDataURL(resultBlob) }) @@ -91,9 +65,31 @@ export async function downscaleDataUrlForPreview( bitmap.close() } } catch { - // Downscaling failed (unsupported format, OOM, etc.) — return a tiny - // placeholder rather than the original multi-MB data URL, which would - // re-introduce the main-thread decode freeze this function exists to prevent. return FALLBACK_PLACEHOLDER } } + +export async function downscaleDataUrlForPreview( + dataUrl: string, + maxLongEdge = DEFAULT_MAX_LONG_EDGE +): Promise { + if (!dataUrl.startsWith('data:image/') || dataUrl.indexOf(',') === -1) { + return dataUrl + } + + // Never hand a full-resolution image to the pill if resize support is absent. + // A tiny placeholder is preferable to recreating the renderer failure this + // helper exists to prevent. + if (typeof createImageBitmap !== 'function' || typeof OffscreenCanvas !== 'function') { + return FALLBACK_PLACEHOLDER + } + + const task = resizeQueue.then(() => downscaleDataUrl(dataUrl, maxLongEdge)) + + resizeQueue = task.then( + () => undefined, + () => undefined + ) + + return task +} diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index f7a4fbe609..37069491c4 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -9,9 +9,10 @@ export interface ComposerAttachment { label: string detail?: string refText?: string + /** Legacy/on-demand full source. New local image chips omit this and read + * `path` only when the lightbox opens, avoiding retained multi-MB base64. */ previewUrl?: string - /** Downscaled data URL for the attachment card's pill only. Keeps the - * full-resolution `previewUrl` for lightbox/download/model bytes. */ + /** Downscaled data URL for the attachment card and optimistic bubble only. */ thumbnailUrl?: string path?: string attachedSessionId?: string