fix: discard the warmup capture frame in the parallel coordinator

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",

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,16 +47,73 @@ function makeFakeSession(): CaptureSession {
       beforeCaptureMs: 50,
       screenshotMs: 200,
       totalMs: 350,
+      frameMs: [40, 50],
     },
-    captureMode: "beginframe",
+    captureMode: "screenshot",
     beginFrameTimeTicks: 0,
     beginFrameIntervalMs: 33,
     beginFrameHasDamageCount: 4,
     beginFrameNoDamageCount: 3,
   } 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,25 +144,26 @@ 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;
         s.capturePerf.seekMs += 12;
         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,20 +180,21 @@ 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 {
       let thrown: unknown;
       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);
     }
   });
 });

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<CaptureResult> {
   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<CaptureBufferResult> {
-  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<void> {
-  // 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;