Skip to content

Commit 8e0cfc3

Browse files
fix(engine): preserve video frame replacement geometry (#838)
* fix(engine): preserve video frame replacement geometry * test(producer): cover video overlay stretch regression * fix(engine): always pass clip to Page.captureScreenshot Without an explicit clip, Chrome can resolve replaced-element sizing differently at dpr=1 when full-bleed absolute videos interact with overlay layers — producing anisotropic frame stretching on some compositor paths. Always passing clip with scale=dpr (including 1) ensures geometry is locked to the measured viewport dimensions. Credit: brian-t-allen (#837) * test(producer): regenerate style-9-prod baseline for always-clip capture path The always-clip change in screenshotService.ts routes Chrome through a different compositor capture path at dpr=1, producing different video frame compression artifacts. Regenerated inside Dockerfile.test to match CI environment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 952d265 commit 8e0cfc3

10 files changed

Lines changed: 1442 additions & 721 deletions

File tree

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

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
// @vitest-environment node
22
import { describe, it, expect, vi } from "vitest";
3+
import { parseHTML } from "linkedom";
34
import { type Page } from "puppeteer-core";
4-
import { pageScreenshotCapture, cdpSessionCache } from "./screenshotService.js";
5+
import {
6+
pageScreenshotCapture,
7+
cdpSessionCache,
8+
injectVideoFramesBatch,
9+
} from "./screenshotService.js";
510

611
// Stub a Page + CDPSession just enough that pageScreenshotCapture can call
712
// `client.send("Page.captureScreenshot", ...)` and we can inspect the args.
@@ -20,7 +25,7 @@ describe("pageScreenshotCapture supersample plumbing", () => {
2025
const ONE_PIXEL_PNG_B64 =
2126
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=";
2227

23-
it("omits `clip` when deviceScaleFactor is undefined (default 1)", async () => {
28+
it("passes `clip` with scale 1 when deviceScaleFactor is undefined (default 1)", async () => {
2429
const send = vi.fn().mockResolvedValue({ data: ONE_PIXEL_PNG_B64 });
2530
const page = makeFakePageWithCdp(send);
2631

@@ -34,11 +39,13 @@ describe("pageScreenshotCapture supersample plumbing", () => {
3439

3540
expect(send).toHaveBeenCalledWith(
3641
"Page.captureScreenshot",
37-
expect.not.objectContaining({ clip: expect.anything() }),
42+
expect.objectContaining({
43+
clip: { x: 0, y: 0, width: 1920, height: 1080, scale: 1 },
44+
}),
3845
);
3946
});
4047

41-
it("omits `clip` when deviceScaleFactor is exactly 1", async () => {
48+
it("passes `clip` with scale 1 when deviceScaleFactor is exactly 1", async () => {
4249
const send = vi.fn().mockResolvedValue({ data: ONE_PIXEL_PNG_B64 });
4350
const page = makeFakePageWithCdp(send);
4451

@@ -50,8 +57,8 @@ describe("pageScreenshotCapture supersample plumbing", () => {
5057
deviceScaleFactor: 1,
5158
});
5259

53-
const params = send.mock.calls[0]?.[1] as { clip?: unknown };
54-
expect(params.clip).toBeUndefined();
60+
const params = send.mock.calls[0]?.[1] as { clip?: { scale: number } };
61+
expect(params.clip).toEqual({ x: 0, y: 0, width: 1920, height: 1080, scale: 1 });
5562
});
5663

5764
it("passes `clip` with `scale = dpr` when deviceScaleFactor > 1 (the supersample contract)", async () => {
@@ -90,3 +97,97 @@ describe("pageScreenshotCapture supersample plumbing", () => {
9097
expect(params.clip?.scale).toBe(3);
9198
});
9299
});
100+
101+
describe("injectVideoFramesBatch replacement layout", () => {
102+
it("does not copy opposing inset constraints onto the injected frame image", async () => {
103+
const { window, document } = parseHTML(
104+
'<html><body><div id="root"><video id="clip" style="position:absolute;inset:0;width:100%;height:100%;object-fit:cover"></video></div></body></html>',
105+
);
106+
107+
Object.defineProperty(window.HTMLImageElement.prototype, "decode", {
108+
configurable: true,
109+
value: () => Promise.resolve(),
110+
});
111+
112+
const video = document.getElementById("clip") as HTMLVideoElement;
113+
Object.defineProperties(video, {
114+
offsetLeft: { configurable: true, get: () => 0 },
115+
offsetTop: { configurable: true, get: () => 0 },
116+
offsetWidth: { configurable: true, get: () => 1920 },
117+
offsetHeight: { configurable: true, get: () => 1080 },
118+
});
119+
video.getBoundingClientRect = () =>
120+
({
121+
x: 0,
122+
y: 0,
123+
left: 0,
124+
top: 0,
125+
right: 1920,
126+
bottom: 1080,
127+
width: 1920,
128+
height: 1080,
129+
toJSON: () => ({}),
130+
}) as DOMRect;
131+
132+
const computedStyle = document.createElement("div").style;
133+
computedStyle.position = "absolute";
134+
computedStyle.width = "1920px";
135+
computedStyle.height = "1080px";
136+
computedStyle.top = "0px";
137+
computedStyle.left = "0px";
138+
computedStyle.right = "0px";
139+
computedStyle.bottom = "0px";
140+
computedStyle.inset = "0px";
141+
computedStyle.objectFit = "cover";
142+
computedStyle.objectPosition = "center center";
143+
computedStyle.zIndex = "3";
144+
computedStyle.opacity = "1";
145+
Object.defineProperty(window, "getComputedStyle", {
146+
configurable: true,
147+
value: () => computedStyle,
148+
});
149+
150+
const globals = globalThis as unknown as {
151+
window?: typeof window;
152+
document?: Document;
153+
};
154+
const previousWindow = globals.window;
155+
const previousDocument = globals.document;
156+
globals.window = window;
157+
globals.document = document;
158+
try {
159+
const page = {
160+
evaluate: async (
161+
fn: (
162+
updates: Array<{ videoId: string; dataUri: string }>,
163+
visualProperties: string[],
164+
) => Promise<void>,
165+
updates: Array<{ videoId: string; dataUri: string }>,
166+
visualProperties: string[],
167+
) => fn(updates, visualProperties),
168+
} as unknown as Page;
169+
170+
await injectVideoFramesBatch(page, [
171+
{
172+
videoId: "clip",
173+
dataUri:
174+
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
175+
},
176+
]);
177+
} finally {
178+
globals.window = previousWindow;
179+
globals.document = previousDocument;
180+
}
181+
182+
const img = video.nextElementSibling as HTMLImageElement | null;
183+
expect(img).not.toBeNull();
184+
expect(img?.style.position).toBe("absolute");
185+
expect(img?.style.left).toBe("0px");
186+
expect(img?.style.top).toBe("0px");
187+
expect(img?.style.width).toBe("1920px");
188+
expect(img?.style.height).toBe("1080px");
189+
expect(img?.style.right).toBe("auto");
190+
expect(img?.style.bottom).toBe("auto");
191+
expect(img?.style.inset).toBe("auto");
192+
});
193+
});

packages/engine/src/services/screenshotService.ts

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -130,19 +130,14 @@ export async function pageScreenshotCapture(page: Page, options: CaptureOptions)
130130
const client = await getCdpSession(page);
131131
const isPng = options.format === "png";
132132
const dpr = options.deviceScaleFactor ?? 1;
133-
// When supersampling, pass an explicit clip with `scale` so Chrome emits a
134-
// screenshot at device-pixel dimensions (`width × height × dpr`). Without
135-
// this, `Page.captureScreenshot` returns at CSS dimensions regardless of
136-
// the viewport's deviceScaleFactor.
137-
const clip =
138-
dpr > 1 ? { x: 0, y: 0, width: options.width, height: options.height, scale: dpr } : undefined;
133+
const clip = { x: 0, y: 0, width: options.width, height: options.height, scale: dpr };
139134
const result = await client.send("Page.captureScreenshot", {
140135
format: isPng ? "png" : "jpeg",
141136
quality: isPng ? undefined : (options.quality ?? 80),
142137
fromSurface: true,
143138
captureBeyondViewport: false,
144139
optimizeForSpeed: !isPng,
145-
...(clip ? { clip } : {}),
140+
clip,
146141
});
147142
return Buffer.from(result.data, "base64");
148143
}
@@ -382,6 +377,15 @@ export async function injectVideoFramesBatch(
382377
await page.evaluate(
383378
async (items: Array<{ videoId: string; dataUri: string }>, visualProperties: string[]) => {
384379
const pendingDecodes: Array<Promise<void>> = [];
380+
const replacementLayoutProperties = new Set([
381+
"width",
382+
"height",
383+
"top",
384+
"left",
385+
"right",
386+
"bottom",
387+
"inset",
388+
]);
385389
for (const item of items) {
386390
const video = document.getElementById(item.videoId) as HTMLVideoElement | null;
387391
if (!video) continue;
@@ -395,7 +399,6 @@ export async function injectVideoFramesBatch(
395399
// and accurately reflects the user's intent on every frame.
396400
const opacityParsed = parseFloat(computedStyle.opacity);
397401
const computedOpacity = Number.isNaN(opacityParsed) ? 1 : opacityParsed;
398-
const sourceIsStatic = !computedStyle.position || computedStyle.position === "static";
399402

400403
if (isNewImage) {
401404
img = document.createElement("img");
@@ -406,10 +409,35 @@ export async function injectVideoFramesBatch(
406409
}
407410
if (!img) continue;
408411

412+
for (const property of visualProperties) {
413+
// Opacity is handled explicitly via `computedOpacity` below — copying
414+
// via the generic loop would race against the opacity:0 hide applied
415+
// to the <video> at the end of this function. GSAP may animate
416+
// opacity either on a wrapper (the <img> inherits via the stacking
417+
// context) or directly on the <video> (we must copy it to the <img>
418+
// since they are siblings). Reading computedStyle.opacity before
419+
// hiding the <video> handles both cases correctly.
420+
if (property === "opacity") continue;
421+
// Layout is set from the video's used box below. Copying authored
422+
// opposing constraints such as `inset: 0` / `right: 0` onto the
423+
// replacement <img> can overconstrain replaced-image sizing and make
424+
// some Chrome capture paths resample the frame anisotropically.
425+
if (replacementLayoutProperties.has(property)) {
426+
continue;
427+
}
428+
const value = computedStyle.getPropertyValue(property);
429+
if (value) {
430+
img.style.setProperty(property, value);
431+
}
432+
}
433+
409434
// Always use absolute positioning so the <img> overlays the <video>
410435
// instead of flowing below it. With position:relative, both elements
411436
// stack vertically — the <img> lands below the video and gets clipped
412437
// by any overflow:hidden ancestor (e.g., border-radius wrappers).
438+
//
439+
// Apply this after visual style copying so the measured used box is
440+
// the final authority for replacement frame geometry.
413441
{
414442
const videoRect = video.getBoundingClientRect();
415443
const offsetLeft = Number.isFinite(video.offsetLeft) ? video.offsetLeft : 0;
@@ -429,30 +457,6 @@ export async function injectVideoFramesBatch(
429457
img.style.objectPosition = computedStyle.objectPosition;
430458
img.style.zIndex = computedStyle.zIndex;
431459

432-
for (const property of visualProperties) {
433-
// Opacity is handled explicitly via `computedOpacity` below — copying
434-
// via the generic loop would race against the opacity:0 hide applied
435-
// to the <video> at the end of this function. GSAP may animate
436-
// opacity either on a wrapper (the <img> inherits via the stacking
437-
// context) or directly on the <video> (we must copy it to the <img>
438-
// since they are siblings). Reading computedStyle.opacity before
439-
// hiding the <video> handles both cases correctly.
440-
if (property === "opacity") continue;
441-
if (
442-
sourceIsStatic &&
443-
(property === "top" ||
444-
property === "left" ||
445-
property === "right" ||
446-
property === "bottom" ||
447-
property === "inset")
448-
) {
449-
continue;
450-
}
451-
const value = computedStyle.getPropertyValue(property);
452-
if (value) {
453-
img.style.setProperty(property, value);
454-
}
455-
}
456460
img.decoding = "sync";
457461
if (img.getAttribute("src") !== item.dataUri) {
458462
img.src = item.dataUri;
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"name": "render-video-overlay-stretch",
3+
"description": "Regression fixture for #837. Renders two full-bleed MP4 clips with position:absolute/inset:0 and an image overlay across the cut. The injected video-frame <img> must preserve the source video's measured 16:9 box instead of inheriting opposing inset constraints that can stretch the frame during overlay transitions.",
4+
"tags": ["regression", "video", "overlay"],
5+
"minPsnr": 30,
6+
"maxFrameFailures": 0,
7+
"minAudioCorrelation": 0,
8+
"maxAudioLagWindows": 1,
9+
"renderConfig": {
10+
"fps": 30,
11+
"workers": 1
12+
}
13+
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8">
5+
<meta name="viewport" content="width=480, height=270">
6+
<title>Video Overlay Stretch Regression</title>
7+
<style>
8+
* {
9+
box-sizing: border-box;
10+
}
11+
12+
html,
13+
body {
14+
margin: 0;
15+
padding: 0;
16+
width: 480px;
17+
height: 270px;
18+
overflow: hidden;
19+
background: #000;
20+
}
21+
22+
#root {
23+
position: relative;
24+
width: 480px;
25+
height: 270px;
26+
overflow: hidden;
27+
background: #000;
28+
}
29+
30+
.scene-video {
31+
position: absolute;
32+
inset: 0;
33+
width: 100%;
34+
height: 100%;
35+
object-fit: cover;
36+
object-position: center center;
37+
}
38+
39+
.transition-overlay {
40+
position: absolute;
41+
inset: 0;
42+
width: 100%;
43+
height: 100%;
44+
object-fit: cover;
45+
opacity: 0.82;
46+
}
47+
48+
.cut-label {
49+
position: absolute;
50+
right: 12px;
51+
bottom: 12px;
52+
z-index: 20;
53+
padding: 5px 8px;
54+
border: 2px solid rgba(255, 255, 255, 0.88);
55+
color: #fff;
56+
background: rgba(0, 0, 0, 0.68);
57+
font: 700 13px Arial, Helvetica, sans-serif;
58+
}
59+
</style>
60+
</head>
61+
<body>
62+
<div id="root" data-composition-id="video-overlay-stretch" data-start="0" data-duration="4" data-width="480" data-height="270">
63+
<video id="clip-a" class="clip scene-video" src="clip.mp4" data-start="0" data-duration="2" data-track-index="0" muted playsinline data-end="2" data-has-audio="false"></video>
64+
65+
<video id="clip-b" class="clip scene-video" src="clip.mp4" data-start="2" data-duration="2" data-track-index="1" muted playsinline data-end="4" data-has-audio="false"></video>
66+
67+
<img id="transition" class="clip transition-overlay" src="transition-overlay.png" data-start="1.6" data-duration="0.8" data-track-index="10" alt="">
68+
69+
<div class="clip cut-label" data-start="0" data-duration="2" data-track-index="20">
70+
CLIP A
71+
</div>
72+
<div class="clip cut-label" data-start="2" data-duration="2" data-track-index="21">
73+
CLIP B
74+
</div>
75+
</div>
76+
77+
<script>window.__timelines = window.__timelines || {};</script></body>
78+
</html>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
version https://git-lfs.github.com/spec/v1
2+
oid sha256:95f96e2676be7808074ca22dc06f68bb845c83ff3f2036afd75730c4c487cc3a
3+
size 20066
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
version https://git-lfs.github.com/spec/v1
2+
oid sha256:da59ab7e77075ae083b9c957bc6d7896a0e0d62eeef4f50be63e278a1feb7303
3+
size 5358

0 commit comments

Comments
 (0)