mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-04 07:19:52 +00:00
fix(engine): disqualify static-frame dedup on any tl.call()
Real bug report: a mono count span driven by a GSAP tl.call() (a counter going "0 sur 0" -> "1 sur 1" at a later beat) rendered the LATER value baked in from frame 0 of an EARLIER, unrelated static-hold span, despite the dedup log reporting "verified". Root cause: computeStaticFrameSet's tween walker only tracks property tweens, so a call()-driven textContent mutation carries no tracked interval and the span around it looks fully static. verifyStaticFramesSafe does catch genuine drift WITHIN a run it's checking, but a call() is a one-shot side effect wired as both onComplete and onReverseComplete (GSAP has no separate "undo" — crossing it in either direction fires the SAME forward mutation). Verifying a LATER run forward-seeks past the call(), permanently mutating the live page; an EARLIER run already passed its own check before that happened, so nothing re-verifies it afterward. Real capture then starts on the same corrupted page and bakes the wrong value into the earlier span's reused buffer. No reliable way to tell a DOM-mutating call() from a harmless one (analytics ping, class toggle) without executing it, so this disqualifies the whole comp on ANY call() — conservative, costs some dedup perf on comps that use call() harmlessly, but correctness over speed.
This commit is contained in:
@@ -0,0 +1,68 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { computeStaticFrameSet } from "./frameCapture.js";
|
||||
|
||||
/**
|
||||
* Regression lock: a GSAP `tl.call()` disqualifies a composition from
|
||||
* static-frame dedup, even though the tween walker can't see it as an
|
||||
* "animated" interval (a call() carries no property change to track).
|
||||
*
|
||||
* `tl.call()` is a zero-duration tween whose vars wire the callback as
|
||||
* `onComplete` (and `onReverseComplete` — GSAP fires the SAME forward side
|
||||
* effect on backward crossing too, there is no separate "undo"). A one-shot
|
||||
* DOM mutation driven this way (e.g. a counter's textContent) is not
|
||||
* seek-idempotent: the static-dedup verifier's own arm-time seeking can
|
||||
* permanently fire it while checking a LATER run, corrupting the page for an
|
||||
* EARLIER, unrelated run's real capture — even though each run's own
|
||||
* verification passes in isolation, so "verified" still gets logged. Real
|
||||
* incident: tools-onboarding FR render, beat-1 title card baked in beat-6's
|
||||
* counter value from frame 0 onward.
|
||||
*/
|
||||
describe("computeStaticFrameSet disqualifies a comp containing a tl.call()", () => {
|
||||
function makePage(evalResult: Record<string, unknown>) {
|
||||
return {
|
||||
evaluate: vi
|
||||
.fn()
|
||||
// First call: the main computeStaticFrameSet in-page scan.
|
||||
.mockResolvedValueOnce(evalResult)
|
||||
// Second call: computeClipBoundaryFrames' own [data-start] scan.
|
||||
.mockResolvedValueOnce([]),
|
||||
} as unknown as Parameters<typeof computeStaticFrameSet>[0];
|
||||
}
|
||||
|
||||
it("is ineligible when a tl.call() is present, even with zero tracked tween intervals", async () => {
|
||||
const page = makePage({
|
||||
intervals: [],
|
||||
tweenCount: 1,
|
||||
duration: 10,
|
||||
hasVideo: false,
|
||||
hasCanvas: false,
|
||||
hasNonGsapAnim: false,
|
||||
hasUnresolvableClipStart: false,
|
||||
hasTimelineCall: true,
|
||||
});
|
||||
|
||||
const result = await computeStaticFrameSet(page, 30);
|
||||
|
||||
expect(result.eligible).toBe(false);
|
||||
expect(result.reason).toContain("tl.call()");
|
||||
expect(result.staticFrameSet.size).toBe(0);
|
||||
});
|
||||
|
||||
it("stays eligible on an otherwise-identical comp with no tl.call()", async () => {
|
||||
const page = makePage({
|
||||
intervals: [],
|
||||
tweenCount: 1,
|
||||
duration: 10,
|
||||
hasVideo: false,
|
||||
hasCanvas: false,
|
||||
hasNonGsapAnim: false,
|
||||
hasUnresolvableClipStart: false,
|
||||
hasTimelineCall: false,
|
||||
});
|
||||
|
||||
const result = await computeStaticFrameSet(page, 30);
|
||||
|
||||
expect(result.eligible).toBe(true);
|
||||
expect(result.staticFrameSet.size).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
@@ -1960,7 +1960,7 @@ async function computeClipBoundaryFrames(page: Page, fps: number): Promise<Set<n
|
||||
* comp on any signal the tween-walker can't see: video / canvas / webgl (redraw without
|
||||
* a tween), zero tweens (non-GSAP animation), or a running CSS/WAAPI animation.
|
||||
*/
|
||||
async function computeStaticFrameSet(
|
||||
export async function computeStaticFrameSet(
|
||||
page: Page,
|
||||
fps: number,
|
||||
): Promise<{
|
||||
@@ -1979,11 +1979,23 @@ async function computeStaticFrameSet(
|
||||
duration(): number;
|
||||
totalDuration?(): number;
|
||||
getChildren?(nested: boolean, tweens: boolean, timelines: boolean): AnyTween[];
|
||||
vars?: Record<string, unknown>;
|
||||
};
|
||||
const intervals: Array<{ start: number; end: number }> = [];
|
||||
let tweenCount = 0;
|
||||
// totalDuration() (NOT duration()): a repeat/yoyo tween animates past one iteration;
|
||||
// a repeating timeline is marked opaque over its whole span (conservative).
|
||||
// A GSAP tl.call() is a zero-duration tween whose vars wire the callback as
|
||||
// onComplete (and onReverseComplete, fired on backward crossing — GSAP has
|
||||
// no separate "undo" callback, so both directions invoke the SAME forward
|
||||
// side effect). A one-shot DOM mutation driven this way (e.g. a counter's
|
||||
// textContent) is not seek-idempotent: crossing it during the static-dedup
|
||||
// verifier's own arm-time seeking permanently mutates the page, and that
|
||||
// corruption can leak into a LATER, unrelated static run's real capture
|
||||
// (the verifier's mismatch check only catches drift within the run being
|
||||
// checked, not contamination from a run checked afterward). No reliable
|
||||
// way to tell a DOM-mutating call() from a harmless one (analytics ping,
|
||||
// class toggle with no visual effect) without executing it, so disqualify
|
||||
// the whole comp on ANY call() rather than risk shipping wrong pixels.
|
||||
let hasTimelineCall = false;
|
||||
function walk(tl: AnyTween, offset: number): void {
|
||||
if (typeof tl.getChildren !== "function") return;
|
||||
for (const child of tl.getChildren(false, true, true)) {
|
||||
@@ -1996,6 +2008,13 @@ async function computeStaticFrameSet(
|
||||
} else {
|
||||
tweenCount++;
|
||||
intervals.push({ start, end: start + total });
|
||||
if (
|
||||
total <= 1e-6 &&
|
||||
(typeof child.vars?.onComplete === "function" ||
|
||||
typeof child.vars?.onReverseComplete === "function")
|
||||
) {
|
||||
hasTimelineCall = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2039,6 +2058,7 @@ async function computeStaticFrameSet(
|
||||
hasCanvas,
|
||||
hasNonGsapAnim,
|
||||
hasUnresolvableClipStart,
|
||||
hasTimelineCall,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -2050,6 +2070,7 @@ async function computeStaticFrameSet(
|
||||
hasCanvas,
|
||||
hasNonGsapAnim,
|
||||
hasUnresolvableClipStart,
|
||||
hasTimelineCall,
|
||||
} = result as {
|
||||
intervals: Array<{ start: number; end: number }>;
|
||||
tweenCount: number;
|
||||
@@ -2058,6 +2079,7 @@ async function computeStaticFrameSet(
|
||||
hasCanvas: boolean;
|
||||
hasNonGsapAnim: boolean;
|
||||
hasUnresolvableClipStart: boolean;
|
||||
hasTimelineCall: boolean;
|
||||
};
|
||||
const totalFrames = Math.max(1, Math.ceil(duration * fps));
|
||||
const animated = new Set<number>();
|
||||
@@ -2073,6 +2095,12 @@ async function computeStaticFrameSet(
|
||||
if (hasCanvas) reasons.push("canvas/webgl");
|
||||
if (tweenCount === 0) reasons.push("no GSAP tweens (non-GSAP animation)");
|
||||
if (hasNonGsapAnim) reasons.push("running CSS/WAAPI animation");
|
||||
// tl.call() side effects are not seek-idempotent (see hasTimelineCall detection
|
||||
// above) — the arm-time verifier's own forward-seeking can permanently fire
|
||||
// one, corrupting the page for a later, unrelated static run's real capture
|
||||
// even though each run's own verification passes in isolation (HF static-
|
||||
// dedup content-drift report, tools-onboarding FR render).
|
||||
if (hasTimelineCall) reasons.push("tl.call() side effect (not seek-safe)");
|
||||
if (hasUnresolvableClipStart) reasons.push("unresolvable clip start (reference expression)");
|
||||
const eligible = reasons.length === 0;
|
||||
const staticFrameSet = new Set<number>();
|
||||
|
||||
Reference in New Issue
Block a user