diff --git a/packages/cli/src/capture/captureCompositionFrame.ts b/packages/cli/src/capture/captureCompositionFrame.ts index a9f117af2cb..637b2049a01 100644 --- a/packages/cli/src/capture/captureCompositionFrame.ts +++ b/packages/cli/src/capture/captureCompositionFrame.ts @@ -35,11 +35,20 @@ export const AUDIT_SEEK_OPTIONS = { waitForFontsMs: 500, } as const; +export const SCREENSHOT_IMAGE_WAIT_MS = 3_000; + export const SCREENSHOT_SEEK_OPTIONS = { ...AUDIT_SEEK_OPTIONS, - waitForImagesMs: 120, + waitForImagesMs: SCREENSHOT_IMAGE_WAIT_MS, } as const; +export interface UnsettledImages { + pending: string[]; + failed: string[]; +} + +const ALL_IMAGES_SETTLED: UnsettledImages = { pending: [], failed: [] }; + // Geometry-only seek for the dense content_overlap grid: getBoundingClientRect is valid synchronously after setTime, so drop all post-seek waits (rAF/font/sleep) that would multiply across the dense grid. export const DENSE_GEOMETRY_SEEK_OPTIONS = { ...AUDIT_SEEK_OPTIONS, @@ -541,7 +550,7 @@ export async function seekCompositionTimelineOnly( export async function settleCompositionSeek( page: CompositionSeekPage, options: SeekCompositionTimelineOptions = {}, -): Promise { +): Promise { await page.evaluate( async (barrier: SeekBarrierOptions) => { const waitForCompletion = Reflect.get(window, "__hfWaitForSeekCompletion"); @@ -577,11 +586,14 @@ export async function settleCompositionSeek( if (waitForFontsMs > 0) { await waitForCompositionFonts(page, waitForFontsMs, animationFrameSettle === "double" ? 2 : 1); } - if (options.waitForImagesMs) await waitForCompositionImages(page, options.waitForImagesMs); + const images = options.waitForImagesMs + ? await waitForCompositionImages(page, options.waitForImagesMs) + : ALL_IMAGES_SETTLED; if (options.settleMs !== undefined) { const settleMs = Math.max(0, options.settleMs); await new Promise((resolveSettle) => setTimeout(resolveSettle, settleMs)); } + return images; } export async function waitForPreferredSeekTarget( @@ -609,23 +621,58 @@ export async function waitForPreferredSeekTarget( } } +/** Waits, up to `timeoutMs`, for every visible image layer (, SVG , CSS background-image url) to decode. */ async function waitForCompositionImages( page: CompositionEvaluationPage, timeoutMs: number, -): Promise { - await page.evaluate( - (ms: number) => - new Promise((resolveWait) => { - const deadline = setTimeout(resolveWait, ms); - void Promise.all( - Array.from(document.images, (image) => image.decode().catch(() => {})), - ).then(() => { - clearTimeout(deadline); - resolveWait(); - }); - }), +): Promise { + const unsettled = await page.evaluate( + // fallow-ignore-next-line complexity + async (ms: number) => { + const layers = new Map(); + const add = (src: string, image?: HTMLImageElement): void => { + if (src && !layers.has(src)) layers.set(src, image ?? Object.assign(new Image(), { src })); + }; + const absolute = (href: string): string => new URL(href, document.baseURI).href; + for (const element of document.querySelectorAll("*")) { + const style = getComputedStyle(element); + if (style.display === "none" || style.visibility === "hidden") continue; + if (element instanceof HTMLImageElement) add(element.currentSrc || element.src, element); + if (element instanceof SVGImageElement && element.href.baseVal) { + add(absolute(element.href.baseVal)); + } + for (const match of style.backgroundImage.matchAll( + /url\(\s*(?:"([^"]*)"|'([^']*)'|([^)]*))\s*\)/g, + )) { + const href = match[1] ?? match[2] ?? match[3]; + if (href) add(absolute(href)); + } + } + const state = new Map(); + const decodes = [...layers].map(([src, image]) => { + state.set(src, "pending"); + return image.decode().then( + () => void state.set(src, "ready"), + () => void state.set(src, "failed"), + ); + }); + let timer: ReturnType | undefined; + await Promise.race([ + Promise.all(decodes), + new Promise((resolveWait) => { + timer = setTimeout(resolveWait, ms); + }), + ]); + clearTimeout(timer); + const named = (want: string): string[] => + [...state] + .filter(([, value]) => value === want) + .map(([src]) => (src.startsWith("data:") ? `${src.slice(0, 40)}...` : src)); + return { pending: named("pending"), failed: named("failed") }; + }, timeoutMs, ); + return unsettled as UnsettledImages; } export async function waitForCompositionFonts( diff --git a/packages/cli/src/commands/compare.ts b/packages/cli/src/commands/compare.ts index fac16738b73..d7d7d38383d 100644 --- a/packages/cli/src/commands/compare.ts +++ b/packages/cli/src/commands/compare.ts @@ -6,6 +6,7 @@ import { defineCommand } from "citty"; import { createContactSheet } from "../capture/contactSheet.js"; import { openSettledCompositionPage, + SCREENSHOT_IMAGE_WAIT_MS, seekCompositionTimeline, } from "../capture/captureCompositionFrame.js"; import { c } from "../ui/colors.js"; @@ -285,7 +286,9 @@ async function renderCompareVariant( }); try { if (opts.atSeconds > 0) { - await seekCompositionTimeline(page, opts.atSeconds); + await seekCompositionTimeline(page, opts.atSeconds, { + waitForImagesMs: SCREENSHOT_IMAGE_WAIT_MS, + }); } await page.screenshot({ path: opts.framePath, type: "png" }); return { framePath: opts.framePath, renderReadyTimedOut }; diff --git a/packages/cli/src/commands/snapshot-contactSheet.test.ts b/packages/cli/src/commands/snapshot-contactSheet.test.ts index de1114f8c21..61e46a9675a 100644 --- a/packages/cli/src/commands/snapshot-contactSheet.test.ts +++ b/packages/cli/src/commands/snapshot-contactSheet.test.ts @@ -32,7 +32,7 @@ vi.mock("../capture/captureCompositionFrame.js", async (importOriginal) => ({ })), seekCompositionTimeline: capture.seek, seekCompositionTimelineOnly: capture.seek, - settleCompositionSeek: vi.fn(async () => undefined), + settleCompositionSeek: vi.fn(async () => ({ pending: [], failed: [] })), })); vi.mock("../utils/staticProjectServer.js", () => ({ diff --git a/packages/cli/src/commands/snapshot.lateImages.browser.test.ts b/packages/cli/src/commands/snapshot.lateImages.browser.test.ts new file mode 100644 index 00000000000..3f855489786 --- /dev/null +++ b/packages/cli/src/commands/snapshot.lateImages.browser.test.ts @@ -0,0 +1,136 @@ +import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { createServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import sharp from "sharp"; +import { afterAll, beforeAll, expect, it, vi } from "vitest"; +import snapshotCommand from "./snapshot.js"; + +// One layer per test (an img, a CSS background or an SVG image), revealed by the seek and served slowly. +// Each kind must be waited for on its own, so a screenshot taken right after the seek settles shows it blank. +const LATE_MS = 800; +const COLORS = { red: [255, 0, 0], green: [0, 255, 0], blue: [0, 0, 255] } as const; + +let server: Server; +let base: string; +let root: string; +const pngs: Record = {}; + +beforeAll(async () => { + for (const [name, [r, g, b]] of Object.entries(COLORS)) { + pngs[name] = await sharp({ + create: { width: 4, height: 4, channels: 3, background: { r, g, b } }, + }) + .png() + .toBuffer(); + } + server = createServer((req, res) => { + const name = (req.url ?? "").replace(/^\/|\.png$/g, ""); + if (name === "hang") return; // never answers + const png = pngs[name]; + if (!png) return void res.writeHead(404).end(); + setTimeout(() => res.writeHead(200, { "Content-Type": "image/png" }).end(png), LATE_MS); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + base = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; + root = mkdtempSync(join(tmpdir(), "hf-snapshot-late-images-")); +}); + +afterAll(() => { + server.closeAllConnections(); + server.close(); + rmSync(root, { recursive: true, force: true }); +}); + +function writeProject(name: string, reveal: string): string { + const dir = join(root, name); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "index.html"), + ` +
+ +
+ +
+`, + ); + return dir; +} + +async function snapshotAt(dir: string, time: string) { + const output = join(dir, "out"); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + try { + await snapshotCommand.run?.({ + args: { + dir, + at: time, + end: false, + json: true, + output, + describe: "false", + "browser-gpu": false, + }, + } as never); + const envelope = JSON.parse(String(log.mock.calls.at(-1)?.[0])); + return { envelope, output }; + } finally { + vi.restoreAllMocks(); + } +} + +async function pixel(file: string, x: number, y: number): Promise { + const { data, info } = await sharp(file) + .removeAlpha() + .raw() + .toBuffer({ resolveWithObject: true }); + const o = (y * info.width + x) * info.channels; + return [data[o]!, data[o + 1]!, data[o + 2]!]; +} + +it.each([ + ["an ", `document.getElementById("a").src = "BASE/red.png";`, 50, [255, 0, 0]], + [ + "a CSS background-image", + `document.getElementById("b").style.backgroundImage = "url(BASE/green.png)";`, + 160, + [0, 255, 0], + ], + [ + "an SVG ", + `document.getElementById("c").setAttribute("href", "BASE/blue.png");`, + 270, + [0, 0, 255], + ], +])( + "shows %s that the seek reveals and that loads late", + async (kind, reveal, x, color) => { + const dir = writeProject(`late-${x}`, reveal.replaceAll("BASE", base)); + + const { envelope } = await snapshotAt(dir, "1.5"); + + expect(await pixel(envelope.frames[0].path as string, x, 90)).toEqual(color); + expect(envelope.warnings ?? []).toEqual([]); + }, + 120_000, +); + +it("names an image layer that has not loaded by the cap in the warnings of the --json envelope", async () => { + const dir = writeProject("hung", `document.getElementById("a").src = "${base}/hang.png";`); + + const { envelope, output } = await snapshotAt(dir, "1.5"); + + expect(readdirSync(output).some((f) => f.endsWith(".png"))).toBe(true); + expect(envelope.warnings).toEqual([expect.stringContaining(`${base}/hang.png`)]); +}, 120_000); diff --git a/packages/cli/src/commands/snapshot.test.ts b/packages/cli/src/commands/snapshot.test.ts index 2cb8a62fef6..04427afe4a8 100644 --- a/packages/cli/src/commands/snapshot.test.ts +++ b/packages/cli/src/commands/snapshot.test.ts @@ -14,7 +14,7 @@ const snapshotState = vi.hoisted(() => ({ seek: vi.fn(async (_page: unknown, _time: number, _options?: unknown): Promise => { throw new Error("seek reached"); }), - settle: vi.fn(async (_page: unknown, _options?: unknown): Promise => undefined), + settle: vi.fn(async (_page: unknown, _options?: unknown) => ({ pending: [], failed: [] })), closeServer: vi.fn(async () => undefined), })); @@ -183,9 +183,11 @@ describe("snapshot --at seeks", () => { args: { dir: project, at: "19.019018", end: false, output: join(project, "out") }, } as never), ).rejects.toBeDefined(); - expect(snapshotState.seek).toHaveBeenCalledWith(expect.anything(), 19.019018, { - exactTime: true, - }); + expect(snapshotState.seek).toHaveBeenCalledWith( + expect.anything(), + 19.019018, + expect.objectContaining({ exactTime: true }), + ); expect(close).toHaveBeenCalled(); } finally { log.mockRestore(); @@ -228,6 +230,7 @@ describe("snapshot video overlays and the blurred frame", () => { snapshotState.settle.mockClear(); snapshotState.settle.mockImplementation(async () => { log.push("settle"); + return { pending: [], failed: [] }; }); const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts index 311c3cd184b..170f00dfcca 100644 --- a/packages/cli/src/commands/snapshot.ts +++ b/packages/cli/src/commands/snapshot.ts @@ -31,8 +31,10 @@ import { parseZoomTarget, resolveCropRegion, runFfmpegOnce, + SCREENSHOT_IMAGE_WAIT_MS, seekCompositionTimelineOnly, settleCompositionSeek, + type UnsettledImages, type ZoomTarget, } from "../capture/captureCompositionFrame.js"; import { @@ -608,7 +610,7 @@ async function captureSnapshots( }); if (duration <= 0 && !opts.at?.length) { - return { paths: [] }; + return { paths: [], warnings }; } const readTiming = () => @@ -749,7 +751,7 @@ async function captureSnapshots( for (let i = 0; i < positions.length; i++) { const time = positions[i]!; - const seekOptions = { exactTime: true }; + const seekOptions = { exactTime: true, waitForImagesMs: SCREENSHOT_IMAGE_WAIT_MS }; await seekCompositionTimelineOnly(page, time, seekOptions); if (cameraExpr) await page.evaluate(cameraExpr); @@ -894,7 +896,7 @@ async function captureSnapshots( } } - await settleCompositionSeek(page, seekOptions); + warnUnsettledImages(await settleCompositionSeek(page, seekOptions), time, warnings); await recaptureSnapshotComposite(page); const timeLabel = formatSnapshotTimestamp(time); @@ -1008,6 +1010,7 @@ async function captureSnapshots( return { paths: savedPaths, + warnings, ...(review && reviewList ? { review: { @@ -1025,9 +1028,24 @@ async function captureSnapshots( interface CapturedSnapshots { /** Project-relative where possible, as printed. */ paths: string[]; + /** Why a frame may differ from the composition: shown on the console and in `--json`. */ + warnings: string[]; review?: { plan: ReviewPlan; frames: Array }; } +function warnUnsettledImages(images: UnsettledImages, time: number, warnings: string[]): void { + const groups = [ + [images.pending, `did not finish loading within ${SCREENSHOT_IMAGE_WAIT_MS} ms`], + [images.failed, "failed to load"], + ] as const; + for (const [sources, what] of groups) { + if (sources.length === 0) continue; + const message = `${sources.length} image(s) ${what} at ${time}s, so the frame may be missing them: ${sources.join(", ")}`; + warnings.push(message); + console.warn(` ${c.warn("⚠")} ${message}`); + } +} + function warnSnapshotsNotRecorded(error: unknown): undefined { console.warn(` ${c.warn("⚠")} Snapshots were not recorded: ${normalizeErrorMessage(error)}`); return undefined; @@ -1080,6 +1098,7 @@ function snapshotEnvelope( outputDir, frames, contactSheets: extra.contactSheets.map((sheet) => resolve(sheet)), + ...(captured.warnings.length > 0 ? { warnings: captured.warnings } : {}), ...(extra.describedPath ? { descriptions: extra.describedPath } : {}), ...(review ? {