fix(engine): defend macOS regular Chrome screenshots

Fixes #1699.
This commit is contained in:
Miguel Ángel
2026-06-24 19:21:46 -04:00
committed by GitHub
parent 821bf2921a
commit 89ff299a11
13 changed files with 819 additions and 15 deletions
@@ -0,0 +1,36 @@
// @vitest-environment node
import { describe, expect, it } from "vitest";
import { resolveCaptureSessionOptions } from "./frameCapture.js";
describe("createCaptureSession captureBeyondViewport defaults", () => {
it("plumbs the macOS regular-Chrome default into returned session options", () => {
const options = resolveCaptureSessionOptions(
{
width: 1920,
height: 1080,
fps: { num: 30, den: 1 },
format: "jpeg",
},
"Chrome/149.0.7827.155",
"darwin",
);
expect(options.captureBeyondViewport).toBe(true);
});
it("preserves explicit caller overrides", () => {
const options = resolveCaptureSessionOptions(
{
width: 1920,
height: 1080,
fps: { num: 30, den: 1 },
format: "jpeg",
captureBeyondViewport: false,
},
"Chrome/149.0.7827.155",
"darwin",
);
expect(options.captureBeyondViewport).toBe(false);
});
});
+18 -4
View File
@@ -28,6 +28,7 @@ import {
getCdpSession,
pageScreenshotCapture,
initTransparentBackground,
shouldDefaultCaptureBeyondViewport,
} from "./screenshotService.js";
import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
import type {
@@ -310,6 +311,18 @@ export async function driveWarmupTicks(
}
}
export function resolveCaptureSessionOptions(
options: CaptureOptions,
browserVersion: string,
platform: NodeJS.Platform = process.platform,
): CaptureOptions {
return {
...options,
captureBeyondViewport:
options.captureBeyondViewport ?? shouldDefaultCaptureBeyondViewport(browserVersion, platform),
};
}
async function waitForCloseWithTimeout(promise: Promise<unknown>): Promise<boolean> {
let timedOut = false;
let timer: ReturnType<typeof setTimeout> | undefined;
@@ -417,6 +430,7 @@ export async function createCaptureSession(
}, variablesJson);
}
const browserVersion = await browser.version();
const sessionOptions = resolveCaptureSessionOptions(options, browserVersion);
const expectedMajor = config?.expectedChromiumMajor;
if (Number.isFinite(expectedMajor)) {
const actualChromiumMajor = Number.parseInt(
@@ -430,9 +444,9 @@ export async function createCaptureSession(
}
}
const viewport: Viewport = {
width: options.width,
height: options.height,
deviceScaleFactor: options.deviceScaleFactor || 1,
width: sessionOptions.width,
height: sessionOptions.height,
deviceScaleFactor: sessionOptions.deviceScaleFactor || 1,
};
await page.setViewport(viewport);
@@ -446,7 +460,7 @@ export async function createCaptureSession(
return {
browser,
page,
options,
options: sessionOptions,
serverUrl,
outputDir,
onBeforeCapture,
@@ -7,6 +7,7 @@ import {
cdpSessionCache,
injectVideoFramesBatch,
syncVideoFrameVisibility,
shouldDefaultCaptureBeyondViewport,
} from "./screenshotService.js";
// Stub a Page + CDPSession just enough that pageScreenshotCapture can call
@@ -121,6 +122,22 @@ describe("pageScreenshotCapture supersample plumbing", () => {
});
});
describe("shouldDefaultCaptureBeyondViewport", () => {
it("guards regular Chrome on macOS", () => {
expect(shouldDefaultCaptureBeyondViewport("Chrome/149.0.7827.155", "darwin")).toBe(true);
});
it("keeps chrome-headless-shell on the faster viewport-bound path", () => {
expect(shouldDefaultCaptureBeyondViewport("HeadlessChrome/148.0.7778.97", "darwin")).toBe(
false,
);
});
it("does not change regular Chrome defaults on non-macOS platforms", () => {
expect(shouldDefaultCaptureBeyondViewport("Chrome/149.0.7827.155", "linux")).toBe(false);
});
});
describe("injectVideoFramesBatch replacement layout", () => {
it("does not copy opposing inset constraints onto the injected frame image", async () => {
const { window, document } = parseHTML(
@@ -19,6 +19,17 @@ export async function getCdpSession(page: Page): Promise<import("puppeteer-core"
return client;
}
export function shouldDefaultCaptureBeyondViewport(
browserVersion: string,
platform: NodeJS.Platform = process.platform,
): boolean {
// Regular Chrome's viewport-bound screenshot path can expose a compositor
// surface shorter than the page viewport on affected macOS builds. In that
// case Chrome fills the clipped area with the page background. Headless shell
// reports as HeadlessChrome and keeps the faster viewport-bound path.
return platform === "darwin" && browserVersion.startsWith("Chrome/");
}
/**
* BeginFrame result with screenshot data and damage detection.
*/
+5 -4
View File
@@ -93,10 +93,11 @@ export interface CaptureOptions {
quality?: number;
deviceScaleFactor?: number;
/**
* Opt into Chrome's capture-beyond-viewport screenshot path. Keep this off
* for ordinary viewport-sized captures because it is substantially slower in
* Chrome's screenshot compositor path. Enable for known compositor edge cases
* such as native video surfaces in tall portrait renders.
* Opt into Chrome's capture-beyond-viewport screenshot path. Leave undefined
* to let the engine pick the safe browser-specific default. Pass false only
* when the caller explicitly wants Chrome's faster viewport-bound path.
* Enable for known compositor edge cases such as native video surfaces in
* tall portrait renders.
*/
captureBeyondViewport?: boolean;
/**
@@ -507,7 +507,7 @@ export async function renderChunk(
// declare `data-composition-variables` leave this undefined and the
// engine skips the `evaluateOnNewDocument` injection.
variables: encoder.variables,
captureBeyondViewport: (planVideos?.videos.length ?? 0) > 0,
...((planVideos?.videos.length ?? 0) > 0 ? { captureBeyondViewport: true } : {}),
// lock the BeginFrame warmup loop to a fixed iteration count so
// `beginFrameTimeTicks` is host-independent. Only chunks ever set this.
lockWarmupTicks: true,
@@ -126,6 +126,8 @@ export interface CaptureStageResult {
probeSession: CaptureSession | null;
/** Browser console buffer from whichever session was active last. */
lastBrowserConsole: string[];
/** Engine-resolved screenshot flag from the consumed sequential/probe session, when observed. */
captureBeyondViewport?: boolean;
}
export async function runCaptureStage(input: CaptureStageInput): Promise<CaptureStageResult> {
@@ -150,6 +152,7 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
} = input;
let { workerCount, probeSession } = input;
let lastBrowserConsole: string[] = [];
let captureBeyondViewport: boolean | undefined = probeSession?.options.captureBeyondViewport;
// Derive a local cfg view rather than reading `forceScreenshot` from the
// caller-owned `cfg`. The sequencer threads the resolved value via the
@@ -228,6 +231,7 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
workerCount = lastAttempt.workers;
}
if (probeSession) {
captureBeyondViewport = probeSession.options.captureBeyondViewport;
lastBrowserConsole = probeSession.browserConsoleBuffer;
await closeCaptureSession(probeSession);
probeSession = null;
@@ -245,6 +249,7 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
videoInjector,
captureCfg,
));
captureBeyondViewport = session.options.captureBeyondViewport;
if (probeSession) {
prepareCaptureSessionForReuse(session, framesDir, videoInjector);
probeSession = null;
@@ -304,5 +309,5 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
}
}
return { workerCount, probeSession, lastBrowserConsole };
return { workerCount, probeSession, lastBrowserConsole, captureBeyondViewport };
}
@@ -126,6 +126,8 @@ export type CaptureStreamingStageResult =
probeSession: CaptureSession | null;
lastBrowserConsole: string[];
workerCount: number;
/** Engine-resolved screenshot flag from the consumed sequential/probe session, when observed. */
captureBeyondViewport?: boolean;
}
| {
/** Spawn failed (non-abort) — sequencer should fall back to the disk path. */
@@ -156,6 +158,7 @@ export async function runCaptureStreamingStage(
} = input;
let { workerCount, probeSession } = input;
let lastBrowserConsole: string[] = [];
let captureBeyondViewport: boolean | undefined = probeSession?.options.captureBeyondViewport;
// Derive a local cfg view rather than reading `forceScreenshot` from the
// caller-owned `cfg`. The sequencer threads the resolved value via the
@@ -241,6 +244,7 @@ export async function runCaptureStreamingStage(
pushWorkerDedupPerfs(workerResults, dedupPerfs);
if (probeSession) {
captureBeyondViewport = probeSession.options.captureBeyondViewport;
lastBrowserConsole = probeSession.browserConsoleBuffer;
await closeCaptureSession(probeSession);
probeSession = null;
@@ -258,6 +262,7 @@ export async function runCaptureStreamingStage(
videoInjector,
captureCfg,
));
captureBeyondViewport = session.options.captureBeyondViewport;
if (probeSession) {
prepareCaptureSessionForReuse(session, framesDir, videoInjector);
probeSession = null;
@@ -325,6 +330,7 @@ export async function runCaptureStreamingStage(
probeSession,
lastBrowserConsole,
workerCount,
captureBeyondViewport,
};
} finally {
// Defensive cleanup: if the streaming branch threw before
@@ -1074,6 +1074,10 @@ export async function executeRenderJob(
fileServer = probeResult.fileServer;
probeSession = probeResult.probeSession;
lastBrowserConsole = probeResult.lastBrowserConsole;
let resolvedCaptureBeyondViewport = probeSession?.options.captureBeyondViewport;
if (resolvedCaptureBeyondViewport !== undefined) {
updateCaptureObservability({ captureBeyondViewport: resolvedCaptureBeyondViewport });
}
// The probe stage produces `duration` / `totalFrames` values; the
// sequencer owns the `RenderJob` and writes them onto it.
job.duration = probeResult.duration;
@@ -1213,11 +1217,13 @@ export async function executeRenderJob(
quality: needsAlpha ? undefined : job.config.quality === "draft" ? 80 : 95,
variables: job.config.variables,
deviceScaleFactor,
captureBeyondViewport: composition.videos.length > 0,
...(composition.videos.length > 0 ? { captureBeyondViewport: true } : {}),
};
updateCaptureObservability({
captureBeyondViewport: captureOptions.captureBeyondViewport ?? false,
});
resolvedCaptureBeyondViewport =
captureOptions.captureBeyondViewport ?? resolvedCaptureBeyondViewport;
if (resolvedCaptureBeyondViewport !== undefined) {
updateCaptureObservability({ captureBeyondViewport: resolvedCaptureBeyondViewport });
}
// Capture sessions do not need native browser metadata for videos whose
// pixels come from out-of-band FFmpeg frame extraction. Waiting on those
@@ -1442,7 +1448,7 @@ export async function executeRenderJob(
observability.checkpoint("capture_strategy", "resolved", {
workerCount,
forceScreenshot: captureForceScreenshot,
captureBeyondViewport: captureOptions.captureBeyondViewport ?? false,
captureBeyondViewport: resolvedCaptureBeyondViewport ?? null,
useStreamingEncode,
useLayeredComposite,
usePageSideCompositing: usePageSideCompositingForTransitions,
@@ -1592,6 +1598,11 @@ export async function executeRenderJob(
streamingHandled = true;
workerCount = streamingRes.workerCount;
updateCaptureObservability({ workerCount });
if (streamingRes.captureBeyondViewport !== undefined) {
updateCaptureObservability({
captureBeyondViewport: streamingRes.captureBeyondViewport,
});
}
probeSession = streamingRes.probeSession;
lastBrowserConsole = streamingRes.lastBrowserConsole;
perfStages.captureMs = Date.now() - stage4Start;
@@ -1637,6 +1648,11 @@ export async function executeRenderJob(
const captureFrameMs = Date.now() - captureFrameStart;
workerCount = captureRes.workerCount;
updateCaptureObservability({ workerCount });
if (captureRes.captureBeyondViewport !== undefined) {
updateCaptureObservability({
captureBeyondViewport: captureRes.captureBeyondViewport,
});
}
probeSession = captureRes.probeSession;
lastBrowserConsole = captureRes.lastBrowserConsole;
@@ -0,0 +1,13 @@
{
"name": "chrome-screenshot-bottom-edge",
"description": "Regression guard for screenshot capture leaking page background into the bottom of a viewport-sized MP4. The composition paints a red page background below full-frame content so any bottom-edge capture gap collapses PSNR.",
"tags": ["regression", "screenshot", "edge"],
"minPsnr": 30,
"maxFrameFailures": 0,
"minAudioCorrelation": 0,
"maxAudioLagWindows": 1,
"renderConfig": {
"fps": 12,
"workers": 1
}
}
File diff suppressed because one or more lines are too long
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:ffec9cda98f24e06e74f65440ef13a3889ce110bc2b04394fd93f8afb61c8893
size 43189
@@ -0,0 +1,76 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=1920, height=1080" />
<title>Chrome Screenshot Bottom Edge</title>
<script src="https://cdn.jsdelivr.net/npm/gsap@3.14.2/dist/gsap.min.js"></script>
<style>
html,
body {
margin: 0;
width: 1920px;
height: 1080px;
overflow: hidden;
background: rgb(255, 24, 0);
font-family: Inter, system-ui, sans-serif;
}
#root {
position: relative;
width: 1920px;
height: 1080px;
overflow: hidden;
background: linear-gradient(180deg, #084c61 0%, #0b6e4f 100%);
}
.clip {
position: absolute;
inset: 0;
}
#label {
position: absolute;
left: 64px;
bottom: 188px;
color: white;
font-size: 64px;
font-weight: 800;
}
#bottom-proof {
position: absolute;
left: 0;
right: 0;
bottom: 0;
height: 160px;
background: repeating-linear-gradient(
90deg,
white 0,
white 64px,
#111 64px,
#111 128px
);
}
</style>
</head>
<body>
<div
id="root"
data-composition-id="main"
data-width="1920"
data-height="1080"
data-start="0"
data-duration="1"
>
<section id="proof" class="clip" data-start="0" data-duration="1" data-track-index="1">
<div id="label">bottom edge must stay inside the composition</div>
<div id="bottom-proof"></div>
</section>
</div>
<script>
window.__timelines = window.__timelines || {};
window.__timelines.main = gsap.timeline({ paused: true });
</script>
</body>
</html>