From 82803ad95598a9e690817f5f8162ac095178f2fa Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:11:12 -0700 Subject: [PATCH 1/7] fix: discard the warmup capture frame in the parallel coordinator --- .../src/services/parallelCoordinator.test.ts | 108 +++++++++++++++++- .../src/services/parallelCoordinator.ts | 4 + 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/services/parallelCoordinator.test.ts b/packages/engine/src/services/parallelCoordinator.test.ts index 43a3159e063..9e8386b226d 100644 --- a/packages/engine/src/services/parallelCoordinator.test.ts +++ b/packages/engine/src/services/parallelCoordinator.test.ts @@ -1,5 +1,20 @@ -import { describe, it, expect } from "vitest"; +import { beforeEach, describe, it, expect, vi } from "vitest"; + +const frameCaptureMocks = vi.hoisted(() => ({ + createCaptureSession: vi.fn(), + initializeSession: vi.fn(), + discardWarmupCapture: 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 +25,97 @@ import { } from "./parallelCoordinator.js"; import type { EngineConfig } from "../config.js"; +const { executeWorkerTask } = __testing; + +function makeWorkerSession() { + return { + browserConsoleBuffer: [], + captureMode: "screenshot", + 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.discardWarmupCapture.mockResolvedValue(undefined); + frameCaptureMocks.closeCaptureSession.mockResolvedValue(undefined); + frameCaptureMocks.captureFrame.mockResolvedValue(undefined); +}); + +describe("executeWorkerTask", () => { + it("warms a non-zero worker after initialization and before its first frame", async () => { + const callOrder: string[] = []; + frameCaptureMocks.initializeSession.mockImplementation(async () => { + callOrder.push("initialize"); + }); + frameCaptureMocks.discardWarmupCapture.mockImplementation(async () => { + callOrder.push("warmup"); + }); + frameCaptureMocks.captureFrame.mockImplementation(async (_session, frameIndex) => { + callOrder.push(`capture:${frameIndex}`); + }); + + const result = await runWorker(1, 36); + + expect(result.error).toBeUndefined(); + expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledOnce(); + expect(callOrder).toEqual(["initialize", "warmup", "capture:36", "capture:37"]); + }); + + it("warms every worker without changing its reported frame count", async () => { + const results = await Promise.all([runWorker(0, 0), runWorker(1, 36)]); + + expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledTimes(2); + expect(results.map((result) => result.framesCaptured)).toEqual([2, 2]); + expect(frameCaptureMocks.captureFrame).toHaveBeenCalledTimes(4); + }); + + it("returns a worker error and closes the session when warmup fails", async () => { + const session = makeWorkerSession(); + frameCaptureMocks.createCaptureSession.mockResolvedValue(session); + frameCaptureMocks.discardWarmupCapture.mockRejectedValue(new Error("warmup failed")); + + const result = await runWorker(1, 36); + + expect(result).toMatchObject({ error: "warmup failed", framesCaptured: 0 }); + expect(frameCaptureMocks.captureFrame).not.toHaveBeenCalled(); + expect(frameCaptureMocks.closeCaptureSession).toHaveBeenCalledWith(session); + }); + + it("returns a cancellation error and closes the session when aborted during warmup", async () => { + const controller = new AbortController(); + const session = makeWorkerSession(); + frameCaptureMocks.createCaptureSession.mockResolvedValue(session); + frameCaptureMocks.discardWarmupCapture.mockImplementation(async () => { + controller.abort(); + }); + + const result = await runWorker(0, 0, controller.signal); + + expect(result).toMatchObject({ error: "Parallel worker cancelled", framesCaptured: 0 }); + expect(frameCaptureMocks.captureFrame).not.toHaveBeenCalled(); + 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..c9c89557a13 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -13,6 +13,7 @@ import { join } from "path"; import { createCaptureSession, initializeSession, + discardWarmupCapture, closeCaptureSession, captureFrame, captureFrameToBufferPipelined, @@ -418,6 +419,7 @@ async function executeWorkerTask( await assertSwiftShader(session.page, readWebGlVendorInfoFromCanvas); } await initializeSession(session); + await discardWarmupCapture(session); if (process.env.HF_DE_PAR_DEBUG === "1") { console.log( `[par:w${task.workerId}] init done (mode=${session.captureMode} workerEncode=${session.workerEncodeEnabled === true})`, @@ -459,6 +461,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 From 121a02e0ec9aa69c57e1fbf5e14759ef0ad8cf46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 15 Jul 2026 17:43:10 +0000 Subject: [PATCH 2/7] fix(engine): settle screenshot worker boundaries safely --- .../frameCapture-discardWarmup.test.ts | 41 ++++++++++++++----- packages/engine/src/services/frameCapture.ts | 20 +++++---- .../src/services/parallelCoordinator.test.ts | 29 +++++++++---- .../src/services/parallelCoordinator.ts | 22 +++++++++- .../tests/webm-transparency/meta.json | 2 +- 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/packages/engine/src/services/frameCapture-discardWarmup.test.ts b/packages/engine/src/services/frameCapture-discardWarmup.test.ts index 8e43fd501ec..461cc843980 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 @@ -45,7 +45,7 @@ function makeFakeSession(): CaptureSession { screenshotMs: 200, totalMs: 350, }, - captureMode: "beginframe", + captureMode: "screenshot", beginFrameTimeTicks: 0, beginFrameIntervalMs: 33, beginFrameHasDamageCount: 4, @@ -53,7 +53,28 @@ function makeFakeSession(): CaptureSession { } as unknown as CaptureSession; } +function cleanupSession(session: CaptureSession): void { + rmSync(session.outputDir, { recursive: true, force: true }); +} + describe("discardWarmupCapture", () => { + 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 +91,7 @@ describe("discardWarmupCapture", () => { expect(receivedFrameIndex).toBe(0); expect(receivedTime).toBe(0); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -84,7 +105,7 @@ describe("discardWarmupCapture", () => { }); expect(received).toEqual({ fi: 240, t: 8 }); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -102,7 +123,7 @@ describe("discardWarmupCapture", () => { }); expect(session.capturePerf).toEqual(before); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -119,7 +140,7 @@ describe("discardWarmupCapture", () => { expect(session.beginFrameHasDamageCount).toBe(hasBefore); expect(session.beginFrameNoDamageCount).toBe(noBefore); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -146,7 +167,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 +184,7 @@ describe("discardWarmupCapture", () => { const after = readdirSync(session.outputDir); expect(after).toEqual(before); } finally { - rmSync(session.outputDir, { recursive: true, force: true }); + cleanupSession(session); } }); @@ -177,7 +198,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..2a8c3bd9880 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -3233,14 +3233,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,6 +3262,9 @@ export async function discardWarmupCapture( time: number = 0, innerCapture: DiscardWarmupInnerCapture = captureFrameCore, ): Promise { + if (session.captureMode !== "screenshot") { + throw new Error("discardWarmupCapture requires screenshot capture mode"); + } // 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. diff --git a/packages/engine/src/services/parallelCoordinator.test.ts b/packages/engine/src/services/parallelCoordinator.test.ts index 9e8386b226d..5600404af3a 100644 --- a/packages/engine/src/services/parallelCoordinator.test.ts +++ b/packages/engine/src/services/parallelCoordinator.test.ts @@ -27,10 +27,12 @@ import type { EngineConfig } from "../config.js"; const { executeWorkerTask } = __testing; -function makeWorkerSession() { +function makeWorkerSession( + captureMode: "screenshot" | "beginframe" | "drawelement" = "screenshot", +) { return { browserConsoleBuffer: [], - captureMode: "screenshot", + captureMode, workerEncodeEnabled: false, }; } @@ -61,7 +63,7 @@ beforeEach(() => { }); describe("executeWorkerTask", () => { - it("warms a non-zero worker after initialization and before its first frame", async () => { + it("settles a non-zero screenshot worker at its first absolute frame", async () => { const callOrder: string[] = []; frameCaptureMocks.initializeSession.mockImplementation(async () => { callOrder.push("initialize"); @@ -76,18 +78,31 @@ describe("executeWorkerTask", () => { const result = await runWorker(1, 36); expect(result.error).toBeUndefined(); - expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledOnce(); + expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledWith(expect.anything(), 36, 1.2); expect(callOrder).toEqual(["initialize", "warmup", "capture:36", "capture:37"]); }); - it("warms every worker without changing its reported frame count", async () => { + it("does not warm worker zero because its initial state is already painted", async () => { const results = await Promise.all([runWorker(0, 0), runWorker(1, 36)]); - expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledTimes(2); + expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledTimes(1); expect(results.map((result) => result.framesCaptured)).toEqual([2, 2]); expect(frameCaptureMocks.captureFrame).toHaveBeenCalledTimes(4); }); + 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(); + expect(frameCaptureMocks.discardWarmupCapture).not.toHaveBeenCalled(); + expect(frameCaptureMocks.captureFrame).toHaveBeenCalledTimes(2); + }, + ); + it("returns a worker error and closes the session when warmup fails", async () => { const session = makeWorkerSession(); frameCaptureMocks.createCaptureSession.mockResolvedValue(session); @@ -108,7 +123,7 @@ describe("executeWorkerTask", () => { controller.abort(); }); - const result = await runWorker(0, 0, controller.signal); + const result = await runWorker(1, 36, controller.signal); expect(result).toMatchObject({ error: "Parallel worker cancelled", framesCaptured: 0 }); expect(frameCaptureMocks.captureFrame).not.toHaveBeenCalled(); diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index c9c89557a13..3d450ec34ed 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -419,7 +419,7 @@ async function executeWorkerTask( await assertSwiftShader(session.page, readWebGlVendorInfoFromCanvas); } await initializeSession(session); - await discardWarmupCapture(session); + await settleFirstScreenshotFrame(session, task, captureOptions); if (process.env.HF_DE_PAR_DEBUG === "1") { console.log( `[par:w${task.workerId}] init done (mode=${session.captureMode} workerEncode=${session.workerEncodeEnabled === true})`, @@ -461,6 +461,26 @@ async function executeWorkerTask( } } +async function settleFirstScreenshotFrame( + session: CaptureSession, + task: WorkerTask, + captureOptions: CaptureOptions, +): Promise { + // Screenshot capture is driven by Page.captureScreenshot rather than + // HeadlessExperimental.beginFrame. On a fresh non-zero worker, the first + // screenshot can race the paint scheduled by __hf.seek(startTime); issue + // one throwaway screenshot at that exact frame so the real capture sees + // the settled state. Worker 0 already starts from the initially-painted + // frame, and BeginFrame/drawElement sessions must never be double-captured + // at the same compositor tick (Chrome stalls on duplicate timestamps). + if (session.captureMode !== "screenshot" || task.startFrame === 0) return; + + const outputOffset = task.outputFrameOffset ?? 0; + const firstFrameIndex = task.startFrame - outputOffset; + const firstTime = (task.startFrame * captureOptions.fps.den) / captureOptions.fps.num; + await discardWarmupCapture(session, firstFrameIndex, firstTime); +} + export const __testing = { executeWorkerTask }; /** 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 } } From 04b68b2a235e82461d772772e835be5e10ffe46b Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:08:58 -0700 Subject: [PATCH 3/7] fix(engine): drop duplicate warmup seek before parallel capture reads discardWarmupCapture no longer runs a full captureFrameCore: the warmup settles the seek once and the real capture reads pixels without issuing a second seek at the same boundary. Test expectations updated. --- .../frameCapture-discardWarmup.test.ts | 25 +++++++-- packages/engine/src/services/frameCapture.ts | 12 +++-- .../src/services/parallelCoordinator.test.ts | 52 ++++++------------- .../src/services/parallelCoordinator.ts | 28 ++-------- 4 files changed, 49 insertions(+), 68 deletions(-) diff --git a/packages/engine/src/services/frameCapture-discardWarmup.test.ts b/packages/engine/src/services/frameCapture-discardWarmup.test.ts index 461cc843980..e259996fbd0 100644 --- a/packages/engine/src/services/frameCapture-discardWarmup.test.ts +++ b/packages/engine/src/services/frameCapture-discardWarmup.test.ts @@ -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 { pageScreenshotCapture } from "./screenshotService.js"; + +vi.mock("./screenshotService.js"); function makeFakeSession(): CaptureSession { // The discardWarmupCapture wrapper only reads `capturePerf`, @@ -44,6 +47,7 @@ function makeFakeSession(): CaptureSession { beforeCaptureMs: 50, screenshotMs: 200, totalMs: 350, + frameMs: [40, 50], }, captureMode: "screenshot", beginFrameTimeTicks: 0, @@ -58,6 +62,20 @@ function cleanupSession(session: CaptureSession): void { } 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("rejects BeginFrame sessions before issuing a duplicate compositor tick", async () => { const session = makeFakeSession(); session.captureMode = "beginframe"; @@ -111,7 +129,7 @@ describe("discardWarmupCapture", () => { 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; @@ -119,6 +137,7 @@ 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); diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 2a8c3bd9880..ef536b186ee 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,7 @@ async function captureFrameCore( ); const screenshotStart = Date.now(); + if (settlePaint) await pageScreenshotCapture(page, options); let screenshotBuffer: Buffer; if (session.captureMode === "beginframe") { @@ -2920,11 +2922,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 +2961,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 }; } @@ -3265,10 +3270,7 @@ export async function discardWarmupCapture( if (session.captureMode !== "screenshot") { throw new Error("discardWarmupCapture requires screenshot capture mode"); } - // 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 }; + 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 5600404af3a..0addf734e03 100644 --- a/packages/engine/src/services/parallelCoordinator.test.ts +++ b/packages/engine/src/services/parallelCoordinator.test.ts @@ -3,7 +3,6 @@ import { beforeEach, describe, it, expect, vi } from "vitest"; const frameCaptureMocks = vi.hoisted(() => ({ createCaptureSession: vi.fn(), initializeSession: vi.fn(), - discardWarmupCapture: vi.fn(), closeCaptureSession: vi.fn(), captureFrame: vi.fn(), captureFrameToBufferPipelined: vi.fn(), @@ -57,7 +56,6 @@ beforeEach(() => { vi.resetAllMocks(); frameCaptureMocks.createCaptureSession.mockImplementation(async () => makeWorkerSession()); frameCaptureMocks.initializeSession.mockResolvedValue(undefined); - frameCaptureMocks.discardWarmupCapture.mockResolvedValue(undefined); frameCaptureMocks.closeCaptureSession.mockResolvedValue(undefined); frameCaptureMocks.captureFrame.mockResolvedValue(undefined); }); @@ -68,26 +66,23 @@ describe("executeWorkerTask", () => { frameCaptureMocks.initializeSession.mockImplementation(async () => { callOrder.push("initialize"); }); - frameCaptureMocks.discardWarmupCapture.mockImplementation(async () => { - callOrder.push("warmup"); - }); - frameCaptureMocks.captureFrame.mockImplementation(async (_session, frameIndex) => { - callOrder.push(`capture:${frameIndex}`); - }); + frameCaptureMocks.captureFrame.mockImplementation( + async (_session, frameIndex, _time, settle) => { + callOrder.push(`capture:${frameIndex}:${settle}`); + }, + ); - const result = await runWorker(1, 36); + await runWorker(1, 36); - expect(result.error).toBeUndefined(); - expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledWith(expect.anything(), 36, 1.2); - expect(callOrder).toEqual(["initialize", "warmup", "capture:36", "capture:37"]); + 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 results = await Promise.all([runWorker(0, 0), runWorker(1, 36)]); + const result = await runWorker(0, 0); - expect(frameCaptureMocks.discardWarmupCapture).toHaveBeenCalledTimes(1); - expect(results.map((result) => result.framesCaptured)).toEqual([2, 2]); - expect(frameCaptureMocks.captureFrame).toHaveBeenCalledTimes(4); + 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)( @@ -98,35 +93,20 @@ describe("executeWorkerTask", () => { const result = await runWorker(1, 36); expect(result.error).toBeUndefined(); - expect(frameCaptureMocks.discardWarmupCapture).not.toHaveBeenCalled(); - expect(frameCaptureMocks.captureFrame).toHaveBeenCalledTimes(2); + 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 warmup fails", async () => { + it("returns a worker error and closes the session when the paint barrier fails", async () => { const session = makeWorkerSession(); frameCaptureMocks.createCaptureSession.mockResolvedValue(session); - frameCaptureMocks.discardWarmupCapture.mockRejectedValue(new Error("warmup failed")); + frameCaptureMocks.captureFrame.mockRejectedValue(new Error("warmup failed")); const result = await runWorker(1, 36); expect(result).toMatchObject({ error: "warmup failed", framesCaptured: 0 }); - expect(frameCaptureMocks.captureFrame).not.toHaveBeenCalled(); - expect(frameCaptureMocks.closeCaptureSession).toHaveBeenCalledWith(session); - }); - - it("returns a cancellation error and closes the session when aborted during warmup", async () => { - const controller = new AbortController(); - const session = makeWorkerSession(); - frameCaptureMocks.createCaptureSession.mockResolvedValue(session); - frameCaptureMocks.discardWarmupCapture.mockImplementation(async () => { - controller.abort(); - }); - - const result = await runWorker(1, 36, controller.signal); - - expect(result).toMatchObject({ error: "Parallel worker cancelled", framesCaptured: 0 }); - expect(frameCaptureMocks.captureFrame).not.toHaveBeenCalled(); + expect(frameCaptureMocks.captureFrame).toHaveBeenCalledWith(session, 36, 1.2, true); expect(frameCaptureMocks.closeCaptureSession).toHaveBeenCalledWith(session); }); }); diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index 3d450ec34ed..4bbdfa2b243 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -13,7 +13,6 @@ import { join } from "path"; import { createCaptureSession, initializeSession, - discardWarmupCapture, closeCaptureSession, captureFrame, captureFrameToBufferPipelined, @@ -295,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 @@ -360,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); @@ -419,7 +420,6 @@ async function executeWorkerTask( await assertSwiftShader(session.page, readWebGlVendorInfoFromCanvas); } await initializeSession(session); - await settleFirstScreenshotFrame(session, task, captureOptions); if (process.env.HF_DE_PAR_DEBUG === "1") { console.log( `[par:w${task.workerId}] init done (mode=${session.captureMode} workerEncode=${session.workerEncodeEnabled === true})`, @@ -461,26 +461,6 @@ async function executeWorkerTask( } } -async function settleFirstScreenshotFrame( - session: CaptureSession, - task: WorkerTask, - captureOptions: CaptureOptions, -): Promise { - // Screenshot capture is driven by Page.captureScreenshot rather than - // HeadlessExperimental.beginFrame. On a fresh non-zero worker, the first - // screenshot can race the paint scheduled by __hf.seek(startTime); issue - // one throwaway screenshot at that exact frame so the real capture sees - // the settled state. Worker 0 already starts from the initially-painted - // frame, and BeginFrame/drawElement sessions must never be double-captured - // at the same compositor tick (Chrome stalls on duplicate timestamps). - if (session.captureMode !== "screenshot" || task.startFrame === 0) return; - - const outputOffset = task.outputFrameOffset ?? 0; - const firstFrameIndex = task.startFrame - outputOffset; - const firstTime = (task.startFrame * captureOptions.fps.den) / captureOptions.fps.num; - await discardWarmupCapture(session, firstFrameIndex, firstTime); -} - export const __testing = { executeWorkerTask }; /** From 08b2c236a00abd4c519ced0a197096872e441fc3 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 6 Sep 2026 19:46:50 -0700 Subject: [PATCH 4/7] test(engine): make discardWarmup throw-path assertion able to fail perfBefore shared the frameMs array by reference, so a mutation inside the throwing capture also mutated the snapshot and the restoration assertion could never fail. Deep-copy frameMs and push a sentinel in the failing capture so the test actually pins frameMs restoration. --- .../engine/src/services/frameCapture-discardWarmup.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/services/frameCapture-discardWarmup.test.ts b/packages/engine/src/services/frameCapture-discardWarmup.test.ts index e259996fbd0..4304ddcf314 100644 --- a/packages/engine/src/services/frameCapture-discardWarmup.test.ts +++ b/packages/engine/src/services/frameCapture-discardWarmup.test.ts @@ -165,7 +165,7 @@ describe("discardWarmupCapture", () => { 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 { @@ -173,6 +173,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"); }); From 3d3bf61600023589881f52d4d94821238a884d96 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 04:20:53 +0000 Subject: [PATCH 5/7] build: sequence producer-dependent adapter builds after the main package group bun run --filter cannot topologically order the workspace because of the producer<->aws-lambda and core<->parsers dependency cycles, so aws-lambda and gcp-cloud-run raced the concurrent producer/core rebuild inside the same filter group. On low-core machines the adapters' tsc reads producer's dist mid-rewrite and fails with a spurious TS2322. Building the adapters in their own group after producer finishes makes bun run build deterministic. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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", From bfe64d6ca9edc3acb19ea9e772fc038289e2661b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 04:22:24 +0000 Subject: [PATCH 6/7] fix(engine): enforce the screenshot-only settle-paint barrier inside captureFrameCore The parallelCoordinator already requests paint settlement only for fresh non-zero screenshot workers, but captureFrameCore itself would have issued the throwaway Page.captureScreenshot in any capture mode if the flag ever leaked. Interleaving an extra screenshot with BeginFrame/drawElement capture is the duplicate-compositor-tick hazard class discardWarmupCapture guards against, so the mode check now lives at the barrier itself. Added a regression test pinning that a BeginFrame session ignores settlePaint. --- .../frameCapture-discardWarmup.test.ts | 23 ++++++++++++++++++- packages/engine/src/services/frameCapture.ts | 12 +++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/engine/src/services/frameCapture-discardWarmup.test.ts b/packages/engine/src/services/frameCapture-discardWarmup.test.ts index 4304ddcf314..4b2dab3e0f0 100644 --- a/packages/engine/src/services/frameCapture-discardWarmup.test.ts +++ b/packages/engine/src/services/frameCapture-discardWarmup.test.ts @@ -24,7 +24,7 @@ import { existsSync, readdirSync, mkdtempSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { captureFrameToBuffer, discardWarmupCapture, type CaptureSession } from "./frameCapture.js"; -import { pageScreenshotCapture } from "./screenshotService.js"; +import { beginFrameCapture, pageScreenshotCapture } from "./screenshotService.js"; vi.mock("./screenshotService.js"); @@ -76,6 +76,27 @@ describe("discardWarmupCapture", () => { 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"; diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index ef536b186ee..318d536c4bc 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -2788,7 +2788,17 @@ async function captureFrameCore( ); const screenshotStart = Date.now(); - if (settlePaint) await pageScreenshotCapture(page, options); + // 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") { From b028b5e061ddfed68e885311d2b5c49ee6b6a26c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 04:23:58 +0000 Subject: [PATCH 7/7] refactor(engine): extract worker debug logging and exempt inherited coordinator complexity The settle-paint edits shifted lines in executeWorkerTask, which made fallow re-flag the coordinator's inherited complexity as new. Extract the repeated HF_DE_PAR_DEBUG log guards into logParDebug (a real two-branch reduction) and mark the remaining session-lifecycle branching with the same fallow-ignore-next-line convention captureFrameRange already uses. --- .../src/services/parallelCoordinator.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index 4bbdfa2b243..8e155891551 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -374,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, @@ -412,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,