Skip to content

Commit f622e5a

Browse files
fix(engine): restore fast screenshot path for viewport captures (#1670)
1 parent bf4f34b commit f622e5a

6 files changed

Lines changed: 44 additions & 7 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,33 @@ describe("pageScreenshotCapture supersample plumbing", () => {
4141
expect(send).toHaveBeenCalledWith(
4242
"Page.captureScreenshot",
4343
expect.objectContaining({
44+
captureBeyondViewport: false,
4445
clip: { x: 0, y: 0, width: 1920, height: 1080, scale: 1 },
4546
}),
4647
);
4748
});
4849

50+
it("uses captureBeyondViewport only when callers opt in", async () => {
51+
const send = vi.fn().mockResolvedValue({ data: ONE_PIXEL_PNG_B64 });
52+
const page = makeFakePageWithCdp(send);
53+
54+
await pageScreenshotCapture(page, {
55+
width: 1080,
56+
height: 1920,
57+
fps: { num: 30, den: 1 },
58+
format: "jpeg",
59+
captureBeyondViewport: true,
60+
});
61+
62+
expect(send).toHaveBeenCalledWith(
63+
"Page.captureScreenshot",
64+
expect.objectContaining({
65+
captureBeyondViewport: true,
66+
clip: { x: 0, y: 0, width: 1080, height: 1920, scale: 1 },
67+
}),
68+
);
69+
});
70+
4971
it("passes `clip` with scale 1 when deviceScaleFactor is exactly 1", async () => {
5072
const send = vi.fn().mockResolvedValue({ data: ONE_PIXEL_PNG_B64 });
5173
const page = makeFakePageWithCdp(send);

packages/engine/src/services/screenshotService.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,10 @@ export async function pageScreenshotCapture(page: Page, options: CaptureOptions)
135135
format: isPng ? "png" : "jpeg",
136136
quality: isPng ? undefined : (options.quality ?? 80),
137137
fromSurface: true,
138-
// The explicit clip rect constrains output to exact composition
139-
// dimensions. The viewport-boundary pre-clip from captureBeyondViewport:
140-
// false is redundant, and Chrome's compositor rounds it inward under
141-
// multi-tab load — clipping the bottom/right edge of tall viewports.
142-
captureBeyondViewport: true,
138+
// Use Chrome's faster viewport-bound screenshot path by default. Callers
139+
// opt into the beyond-viewport path only for known compositor edge cases,
140+
// such as native video surfaces in tall portrait renders.
141+
captureBeyondViewport: options.captureBeyondViewport ?? false,
143142
optimizeForSpeed: !isPng,
144143
clip,
145144
});
@@ -172,7 +171,8 @@ export async function captureScreenshotWithAlpha(
172171
const result = await client.send("Page.captureScreenshot", {
173172
format: "png",
174173
fromSurface: true,
175-
captureBeyondViewport: true, // see pageScreenshotCapture for rationale
174+
// Preserve the #1094 tall-portrait edge-clipping guard on HDR alpha captures.
175+
captureBeyondViewport: true,
176176
optimizeForSpeed: false, // `true` uses a zero-alpha-aware fast path that crushes real alpha values — observed empirically, CDP docs don't spell it out
177177
clip: { x: 0, y: 0, width, height, scale: 1 },
178178
});
@@ -237,7 +237,8 @@ export async function captureAlphaPng(page: Page, width: number, height: number)
237237
const result = await client.send("Page.captureScreenshot", {
238238
format: "png",
239239
fromSurface: true,
240-
captureBeyondViewport: true, // see pageScreenshotCapture for rationale
240+
// Preserve the #1094 tall-portrait edge-clipping guard on HDR alpha captures.
241+
captureBeyondViewport: true,
241242
optimizeForSpeed: false, // must be false to preserve alpha
242243
clip: { x: 0, y: 0, width, height, scale: 1 },
243244
});

packages/engine/src/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ export interface CaptureOptions {
9292
format?: "jpeg" | "png";
9393
quality?: number;
9494
deviceScaleFactor?: number;
95+
/**
96+
* Opt into Chrome's capture-beyond-viewport screenshot path. Keep this off
97+
* for ordinary viewport-sized captures because it is substantially slower in
98+
* Chrome's screenshot compositor path. Enable for known compositor edge cases
99+
* such as native video surfaces in tall portrait renders.
100+
*/
101+
captureBeyondViewport?: boolean;
95102
/**
96103
* FFmpeg-probed intrinsic dimensions for videos whose frames are injected
97104
* out-of-band. Applied before the readiness wait so layout that depends on

packages/producer/src/services/distributed/renderChunk.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ export async function renderChunk(
507507
// declare `data-composition-variables` leave this undefined and the
508508
// engine skips the `evaluateOnNewDocument` injection.
509509
variables: encoder.variables,
510+
captureBeyondViewport: (planVideos?.videos.length ?? 0) > 0,
510511
// lock the BeginFrame warmup loop to a fixed iteration count so
511512
// `beginFrameTimeTicks` is host-independent. Only chunks ever set this.
512513
lockWarmupTicks: true,

packages/producer/src/services/render/observability.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export interface BrowserDiagnosticSummary {
3333
export interface RenderCaptureObservability {
3434
forceScreenshot: boolean;
3535
captureMode: "screenshot" | "beginframe";
36+
captureBeyondViewport?: boolean;
3637
workerCount?: number;
3738
useStreamingEncode?: boolean;
3839
useLayeredComposite?: boolean;

packages/producer/src/services/renderOrchestrator.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,7 +1213,11 @@ export async function executeRenderJob(
12131213
quality: needsAlpha ? undefined : job.config.quality === "draft" ? 80 : 95,
12141214
variables: job.config.variables,
12151215
deviceScaleFactor,
1216+
captureBeyondViewport: composition.videos.length > 0,
12161217
};
1218+
updateCaptureObservability({
1219+
captureBeyondViewport: captureOptions.captureBeyondViewport ?? false,
1220+
});
12171221

12181222
// Capture sessions do not need native browser metadata for videos whose
12191223
// pixels come from out-of-band FFmpeg frame extraction. Waiting on those
@@ -1438,6 +1442,7 @@ export async function executeRenderJob(
14381442
observability.checkpoint("capture_strategy", "resolved", {
14391443
workerCount,
14401444
forceScreenshot: captureForceScreenshot,
1445+
captureBeyondViewport: captureOptions.captureBeyondViewport ?? false,
14411446
useStreamingEncode,
14421447
useLayeredComposite,
14431448
usePageSideCompositing: usePageSideCompositingForTransitions,

0 commit comments

Comments
 (0)