diff --git a/package.json b/package.json index 83eeed4c204..9eb8fa6eee5 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "type": "module", "scripts": { "dev": "bun run studio", - "build": "bun run --filter '@hyperframes/{parsers,lint,studio-server}' build && bun run --filter @hyperframes/core build && bun run --filter '@hyperframes/{core,engine,producer,player,studio,shader-transitions,aws-lambda,gcp-cloud-run,sdk}' build && bun run --filter @hyperframes/cli build && bun run --filter @hyperframes/sdk-playground build", + "build": "bun run --filter '@hyperframes/{parsers,lint,studio-server}' build && bun run --filter @hyperframes/core build && bun run --filter '@hyperframes/{core,engine,producer,player,studio,shader-transitions}' build && bun run --filter '@hyperframes/{aws-lambda,gcp-cloud-run,sdk}' build && bun run --filter @hyperframes/cli build && bun run --filter @hyperframes/sdk-playground build", "build:producer": "bun run --filter @hyperframes/producer build", "studio": "bun run --filter @hyperframes/studio dev", "build:hyperframes-runtime": "bun run --filter @hyperframes/core build:hyperframes-runtime", diff --git a/packages/engine/src/services/frameCapture-discardWarmup.test.ts b/packages/engine/src/services/frameCapture-discardWarmup.test.ts index 8e43fd501ec..4b2dab3e0f0 100644 --- a/packages/engine/src/services/frameCapture-discardWarmup.test.ts +++ b/packages/engine/src/services/frameCapture-discardWarmup.test.ts @@ -1,6 +1,6 @@ /** - * Tests for `discardWarmupCapture` — the helper distributed chunk workers - * run before their first real capture to prime `lastFrameCache`. + * Tests for `discardWarmupCapture` — the helper screenshot-mode parallel + * workers run before their first real capture to settle their first seek. * * The helper is a thin wrapper around the inner `captureFrameCore` * machinery, so its testable contract is post-conditional rather than @@ -19,11 +19,14 @@ * `innerCapture` for exactly this reason). We don't need a real Chrome. */ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { existsSync, readdirSync, mkdtempSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { discardWarmupCapture, type CaptureSession } from "./frameCapture.js"; +import { captureFrameToBuffer, discardWarmupCapture, type CaptureSession } from "./frameCapture.js"; +import { beginFrameCapture, pageScreenshotCapture } from "./screenshotService.js"; + +vi.mock("./screenshotService.js"); function makeFakeSession(): CaptureSession { // The discardWarmupCapture wrapper only reads `capturePerf`, @@ -44,8 +47,9 @@ function makeFakeSession(): CaptureSession { beforeCaptureMs: 50, screenshotMs: 200, totalMs: 350, + frameMs: [40, 50], }, - captureMode: "beginframe", + captureMode: "screenshot", beginFrameTimeTicks: 0, beginFrameIntervalMs: 33, beginFrameHasDamageCount: 4, @@ -53,7 +57,63 @@ function makeFakeSession(): CaptureSession { } as unknown as CaptureSession; } +function cleanupSession(session: CaptureSession): void { + rmSync(session.outputDir, { recursive: true, force: true }); +} + describe("discardWarmupCapture", () => { + it("settles and retains screenshots after a single seek", async () => { + const session = makeFakeSession(); + const evaluate = vi.fn(async () => false); + session.page = { evaluate } as unknown as CaptureSession["page"]; + vi.mocked(pageScreenshotCapture) + .mockResolvedValueOnce(Buffer.from("warmup")) + .mockResolvedValueOnce(Buffer.from("retained")); + const result = await captureFrameToBuffer(session, 36, 1.2, true); + expect(evaluate.mock.calls.filter((call) => call.length === 2)).toHaveLength(1); + expect(pageScreenshotCapture).toHaveBeenCalledTimes(2); + expect(result.buffer.toString()).toBe("retained"); + cleanupSession(session); + }); + + it("ignores settlePaint in BeginFrame mode so no extra screenshot interleaves the tick", async () => { + // The parallelCoordinator only requests settlement for screenshot workers, + // but captureFrameCore must enforce the mode itself: a throwaway + // Page.captureScreenshot interleaved with HeadlessExperimental.beginFrame + // is the duplicate-compositor-tick hazard class discardWarmupCapture guards. + const session = makeFakeSession(); + session.captureMode = "beginframe"; + const evaluate = vi.fn(async () => false); + session.page = { evaluate } as unknown as CaptureSession["page"]; + vi.mocked(pageScreenshotCapture).mockClear(); + vi.mocked(beginFrameCapture).mockResolvedValue({ + buffer: Buffer.from("beginframe"), + hasDamage: true, + }); + const result = await captureFrameToBuffer(session, 36, 1.2, true); + expect(pageScreenshotCapture).not.toHaveBeenCalled(); + expect(beginFrameCapture).toHaveBeenCalledTimes(1); + expect(result.buffer.toString()).toBe("beginframe"); + cleanupSession(session); + }); + + it("rejects BeginFrame sessions before issuing a duplicate compositor tick", async () => { + const session = makeFakeSession(); + session.captureMode = "beginframe"; + let called = false; + try { + await expect( + discardWarmupCapture(session, 36, 1.2, async () => { + called = true; + return { buffer: Buffer.alloc(0), quantizedTime: 1.2, captureTimeMs: 0 }; + }), + ).rejects.toThrow("screenshot capture mode"); + expect(called).toBe(false); + } finally { + cleanupSession(session); + } + }); + it("calls the inner capture exactly once with (frameIndex=0, time=0) by default", async () => { const session = makeFakeSession(); try { @@ -70,7 +130,7 @@ describe("discardWarmupCapture", () => { expect(receivedFrameIndex).toBe(0); expect(receivedTime).toBe(0); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -84,13 +144,13 @@ describe("discardWarmupCapture", () => { }); expect(received).toEqual({ fi: 240, t: 8 }); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); it("restores perf counters after the inner capture mutates them", async () => { const session = makeFakeSession(); - const before = { ...session.capturePerf }; + const before = { ...session.capturePerf, frameMs: [...session.capturePerf.frameMs] }; try { await discardWarmupCapture(session, 0, 0, async (s) => { s.capturePerf.frames += 1; @@ -98,11 +158,12 @@ describe("discardWarmupCapture", () => { s.capturePerf.beforeCaptureMs += 5; s.capturePerf.screenshotMs += 33; s.capturePerf.totalMs += 50; + s.capturePerf.frameMs.push(999); return { buffer: Buffer.alloc(0), quantizedTime: 0, captureTimeMs: 50 }; }); expect(session.capturePerf).toEqual(before); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -119,13 +180,13 @@ describe("discardWarmupCapture", () => { expect(session.beginFrameHasDamageCount).toBe(hasBefore); expect(session.beginFrameNoDamageCount).toBe(noBefore); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); it("restores state even when the inner capture throws", async () => { const session = makeFakeSession(); - const perfBefore = { ...session.capturePerf }; + const perfBefore = { ...session.capturePerf, frameMs: [...session.capturePerf.frameMs] }; const hasBefore = session.beginFrameHasDamageCount; const noBefore = session.beginFrameNoDamageCount; try { @@ -133,6 +194,7 @@ describe("discardWarmupCapture", () => { try { await discardWarmupCapture(session, 0, 0, async (s) => { s.capturePerf.frames += 5; + s.capturePerf.frameMs.push(999); s.beginFrameNoDamageCount += 2; throw new Error("simulated capture failure"); }); @@ -146,7 +208,7 @@ describe("discardWarmupCapture", () => { expect(session.beginFrameHasDamageCount).toBe(hasBefore); expect(session.beginFrameNoDamageCount).toBe(noBefore); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -163,7 +225,7 @@ describe("discardWarmupCapture", () => { const after = readdirSync(session.outputDir); expect(after).toEqual(before); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -177,7 +239,7 @@ describe("discardWarmupCapture", () => { })); expect(result).toBeUndefined(); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); }); diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 54a1a529d64..318d536c4bc 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -2751,6 +2751,7 @@ async function captureFrameCore( session: CaptureSession, frameIndex: number, time: number, + settlePaint: boolean = false, ): Promise<{ buffer: Buffer; quantizedTime: number; captureTimeMs: number }> { const { page, options } = session; const startTime = Date.now(); @@ -2787,6 +2788,17 @@ async function captureFrameCore( ); const screenshotStart = Date.now(); + // Paint-settlement barrier (fresh non-zero screenshot workers): the seek + // above may have scheduled rAF/compositor work that has not painted yet. + // One throwaway Page.captureScreenshot lets that work land so the retained + // screenshot below reads settled pixels — same seek, no second + // prepareFrameForCapture. Screenshot mode only, enforced here and not just + // at the call site: interleaving an extra captureScreenshot with + // BeginFrame/drawElement capture risks the duplicate-compositor-tick + // stalls documented on discardWarmupCapture. + if (settlePaint && session.captureMode === "screenshot") { + await pageScreenshotCapture(page, options); + } let screenshotBuffer: Buffer; if (session.captureMode === "beginframe") { @@ -2920,11 +2932,13 @@ export async function captureFrame( session: CaptureSession, frameIndex: number, time: number, + settlePaint: boolean = false, ): Promise { const { buffer, quantizedTime, captureTimeMs } = await captureFrameCore( session, frameIndex, time, + settlePaint, ); const framePath = writeCapturedFrame(session, frameIndex, buffer); return { frameIndex, time: quantizedTime, path: framePath, captureTimeMs }; @@ -2957,8 +2971,9 @@ export async function captureFrameToBuffer( session: CaptureSession, frameIndex: number, time: number, + settlePaint: boolean = false, ): Promise { - const { buffer, captureTimeMs } = await captureFrameCore(session, frameIndex, time); + const { buffer, captureTimeMs } = await captureFrameCore(session, frameIndex, time, settlePaint); return { buffer, captureTimeMs }; } @@ -3233,14 +3248,15 @@ export type DiscardWarmupInnerCapture = ( * side-effects (perf counters, BeginFrame damage tallies) so downstream * captures see state identical to a fresh session. * - * Distributed chunk workers need this because Chrome's BeginFrame screenshot - * pipeline maintains a per-process `lastFrameCache`: when a captured frame's - * `hasDamage` reports `false`, the screenshot path returns the previously - * captured buffer. For chunk N (N > 0) the worker has no prior frame in its - * cache, so the very first capture's `hasDamage` reporting diverges from - * what an in-process render at the same absolute frame index would see (the - * in-process renderer always has frame N-1 cached). One discard capture - * before the first real capture primes the cache. + * Screenshot-mode parallel workers use this to settle the paint scheduled by + * their first non-zero `__hf.seek()`: the throwaway Page.captureScreenshot + * produces a frame, allowing rAF/compositor work to land before the real + * capture reads the pixels. + * + * This helper is intentionally forbidden for BeginFrame and drawElement + * sessions. Repeating a BeginFrame timestamp stalls Chrome's compositor, and + * priming with an earlier frame makes the subsequent real capture move time + * backwards, which stalls for the same reason (see producer renderChunk). * * The function intentionally restores perf state so the warmup capture does * NOT bias `getCapturePerfSummary()`'s per-frame averages. @@ -3261,10 +3277,10 @@ export async function discardWarmupCapture( time: number = 0, innerCapture: DiscardWarmupInnerCapture = captureFrameCore, ): Promise { - // Snapshot the side-effect counters captureFrameCore mutates. We use a - // shallow `{...}` for capturePerf because all five fields are primitive - // numbers — no nested state to deep-copy. - const perfBefore = { ...session.capturePerf }; + if (session.captureMode !== "screenshot") { + throw new Error("discardWarmupCapture requires screenshot capture mode"); + } + const perfBefore = { ...session.capturePerf, frameMs: [...session.capturePerf.frameMs] }; const hasDamageBefore = session.beginFrameHasDamageCount; const noDamageBefore = session.beginFrameNoDamageCount; const dedupCountBefore = session.staticDedupCount; diff --git a/packages/engine/src/services/parallelCoordinator.test.ts b/packages/engine/src/services/parallelCoordinator.test.ts index 43a3159e063..0addf734e03 100644 --- a/packages/engine/src/services/parallelCoordinator.test.ts +++ b/packages/engine/src/services/parallelCoordinator.test.ts @@ -1,5 +1,19 @@ -import { describe, it, expect } from "vitest"; +import { beforeEach, describe, it, expect, vi } from "vitest"; + +const frameCaptureMocks = vi.hoisted(() => ({ + createCaptureSession: vi.fn(), + initializeSession: vi.fn(), + closeCaptureSession: vi.fn(), + captureFrame: vi.fn(), + captureFrameToBufferPipelined: vi.fn(), + captureFrameToBuffer: vi.fn(), + getCapturePerfSummary: vi.fn(), +})); + +vi.mock("./frameCapture.js", () => frameCaptureMocks); + import { + __testing, calculateOptimalWorkers, distributeFrames, formatWorkerFailure, @@ -10,6 +24,93 @@ import { } from "./parallelCoordinator.js"; import type { EngineConfig } from "../config.js"; +const { executeWorkerTask } = __testing; + +function makeWorkerSession( + captureMode: "screenshot" | "beginframe" | "drawelement" = "screenshot", +) { + return { + browserConsoleBuffer: [], + captureMode, + workerEncodeEnabled: false, + }; +} + +const captureOptions = { + width: 1280, + height: 720, + fps: { num: 30, den: 1 }, +}; + +function runWorker(workerId: number, startFrame: number, signal?: AbortSignal) { + return executeWorkerTask( + { workerId, startFrame, endFrame: startFrame + 2, outputDir: "/tmp" }, + "http://127.0.0.1:4173", + captureOptions, + () => null, + signal, + ); +} + +beforeEach(() => { + vi.resetAllMocks(); + frameCaptureMocks.createCaptureSession.mockImplementation(async () => makeWorkerSession()); + frameCaptureMocks.initializeSession.mockResolvedValue(undefined); + frameCaptureMocks.closeCaptureSession.mockResolvedValue(undefined); + frameCaptureMocks.captureFrame.mockResolvedValue(undefined); +}); + +describe("executeWorkerTask", () => { + it("settles a non-zero screenshot worker at its first absolute frame", async () => { + const callOrder: string[] = []; + frameCaptureMocks.initializeSession.mockImplementation(async () => { + callOrder.push("initialize"); + }); + frameCaptureMocks.captureFrame.mockImplementation( + async (_session, frameIndex, _time, settle) => { + callOrder.push(`capture:${frameIndex}:${settle}`); + }, + ); + + await runWorker(1, 36); + + expect(callOrder).toEqual(["initialize", "capture:36:true", "capture:37:false"]); + }); + + it("does not warm worker zero because its initial state is already painted", async () => { + const result = await runWorker(0, 0); + + expect(result.framesCaptured).toBe(2); + const settles = frameCaptureMocks.captureFrame.mock.calls.map((call) => call[3]); + expect(settles).toEqual([false, false]); + }); + + it.each(["beginframe", "drawelement"] as const)( + "does not issue a duplicate warmup capture in %s mode", + async (captureMode) => { + frameCaptureMocks.createCaptureSession.mockResolvedValue(makeWorkerSession(captureMode)); + + const result = await runWorker(1, 36); + + expect(result.error).toBeUndefined(); + const settles = frameCaptureMocks.captureFrame.mock.calls.map((call) => call[3]); + expect(settles).toEqual([false, false]); + }, + ); + + it("returns a worker error and closes the session when the paint barrier fails", async () => { + const session = makeWorkerSession(); + frameCaptureMocks.createCaptureSession.mockResolvedValue(session); + frameCaptureMocks.captureFrame.mockRejectedValue(new Error("warmup failed")); + + const result = await runWorker(1, 36); + + expect(result).toMatchObject({ error: "warmup failed", framesCaptured: 0 }); + expect(frameCaptureMocks.captureFrame).toHaveBeenCalledWith(session, 36, 1.2, true); + expect(frameCaptureMocks.closeCaptureSession).toHaveBeenCalledWith(session); + }); +}); + describe("distributeFrames", () => { it("distributes frames evenly across workers", () => { const tasks = distributeFrames(100, 4, "/tmp/work"); diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index 3fe771dde96..8e155891551 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -294,6 +294,7 @@ async function captureFrameRange( let framesCaptured = 0; const outputOffset = task.outputFrameOffset ?? 0; const stride = task.frameStride ?? 1; + const settleFirstPaint = session.captureMode === "screenshot" && task.startFrame !== 0; // Depth-2 pipelined drawElement produce (HF_DE_PARALLEL_STREAM spike): frame // k's in-page worker encode overlaps frame k+stride's produce phase — the // same shape as the sequential worker-encode loop. Only engaged when the @@ -359,12 +360,13 @@ async function captureFrameRange( if (signal?.aborted) throw new Error("Parallel worker cancelled"); const time = (i * captureOptions.fps.den) / captureOptions.fps.num; const fileFrameIdx = i - outputOffset; + const settlePaint = settleFirstPaint && i === task.startFrame; if (onFrameBuffer) { - const { buffer } = await captureFrameToBuffer(session, fileFrameIdx, time); + const { buffer } = await captureFrameToBuffer(session, fileFrameIdx, time, settlePaint); await onFrameBuffer(i, buffer, session); } else { - await captureFrame(session, fileFrameIdx, time); + await captureFrame(session, fileFrameIdx, time, settlePaint); } framesCaptured++; if (onFrameCaptured) onFrameCaptured(task.workerId, i); @@ -372,6 +374,13 @@ async function captureFrameRange( return framesCaptured; } +function logParDebug(workerId: number, message: string): void { + if (process.env.HF_DE_PAR_DEBUG === "1") console.log(`[par:w${workerId}] ${message}`); +} + +// Inherited coordinator complexity (session lifecycle + success/error/finally +// result shapes), re-flagged by the settle-paint edits shifting lines. +// fallow-ignore-next-line complexity async function executeWorkerTask( task: WorkerTask, serverUrl: string, @@ -410,19 +419,16 @@ async function executeWorkerTask( createBeforeCaptureHook(), workerConfig, ); - if (process.env.HF_DE_PAR_DEBUG === "1") { - console.log(`[par:w${task.workerId}] session created`); - } + logParDebug(task.workerId, "session created"); // Worker-0-only SwiftShader assertion — see `shouldVerifyWorkerGpu` and #955. if (shouldVerifyWorkerGpu(task.workerId, workerConfig)) { await assertSwiftShader(session.page, readWebGlVendorInfoFromCanvas); } await initializeSession(session); - if (process.env.HF_DE_PAR_DEBUG === "1") { - console.log( - `[par:w${task.workerId}] init done (mode=${session.captureMode} workerEncode=${session.workerEncodeEnabled === true})`, - ); - } + logParDebug( + task.workerId, + `init done (mode=${session.captureMode} workerEncode=${session.workerEncodeEnabled === true})`, + ); framesCaptured = await captureFrameRange( session, task, @@ -459,6 +465,8 @@ async function executeWorkerTask( } } +export const __testing = { executeWorkerTask }; + /** * drawElement self-verify sample count for multi-worker capture. Each worker * arms the same shared sample grid but drains only ~1/N of it, and N diff --git a/packages/producer/tests/webm-transparency/meta.json b/packages/producer/tests/webm-transparency/meta.json index 709f10927dd..1c298bf2fdf 100644 --- a/packages/producer/tests/webm-transparency/meta.json +++ b/packages/producer/tests/webm-transparency/meta.json @@ -9,6 +9,6 @@ "renderConfig": { "fps": 30, "format": "webm", - "workers": 1 + "workers": 4 } }