From acf6766ed8bc80e5a309b4b034c528999eb011bb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 00:51:19 +0200 Subject: [PATCH] refactor(studio): one owner for clip-relative keyframe rows Each keyframe-cache writer re-derived a clip-relative percentage inline, and the post-commit writer rounded to 0.1% while the others used 0.001%. Selection keys embed that number, so a commit-time rewrite could orphan a live key. toClipPercentage owns the rounding, toClipKeyframes owns the whole row (percentage plus the tween percentage and animation identity the lanes read), and the parsed write reuses elementCacheKeys instead of open-coding the three key variants. --- .../src/hooks/gsapKeyframeCacheHelpers.ts | 28 +++------- packages/studio/src/hooks/gsapShared.test.ts | 21 +++++++- packages/studio/src/hooks/gsapShared.ts | 51 +++++++++++++++++++ .../studio/src/hooks/useGsapTweenCache.ts | 29 ++--------- 4 files changed, 82 insertions(+), 47 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index d65ef1b98..d6a1e3a82 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 { toAbsoluteTime } from "./gsapShared"; +import { toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( @@ -32,25 +32,15 @@ export function updateKeyframeCacheFromParsed( // Convert tween-relative percentages to clip-relative so diamonds // render at the correct position within the timeline clip. - const tweenPos = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDur = anim.duration ?? 1; const timelineEl = elements.find( (el) => el.domId === id || (el.key ?? el.id) === `${targetPath}#${id}`, ); - const elStart = timelineEl?.start ?? 0; - const elDuration = timelineEl?.duration ?? 1; - const clipKeyframes = kfSource.map((kf) => { - const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - const clipPct = - elDuration > 0 ? Math.round(((absTime - elStart) / elDuration) * 1000) / 10 : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - animationId: anim.id, - }; - }); + const clipKeyframes = toClipKeyframes( + kfSource, + anim, + timelineEl?.start ?? 0, + timelineEl?.duration ?? 1, + ); const existing = merged.get(id); if (existing) { @@ -67,9 +57,7 @@ export function updateKeyframeCacheFromParsed( } } for (const [id, entry] of merged) { - setKeyframeCache(`${targetPath}#${id}`, entry); - setKeyframeCache(id, entry); - if (targetPath !== "index.html") setKeyframeCache(`index.html#${id}`, entry); + for (const key of elementCacheKeys(targetPath, id)) setKeyframeCache(key, entry); writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id)); } const targetId = diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 30ed300a0..1d3fc8f36 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; -import { idSelector, isInstantHold, parsePercentageKeyframes } from "./gsapShared"; +import { + idSelector, + isInstantHold, + parsePercentageKeyframes, + toClipPercentage, +} from "./gsapShared"; describe("isInstantHold", () => { const animation = (method: GsapAnimation["method"], duration?: number) => @@ -103,3 +108,17 @@ describe("idSelector", () => { } }); }); + +describe("toClipPercentage", () => { + // Selection keys embed this number, so every keyframe-cache writer has to round + // it identically: a coarser writer rewrites the cache with a different value and + // orphans the live selection key built from the finer one. + it("keeps three decimals so a beat-snapped keyframe lands on its beat", () => { + expect(toClipPercentage(1 / 3, 0, 1, 0)).toBe(33.333); + expect(toClipPercentage(2.5, 2, 4, 0)).toBe(12.5); + }); + + it("passes the tween percentage through for a zero-length clip", () => { + expect(toClipPercentage(5, 0, 0, 42)).toBe(42); + }); +}); diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 629e5976d..fdfcc0aae 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -216,3 +216,54 @@ export function parsePercentageKeyframes( export function toAbsoluteTime(tweenPos: number, tweenDur: number, percentage: number): number { return tweenPos + (percentage / 100) * tweenDur; } + +/** + * 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 + * on the beat dot, and because selection keys embed this number, a writer that + * rounds coarser would orphan a live selection the moment it rewrites the cache. + * A zero-length clip has no percentage to give, so the tween-% passes through. + */ +export function toClipPercentage( + absoluteTime: number, + clipStart: number, + clipDuration: number, + fallbackPercentage: number, +): number { + if (clipDuration <= 0) return fallbackPercentage; + return Math.round(((absoluteTime - clipStart) / clipDuration) * 100000) / 1000; +} + +/** + * One keyframe-cache row per tween keyframe: the percentage re-based onto the + * clip, the original tween percentage kept alongside it, and the animation + * identity every lane and selection key needs. Shared by the cache writers so + * they cannot drift in precision or in which identity fields they record. + */ +export function toClipKeyframes( + source: readonly T[], + anim: GsapAnimation, + clipStart: number, + clipDuration: number, +): Array< + T & { + tweenPercentage: number; + propertyGroup: GsapAnimation["propertyGroup"]; + animationId: string; + } +> { + const tweenStart = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); + const tweenDuration = anim.duration ?? 1; + return source.map((keyframe) => ({ + ...keyframe, + percentage: toClipPercentage( + toAbsoluteTime(tweenStart, tweenDuration, keyframe.percentage), + clipStart, + clipDuration, + keyframe.percentage, + ), + tweenPercentage: keyframe.percentage, + propertyGroup: anim.propertyGroup, + animationId: anim.id, + })); +} diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index feac2efac..766b0f39c 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -8,7 +8,7 @@ import { clearKeyframeCacheForFile, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime } from "./gsapShared"; +import { toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; function extractIdFromSelector(selector: string): string | null { @@ -378,12 +378,7 @@ export function useGsapAnimationsForElement( const tweenDur = anim.duration ?? elDuration; for (const k of kf.keyframes) { const absTime = toAbsoluteTime(tweenPos, tweenDur, k.percentage); - // 0.001% precision (was 0.1%) so a beat-snapped keyframe centers exactly - // on the beat dot, which is rendered at the true beat time. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : k.percentage; + const clipPct = toClipPercentage(absTime, elStart, elDuration, k.percentage); allKeyframes.push({ ...k, percentage: clipPct, @@ -486,9 +481,6 @@ export function usePopulateKeyframeCacheForFile( } const kfData = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim); if (!kfData) continue; - const tweenPos = - anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDur = anim.duration ?? 1; // Attribute the tween to every element it animates (handles class / // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { @@ -498,22 +490,7 @@ export function usePopulateKeyframeCacheForFile( // below records, or expanded lanes have nothing to render. sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren); - const clipKeyframes = kfData.keyframes.map((kf) => { - const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - // 0.001% precision (see useGsapAnimationsForElement) so a beat-snapped - // keyframe centers on the beat dot and both caches agree. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - animationId: anim.id, // parity with other cache writers; inline ease needs it - }; - }); + const clipKeyframes = toClipKeyframes(kfData.keyframes, anim, elStart, elDuration); const existing = mergedByElement.get(id); if (existing) { existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]);