mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-05 00:56:23 +00:00
fix(engine): skip video frame injection when a visual ancestor is hidden
`injectVideoFramesBatch` and `syncVideoFrameVisibility` iterate every `video[data-start]` whose raw time window covers the current seek. Inner `<video>` elements inside `[data-composition-src]` sub-compositions get `data-start="0"` auto-injected by `compileTimingAttrs` and probed-duration cover the entire timeline, so they look "active" even when their host has not yet started. When the runtime then hides the host with `visibility: hidden` (its out-of-window lifecycle), the inner video inherits hidden via the CSS cascade — but our injector responded by painting a replacement `<img class="__render_frame__" style="visibility: visible">` next to the video. `visibility: visible` on the descendant defeats the parent `visibility: hidden` cascade, and because the host has not been morphed by GSAP yet the video's bounding box is its CSS default (usually full-bleed). The result is one full-bleed frame per inactive sub-comp painted over whichever moment is *actually* visible — the overlay symptom the upstream agentic-finecut project saw. Walk ancestors in both functions; if any has `display: none` or `visibility: hidden`, skip the inject and hide any stale `__render_frame__` sibling. The render is now correctly empty for hidden hosts, which is what the surrounding CSS cascade already intends. Tests: - `screenshotService.test.ts`: cover the new guard for both visibility:hidden and display:none hosts, both for the fresh-img and the stale-img paths, plus `syncVideoFrameVisibility` for the case where the time window calls a video "active" but a hidden ancestor still requires its frame to stay hidden. Each test fails against pre-fix `screenshotService.ts`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lirian Su
committed by
Miguel Ángel
Miguel Ángel
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
60cb9552e4
commit
3700cc2a16
@@ -6,6 +6,7 @@ import {
|
||||
pageScreenshotCapture,
|
||||
cdpSessionCache,
|
||||
injectVideoFramesBatch,
|
||||
syncVideoFrameVisibility,
|
||||
} from "./screenshotService.js";
|
||||
|
||||
// Stub a Page + CDPSession just enough that pageScreenshotCapture can call
|
||||
@@ -191,3 +192,195 @@ describe("injectVideoFramesBatch replacement layout", () => {
|
||||
expect(img?.style.inset).toBe("auto");
|
||||
});
|
||||
});
|
||||
|
||||
describe("video-frame injection respects ancestor visibility", () => {
|
||||
// Regression guard: the runtime's `[data-start]` lifecycle hides
|
||||
// out-of-window sub-composition hosts with `visibility:hidden`, but the
|
||||
// injector used to ignore that and paint a replacement <img> for every
|
||||
// active `<video data-start>` element. Inner-PIP videos inside *other*
|
||||
// moments still appear active in the raw time-window check (their auto-
|
||||
// injected `data-start="0"` + probed full-source duration cover the
|
||||
// whole timeline), so the bug produced one full-bleed speaker overlay
|
||||
// per inactive sub-comp — covering whichever moment was actually visible.
|
||||
// See: https://github.com/lirian-su-opus/hyperframes branch issue thread.
|
||||
|
||||
type StyleLike = {
|
||||
display?: string;
|
||||
visibility?: string;
|
||||
opacity?: string;
|
||||
objectFit?: string;
|
||||
objectPosition?: string;
|
||||
zIndex?: string;
|
||||
};
|
||||
|
||||
function setupHostHiddenScenario(hostStyle: StyleLike) {
|
||||
const { window, document } = parseHTML(
|
||||
'<html><body><div id="host"><div id="pip-frame"><video id="pip" data-start="0" data-duration="10"></video></div></div></body></html>',
|
||||
);
|
||||
|
||||
Object.defineProperty(window.HTMLImageElement.prototype, "decode", {
|
||||
configurable: true,
|
||||
value: () => Promise.resolve(),
|
||||
});
|
||||
|
||||
const host = document.getElementById("host") as HTMLElement;
|
||||
const pipFrame = document.getElementById("pip-frame") as HTMLElement;
|
||||
const video = document.getElementById("pip") as HTMLVideoElement;
|
||||
|
||||
Object.defineProperties(video, {
|
||||
offsetLeft: { configurable: true, get: () => 0 },
|
||||
offsetTop: { configurable: true, get: () => 0 },
|
||||
offsetWidth: { configurable: true, get: () => 1080 },
|
||||
offsetHeight: { configurable: true, get: () => 1920 },
|
||||
});
|
||||
video.getBoundingClientRect = () =>
|
||||
({
|
||||
x: 0,
|
||||
y: 0,
|
||||
left: 0,
|
||||
top: 0,
|
||||
right: 1080,
|
||||
bottom: 1920,
|
||||
width: 1080,
|
||||
height: 1920,
|
||||
toJSON: () => ({}),
|
||||
}) as DOMRect;
|
||||
|
||||
const styles = new Map<Element, StyleLike>();
|
||||
styles.set(host, hostStyle);
|
||||
styles.set(pipFrame, {});
|
||||
styles.set(video, { opacity: "1", objectFit: "cover", objectPosition: "center", zIndex: "1" });
|
||||
|
||||
Object.defineProperty(window, "getComputedStyle", {
|
||||
configurable: true,
|
||||
value: (el: Element) => {
|
||||
const declared = styles.get(el) ?? {};
|
||||
return {
|
||||
display: declared.display ?? "block",
|
||||
visibility: declared.visibility ?? "visible",
|
||||
opacity: declared.opacity ?? "1",
|
||||
objectFit: declared.objectFit ?? "fill",
|
||||
objectPosition: declared.objectPosition ?? "50% 50%",
|
||||
zIndex: declared.zIndex ?? "auto",
|
||||
getPropertyValue: (prop: string) => {
|
||||
const camel = prop.replace(/-([a-z])/g, (_, c: string) =>
|
||||
c.toUpperCase(),
|
||||
) as keyof StyleLike;
|
||||
return declared[camel] ?? "";
|
||||
},
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
return { window, document, video, host, pipFrame };
|
||||
}
|
||||
|
||||
function withGlobals<T extends { window: Window; document: Document; video: HTMLVideoElement }>(
|
||||
setup: T,
|
||||
): { teardown: () => void; setup: T } {
|
||||
const globals = globalThis as unknown as { window?: Window; document?: Document };
|
||||
const previousWindow = globals.window;
|
||||
const previousDocument = globals.document;
|
||||
globals.window = setup.window;
|
||||
globals.document = setup.document;
|
||||
return {
|
||||
setup,
|
||||
teardown: () => {
|
||||
globals.window = previousWindow;
|
||||
globals.document = previousDocument;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function passthroughPage(): Page {
|
||||
return {
|
||||
evaluate: async (fn: (...args: unknown[]) => unknown, ...args: unknown[]) =>
|
||||
// The implementation is built to run inside the page sandbox via
|
||||
// `page.evaluate`, but linkedom gives us a DOM compatible enough to
|
||||
// execute the function body directly in Node.
|
||||
Promise.resolve((fn as (...a: unknown[]) => unknown)(...args)),
|
||||
} as unknown as Page;
|
||||
}
|
||||
|
||||
it("skips replacement-frame creation when the video's host has visibility:hidden", async () => {
|
||||
const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
|
||||
try {
|
||||
await injectVideoFramesBatch(passthroughPage(), [
|
||||
{
|
||||
videoId: "pip",
|
||||
dataUri:
|
||||
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
|
||||
},
|
||||
]);
|
||||
} finally {
|
||||
teardown();
|
||||
}
|
||||
|
||||
// No replacement <img> should be injected next to the video — the host is
|
||||
// currently hidden, so painting a frame over it would bleed onto whichever
|
||||
// sibling host is actually visible on this seek.
|
||||
const sibling = setup.video.nextElementSibling as HTMLElement | null;
|
||||
expect(sibling).toBeNull();
|
||||
});
|
||||
|
||||
it("skips replacement-frame creation when the video's host has display:none", async () => {
|
||||
const { teardown, setup } = withGlobals(setupHostHiddenScenario({ display: "none" }));
|
||||
try {
|
||||
await injectVideoFramesBatch(passthroughPage(), [
|
||||
{
|
||||
videoId: "pip",
|
||||
dataUri:
|
||||
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
|
||||
},
|
||||
]);
|
||||
} finally {
|
||||
teardown();
|
||||
}
|
||||
|
||||
const sibling = setup.video.nextElementSibling as HTMLElement | null;
|
||||
expect(sibling).toBeNull();
|
||||
});
|
||||
|
||||
it("hides an existing replacement <img> when the host becomes visibility:hidden", async () => {
|
||||
// First seed an existing __render_frame__ <img> next to the video (the
|
||||
// state the page is in after a previous seek when the host was visible).
|
||||
const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
|
||||
const seededImg = setup.document.createElement("img");
|
||||
seededImg.classList.add("__render_frame__");
|
||||
seededImg.style.visibility = "visible";
|
||||
setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
|
||||
|
||||
try {
|
||||
await injectVideoFramesBatch(passthroughPage(), [
|
||||
{
|
||||
videoId: "pip",
|
||||
dataUri:
|
||||
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
|
||||
},
|
||||
]);
|
||||
} finally {
|
||||
teardown();
|
||||
}
|
||||
|
||||
expect(seededImg.style.visibility).toBe("hidden");
|
||||
});
|
||||
|
||||
it("syncVideoFrameVisibility hides the replacement <img> for ancestor-hidden actives", async () => {
|
||||
const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
|
||||
const seededImg = setup.document.createElement("img");
|
||||
seededImg.classList.add("__render_frame__");
|
||||
seededImg.style.visibility = "visible";
|
||||
setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
|
||||
|
||||
try {
|
||||
// "pip" IS in the active set (per the raw time-window check) but the
|
||||
// host is hidden. sync must keep the <img> hidden, not flip it to
|
||||
// `visibility: visible`.
|
||||
await syncVideoFrameVisibility(passthroughPage(), ["pip"]);
|
||||
} finally {
|
||||
teardown();
|
||||
}
|
||||
|
||||
expect(seededImg.style.visibility).toBe("hidden");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -386,12 +386,39 @@ export async function injectVideoFramesBatch(
|
||||
"bottom",
|
||||
"inset",
|
||||
]);
|
||||
// Walk ancestors looking for a host that the page has hidden via
|
||||
// `display:none` or `visibility:hidden`. The runtime hides
|
||||
// `[data-composition-src]` and `[data-start]` hosts that fall outside
|
||||
// their time window using exactly these properties; a nested
|
||||
// `<video data-start>` inside such a host still appears "active" in the
|
||||
// raw time-window check (its own `data-start`/`data-end` cover the
|
||||
// whole clip), so without this guard we would paint a full-bleed
|
||||
// replacement frame over a sibling host that *is* visible.
|
||||
const isVisualAncestorHidden = (el: HTMLElement): boolean => {
|
||||
let parent = el.parentElement;
|
||||
while (parent !== null && parent !== document.documentElement) {
|
||||
const computed = window.getComputedStyle(parent);
|
||||
if (computed.display === "none" || computed.visibility === "hidden") return true;
|
||||
parent = parent.parentElement;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
for (const item of items) {
|
||||
const video = document.getElementById(item.videoId) as HTMLVideoElement | null;
|
||||
if (!video) continue;
|
||||
|
||||
let img = video.nextElementSibling as HTMLImageElement | null;
|
||||
const isNewImage = !img || !img.classList.contains("__render_frame__");
|
||||
const hasImg = img !== null && img.classList.contains("__render_frame__");
|
||||
|
||||
if (isVisualAncestorHidden(video)) {
|
||||
// Don't paint a frame over a hidden host — if an existing replacement
|
||||
// <img> is still around from when the host was visible, hide it so it
|
||||
// doesn't bleed through a sibling host that *is* visible on this seek.
|
||||
if (hasImg && img) img.style.visibility = "hidden";
|
||||
continue;
|
||||
}
|
||||
|
||||
const isNewImage = !hasImg;
|
||||
const computedStyle = window.getComputedStyle(video);
|
||||
// Read the GSAP-controlled opacity directly from the native <video>.
|
||||
// We hide the <video> below with `visibility: hidden` only (never
|
||||
@@ -489,12 +516,28 @@ export async function syncVideoFrameVisibility(
|
||||
activeVideoIds: string[],
|
||||
): Promise<void> {
|
||||
await page.evaluate((ids: string[]) => {
|
||||
// Mirror the ancestor-visibility guard from `injectVideoFramesBatch`: a
|
||||
// video whose host is `display:none` / `visibility:hidden` (e.g., a
|
||||
// sub-composition that the runtime has marked out-of-window) must not
|
||||
// have its replacement <img> reach `visibility:visible` here, otherwise
|
||||
// it would paint through the hidden host onto whichever sibling host is
|
||||
// currently visible.
|
||||
const isVisualAncestorHidden = (el: HTMLElement): boolean => {
|
||||
let parent = el.parentElement;
|
||||
while (parent !== null && parent !== document.documentElement) {
|
||||
const computed = window.getComputedStyle(parent);
|
||||
if (computed.display === "none" || computed.visibility === "hidden") return true;
|
||||
parent = parent.parentElement;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
const active = new Set(ids);
|
||||
const videos = Array.from(document.querySelectorAll("video[data-start]")) as HTMLVideoElement[];
|
||||
for (const video of videos) {
|
||||
const img = video.nextElementSibling as HTMLElement | null;
|
||||
const hasImg = img && img.classList.contains("__render_frame__");
|
||||
if (active.has(video.id)) {
|
||||
const ancestorHidden = isVisualAncestorHidden(video);
|
||||
if (active.has(video.id) && !ancestorHidden) {
|
||||
// Active video: show injected <img>, hide native <video>.
|
||||
// Do NOT clobber inline opacity here — GSAP-controlled opacity must
|
||||
// survive until injectVideoFramesBatch reads it via getComputedStyle.
|
||||
@@ -506,8 +549,8 @@ export async function syncVideoFrameVisibility(
|
||||
img.style.visibility = "visible";
|
||||
}
|
||||
} else {
|
||||
// Inactive video: hide both. Use visibility only (never opacity) so we
|
||||
// never clobber GSAP-controlled inline opacity.
|
||||
// Inactive (or ancestor-hidden) video: hide both. Use visibility only
|
||||
// (never opacity) so we never clobber GSAP-controlled inline opacity.
|
||||
video.style.removeProperty("display");
|
||||
video.style.setProperty("visibility", "hidden", "important");
|
||||
video.style.setProperty("pointer-events", "none", "important");
|
||||
|
||||
Reference in New Issue
Block a user