Skip to content

Commit 153602d

Browse files
vanceingallsclaude
andcommitted
fix(engine): route ALL video to screenshot fallback — fast video unsupported
Validated on a native amd64 Linux runner that per-frame BeginFrame does NOT make drawElementImage capture video (fast-vs-baseline ~12 dB, region black) — same as macOS. So video falls back to screenshot on every platform, not just macOS. Make the validation workflow manual-only (it fails by design until fast video is implemented). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8538a97 commit 153602d

4 files changed

Lines changed: 22 additions & 45 deletions

File tree

.github/workflows/fast-video-validation.yml

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,9 @@
99
name: Fast-capture video validation
1010

1111
on:
12-
# Temporary: lets the workflow run on this branch before it lands on the
13-
# default branch (workflow_dispatch is only triggerable from default). Remove
14-
# before merge — workflow_dispatch is the intended trigger.
15-
push:
16-
branches: ["drawelement-fast-capture"]
17-
paths:
18-
- ".github/workflows/fast-video-validation.yml"
19-
- "packages/producer/scripts/validate-fast-video.ts"
20-
- "packages/engine/src/services/**"
12+
# Manual only. NOTE: currently fails by design — fast capture cannot capture
13+
# video on any platform yet (see docs/fast-capture-limitations.md, Limitation 2).
14+
# This is the regression gate for if/when fast video is implemented.
2115
workflow_dispatch:
2216
inputs:
2317
composition:

packages/engine/src/services/drawElementService.test.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,16 @@ describe("resolveDrawElementCaptureMode", () => {
5353
expect(resolveDrawElementCaptureMode(false, false)).toBe("drawelement");
5454
});
5555

56-
// ── video routing: needs a per-frame BeginFrame paint for a fresh snapshot ──
57-
it("video without BeginFrame paint → screenshot (stale snapshot otherwise)", () => {
58-
expect(resolveDrawElementCaptureMode(false, false, /* hasVideo */ true, /* bf */ false)).toBe(
59-
"screenshot",
60-
);
56+
// ── video routing: drawElementImage can't capture video on any platform ──
57+
it("video → screenshot (drawElementImage does not capture video frames)", () => {
58+
expect(resolveDrawElementCaptureMode(false, false, /* hasVideo */ true)).toBe("screenshot");
6159
});
6260

63-
it("video WITH BeginFrame paint → drawelement (Linux headless-shell paints each frame)", () => {
64-
expect(resolveDrawElementCaptureMode(false, false, /* hasVideo */ true, /* bf */ true)).toBe(
65-
"drawelement",
66-
);
61+
it("video + GPU still screenshot (verified broken on macOS and Linux/BeginFrame)", () => {
62+
expect(resolveDrawElementCaptureMode(false, false, true)).toBe("screenshot");
6763
});
6864

69-
it("no video → drawelement regardless of BeginFrame", () => {
70-
expect(resolveDrawElementCaptureMode(false, false, false, false)).toBe("drawelement");
71-
});
72-
73-
it("transparent + SwiftShader still screenshot even with BeginFrame", () => {
74-
expect(resolveDrawElementCaptureMode(true, true, false, true)).toBe("screenshot");
65+
it("no video → drawelement", () => {
66+
expect(resolveDrawElementCaptureMode(false, false, false)).toBe("drawelement");
7567
});
7668
});

packages/engine/src/services/drawElementService.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,20 @@ import type { Page } from "puppeteer-core";
2121
* Two cases fall back to screenshot (see docs/fast-capture-limitations.md):
2222
* - transparent + SwiftShader: software-GL drops promoted sub-layers on a
2323
* transparent canvas destination (Chromium bug 521434899).
24-
* - hasVideo + !beginFramePaints: drawElementImage draws a snapshot taken at
25-
* the paint event; without a per-frame BeginFrame paint (e.g. macOS, which
26-
* has no --enable-begin-frame-control) the snapshot is stale → black/frozen
27-
* video. Where BeginFrame drives a paint each frame (Linux headless-shell)
28-
* the snapshot is current and video captures correctly.
29-
*
30-
* @param beginFramePaints true when a per-frame HeadlessExperimental.beginFrame
31-
* advances + paints the compositor before each capture (Linux headless-shell).
24+
* - hasVideo: drawElementImage draws a snapshot taken at the paint event and
25+
* does not capture the freshly-injected per-frame video <img> — the video
26+
* region comes out black/stale. This was verified on BOTH macOS and a native
27+
* amd64 Linux runner (where per-frame BeginFrame *does* paint) — fast-vs-
28+
* baseline PSNR ~12 dB either way — so video is unconditionally routed to
29+
* screenshot capture regardless of platform. Fast video is future R&D.
3230
*/
3331
export function resolveDrawElementCaptureMode(
3432
isSwiftShader: boolean,
3533
transparent: boolean,
3634
hasVideo = false,
37-
beginFramePaints = false,
3835
): "drawelement" | "screenshot" {
3936
if (transparent && isSwiftShader) return "screenshot";
40-
if (hasVideo && !beginFramePaints) return "screenshot";
37+
if (hasVideo) return "screenshot";
4138
return "drawelement";
4239
}
4340

packages/engine/src/services/frameCapture.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,22 +347,16 @@ async function initDrawElementOrTransparentBackground(
347347
if (useDrawElement) {
348348
session.isSwiftShader = await detectSwiftShader(page);
349349
const transparent = session.options.format === "png";
350+
// drawElementImage does not capture the per-frame video <img> (the snapshot
351+
// predates it) — verified black on both macOS and native Linux/BeginFrame —
352+
// so any composition containing <video> falls back to screenshot capture.
350353
const hasVideo = await page.evaluate(() => document.querySelector("video") !== null);
351-
// BeginFrame drives a per-frame paint (Linux headless-shell) → drawElementImage
352-
// reads a fresh snapshot, so video captures correctly. Without it (macOS) the
353-
// snapshot is stale → video would be black; route those renders to screenshot.
354-
const beginFramePaints = session.beginFrameTimeTicks > 0;
355-
const mode = resolveDrawElementCaptureMode(
356-
session.isSwiftShader,
357-
transparent,
358-
hasVideo,
359-
beginFramePaints,
360-
);
354+
const mode = resolveDrawElementCaptureMode(session.isSwiftShader, transparent, hasVideo);
361355
if (mode === "screenshot") {
362356
const reason =
363357
transparent && session.isSwiftShader
364358
? "transparent output on SwiftShader (Chromium bug 521434899)"
365-
: "video without per-frame BeginFrame paint (drawElementImage snapshot would be stale)";
359+
: "composition contains <video> (drawElementImage does not capture video frames)";
366360
console.log(`[engine] fast capture: falling back to screenshot — ${reason}`);
367361
session.captureMode = "screenshot";
368362
if (transparent) {

0 commit comments

Comments
 (0)