From 3f0c20f633e9ac064e45df838e03d4412ddafda8 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 05:04:59 +0200 Subject: [PATCH] fix(studio): compute keyframe percentages in the tween's own time frame A sub-composition tween's resolvedStart is composition-local, while the timeline element resolved for it is the sub-comp HOST, whose start is main-timeline absolute. toClipPercentage subtracted the two frames from each other, so a host mounted at 1.5s cached its 0s tween at -12% and its last tween's end keyframe at 88% instead of 100%. A clip-relative percentage can never be negative. resolveClipTimingBasis now returns the clip start in the frame the tween's own times are measured in: the composition mount (expandedParentStart for an expanded child, the parent composition clip's start otherwise, 0 for a root-composition element) is subtracted, and a sub-comp inner element that falls back to its host's window starts at 0 in that window. It moves to gsapShared so the post-commit cache writer can share it instead of resolving its own basis, which also gives that writer the sub-comp host fallback it was missing. --- .../src/hooks/gsapKeyframeCacheHelpers.ts | 22 ++-- packages/studio/src/hooks/gsapShared.test.ts | 119 ++++++++++++++++++ packages/studio/src/hooks/gsapShared.ts | 53 ++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 37 +----- .../studio/src/hooks/useGsapTweenCache.ts | 15 +-- 5 files changed, 190 insertions(+), 56 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index 929960a16..80fb02e01 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -4,7 +4,7 @@ */ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; -import { idFromSelector, toClipKeyframes } from "./gsapShared"; +import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( @@ -13,7 +13,7 @@ export function updateKeyframeCacheFromParsed( selectionId: string | undefined, mutation: Record, ): void { - const { setKeyframeCache, elements } = usePlayerStore.getState(); + const { setKeyframeCache, elements, domClipChildren } = usePlayerStore.getState(); const idsWithKeyframes = new Set(); const merged = new Map(); const sourceAnimations = new Map(); @@ -31,16 +31,16 @@ export function updateKeyframeCacheFromParsed( sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); // Convert tween-relative percentages to clip-relative so diamonds - // render at the correct position within the timeline clip. - const timelineEl = elements.find( - (el) => el.domId === id || (el.key ?? el.id) === `${targetPath}#${id}`, - ); - const clipKeyframes = toClipKeyframes( - kfSource, - anim, - timelineEl?.start ?? 0, - timelineEl?.duration ?? 1, + // render at the correct position within the timeline clip. The basis comes + // from the shared resolver, so this writer agrees with the AST load on both + // the sub-comp host fallback and the tween's own time frame. + const { elStart, elDuration } = resolveClipTimingBasis( + id, + targetPath, + elements, + domClipChildren, ); + const clipKeyframes = toClipKeyframes(kfSource, anim, elStart, elDuration); const existing = merged.get(id); if (existing) { diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 604b93b69..98717e1a8 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -6,6 +6,7 @@ import { idSelector, isInstantHold, parsePercentageKeyframes, + resolveClipTimingBasis, resolveEditableTweenDuration, toClipKeyframes, toClipPercentage, @@ -169,6 +170,124 @@ describe("toClipKeyframes", () => { }); }); +describe("resolveClipTimingBasis", () => { + // Measured on v-product-promo: `captions-comp` mounts at 1.5s for 12.5s, and the + // six tweens inside it resolve to 0..12.5 — composition-local, not main-timeline + // absolute. Subtracting the host's 1.5 mount from a 0s tween cached pct -12. + const host = { id: "captions-comp", domId: "captions-comp", start: 1.5, duration: 12.5 }; + const children = [{ id: "line", hostId: "captions-comp" }]; + + it("gives a sub-composition inner element the host window in the tween's own frame", () => { + expect(resolveClipTimingBasis("line", "captions.html", [host], children)).toEqual({ + elStart: 0, + elDuration: 12.5, + }); + }); + + it("leaves a root-composition element on the main timeline", () => { + const box = { id: "box", domId: "box", start: 3, duration: 2 }; + expect(resolveClipTimingBasis("box", "index.html", [box], [])).toEqual({ + elStart: 3, + elDuration: 2, + }); + }); + + it("rebases an expanded sub-comp child by its host mount", () => { + // Expanded children carry host-ABSOLUTE display starts; the tweens they own are + // still composition-local, so the basis is the child's local start. + const pill = { id: "pill", domId: "pill", start: 8, duration: 4, expandedParentStart: 6 }; + expect(resolveClipTimingBasis("pill", "scene.html", [pill], [])).toEqual({ + elStart: 2, + elDuration: 4, + }); + }); + + it("rebases by the parent composition clip when the child is not expanded", () => { + const parent = { id: "scene-comp", domId: "scene-comp", start: 5, duration: 10 }; + const pill = { + id: "pill", + domId: "pill", + start: 7, + duration: 3, + parentCompositionId: "scene-comp", + }; + expect(resolveClipTimingBasis("pill", "scene.html", [parent, pill], [])).toEqual({ + elStart: 2, + elDuration: 3, + }); + }); + + it("falls back to a unit window when neither the element nor a host resolves", () => { + expect(resolveClipTimingBasis("ghost", "index.html", [], [])).toEqual({ + elStart: 0, + elDuration: 1, + }); + }); +}); + +describe("sub-composition keyframe percentages", () => { + const host = { id: "captions-comp", domId: "captions-comp", start: 1.5, duration: 12.5 }; + const children = [{ id: "line", hostId: "captions-comp" }]; + const basis = () => resolveClipTimingBasis("line", "captions.html", [host], children); + const inner = (resolvedStart: number, duration?: number) => + ({ + id: `t-${resolvedStart}`, + method: "to", + targetSelector: "#line", + vars: {}, + resolvedStart, + duration, + }) as unknown as GsapAnimation; + const percentages = (animation: GsapAnimation) => { + const { elStart, elDuration } = basis(); + return toClipKeyframes( + [{ percentage: 0 }, { percentage: 100 }], + animation, + elStart, + elDuration, + ).map((row) => row.percentage); + }; + + it("puts a tween on the host's first frame at 0%, never below zero", () => { + // A clip-relative percentage can never be negative; this one cached -12. + expect(percentages(inner(0))).toEqual([0, 100]); + }); + + it("puts the last tween's end keyframe at 100%", () => { + expect(percentages(inner(12.1, 0.4))).toEqual([96.8, 100]); + }); + + it("keeps every measured tween of the fixture inside 0..100", () => { + for (const start of [0, 3.2, 3.5, 7.7, 8, 12.1]) { + for (const percentage of percentages(inner(start, 0.4))) { + expect(percentage).toBeGreaterThanOrEqual(0); + expect(percentage).toBeLessThanOrEqual(100); + } + } + }); + + it("keeps a root-composition tween at the head of its own clip", () => { + const box = { id: "box", domId: "box", start: 3, duration: 2 }; + const { elStart, elDuration } = resolveClipTimingBasis("box", "index.html", [box], []); + const rows = toClipKeyframes([{ percentage: 0 }], inner(3, 2), elStart, elDuration); + expect(rows[0]!.percentage).toBe(0); + }); + + it("passes tween percentages through for a zero-length clip", () => { + expect(toClipKeyframes([{ percentage: 40 }], inner(0, 0.4), 0, 0)[0]!.percentage).toBe(40); + }); + + it("round-trips a clip percentage through the basis it was written with", () => { + // The drag commit converts a dropped clip-% back to a time with this basis + // (useTimelineEditCallbacks) and compares it against the tween's own + // resolvedStart, so the basis has to be in the tween's frame on both sides. + const { elStart, elDuration } = basis(); + const absTime = elStart + (40 / 100) * elDuration; + expect(absTime).toBe(5); + expect(toClipPercentage(absTime, elStart, elDuration, 0)).toBe(40); + }); +}); + describe("idFromSelector", () => { it("round-trips every shape idSelector emits", () => { for (const id of ["hero-word", "el_1", "01-hook-hero-word", "my.class", "1box", '1"x']) { diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 00c79ceda..d80db4ad3 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -259,6 +259,59 @@ export function toAbsoluteTime(tweenPos: number, tweenDur: number, percentage: n return tweenPos + (percentage / 100) * tweenDur; } +/** + * Timing basis for an element's keyframes, expressed in the TWEEN's own time + * frame. Sub-composition internals (e.g. pills inside a scene) aren't timeline + * clips themselves — they're derived at expand time — so they're absent from + * `elements`. Without a basis, elDuration defaulted to 1 and clip-relative + * keyframe percentages blew past 100% (rendering off the clip). Fall back to the + * sub-comp HOST's bounds, resolved via domClipChildren (the host's + * data-composition-src is stripped in the rendered DOM, so we can't query it). + * + * `elStart` is the clip's start in the frame the tween's own times are measured + * in. A sub-composition tween's resolvedStart is composition-local while a + * timeline element's start is main-timeline absolute, so passing the raw element + * start subtracted two different frames from each other: a host mounted at 1.5s + * cached its 0s tween at -12%, and a clip-relative percentage can never be + * negative. The composition's mount is `expandedParentStart` for an expanded + * child, the parent composition clip's start otherwise, and 0 for a + * root-composition element, whose start already IS the tween frame. + */ +export function resolveClipTimingBasis( + elementId: string, + sourceFile: string, + elements: ReadonlyArray<{ + domId?: string; + key?: string; + id: string; + start: number; + duration: number; + expandedParentStart?: number; + parentCompositionId?: string | null; + }>, + domClipChildren: ReadonlyArray<{ id: string; hostId: string }>, +): { elStart: number; elDuration: number } { + const direct = elements.find( + (el) => el.domId === elementId || (el.key ?? el.id) === `${sourceFile}#${elementId}`, + ); + if (direct) { + const parentId = direct.parentCompositionId; + const parent = parentId + ? elements.find((el) => el.domId === parentId || el.id === parentId) + : undefined; + const mount = direct.expandedParentStart ?? parent?.start ?? 0; + return { elStart: direct.start - mount, elDuration: direct.duration }; + } + const hostId = domClipChildren.find((c) => c.id === elementId)?.hostId; + const host = hostId + ? elements.find((el) => el.domId === hostId || (el.key ?? el.id) === `index.html#${hostId}`) + : undefined; + // The inner element is not a clip of its own: the host's window IS the frame + // its tweens are timed in, so the start in that frame is 0, not the host's + // main-timeline mount. + return { elStart: 0, elDuration: host?.duration ?? 1 }; +} + /** * An absolute time as a percentage of a timeline clip, at the one precision every * keyframe-cache writer must share. 0.001% keeps a beat-snapped keyframe centered diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index bca5f4b23..f940d6160 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -1,6 +1,6 @@ /** - * Reading a composition file's GSAP tweens into the keyframe cache: fetch, - * selector -> element id resolution, and the clip-relative timing basis. + * Reading a composition file's GSAP tweens into the keyframe cache: fetch and + * selector -> element id resolution. * Split from useGsapTweenCache to keep that file under the 600-line limit. */ import type { GsapAnimation, GsapKeyframesData, ParsedGsap } from "@hyperframes/core/gsap-parser"; @@ -11,7 +11,7 @@ import { elementCacheKeys, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { idFromSelector, toClipKeyframes } from "./gsapShared"; +import { idFromSelector, resolveClipTimingBasis, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, @@ -103,37 +103,6 @@ export async function fetchParsedAnimations( } } -/** - * Clip-relative timing basis for an element. Sub-composition internals (e.g. pills - * inside a scene) aren't timeline clips themselves — they're derived at expand time - * — so they're absent from `elements`. Without a basis, elDuration defaulted to 1 - * and clip-relative keyframe percentages blew past 100% (rendering off the clip). - * Fall back to the sub-comp HOST's bounds, resolved via domClipChildren (the host's - * data-composition-src is stripped in the rendered DOM, so we can't query it). - */ -export function resolveClipTimingBasis( - elementId: string, - sourceFile: string, - elements: ReadonlyArray<{ - domId?: string; - key?: string; - id: string; - start: number; - duration: number; - }>, - domClipChildren: ReadonlyArray<{ id: string; hostId: string }>, -): { elStart: number; elDuration: number } { - const direct = elements.find( - (el) => el.domId === elementId || (el.key ?? el.id) === `${sourceFile}#${elementId}`, - ); - if (direct) return { elStart: direct.start, elDuration: direct.duration }; - const hostId = domClipChildren.find((c) => c.id === elementId)?.hostId; - const host = hostId - ? elements.find((el) => el.domId === hostId || (el.key ?? el.id) === `index.html#${hostId}`) - : undefined; - return { elStart: host?.start ?? 0, elDuration: host?.duration ?? 1 }; -} - /** * Read one composition file's tweens into the keyframe cache. Split out of the * hook so the effect can run it per file without re-nesting the whole body. diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 0f89be655..23a8ea728 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -7,24 +7,17 @@ import { pruneKeyframeCacheToFiles, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime, toClipPercentage } from "./gsapShared"; +import { resolveClipTimingBasis, toAbsoluteTime, toClipPercentage } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, synthesizeFlatTweenKeyframes, } from "./gsapTweenSynth"; -import { - fetchParsedAnimations, - populateKeyframeCacheFromAst, - resolveClipTimingBasis, -} from "./keyframeCacheAstLoad"; +import { fetchParsedAnimations, populateKeyframeCacheFromAst } from "./keyframeCacheAstLoad"; // Re-exported so callers keep importing the GSAP cache surface from one module. -export { - fetchParsedAnimations, - resolveClipTimingBasis, - resolveSelectorElementIds, -} from "./keyframeCacheAstLoad"; +export { resolveClipTimingBasis } from "./gsapShared"; +export { fetchParsedAnimations, resolveSelectorElementIds } from "./keyframeCacheAstLoad"; /** The selected element's identity for matching tweens to it. */ export interface GsapElementTarget {