From e34e529720376685f640559fc71b53c042afb7ea Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 25 Jul 2026 22:48:00 +0200 Subject: [PATCH] fix(studio): keep gsapAnimations in sync with the keyframe cache An ungrouped tween (mixed property groups classify to propertyGroup undefined) fed keyframeCache but was skipped by every gsapAnimations writer, so the collapsed row drew diamonds the expanded lanes had no source animation to render. Drop the property-group gate at all three writers; lane consumers already filter by group. Also route the same-percentage merge in updateKeyframeCacheFromParsed through deduplicateKeyframes so the easeAmbiguous rule has one owner. --- .../hooks/gsapKeyframeCacheHelpers.test.ts | 24 ++++++++++++ .../src/hooks/gsapKeyframeCacheHelpers.ts | 37 ++++++------------- .../studio/src/hooks/useGsapTweenCache.ts | 12 +++--- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index ee72035f5..7ac1ac2ce 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -195,6 +195,30 @@ describe("updateKeyframeCacheFromParsed", () => { expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]); }); + it("records an ungrouped tween in gsapAnimations too, so the two stores agree", () => { + // `{ x, opacity }` spans two property groups, so the parser leaves + // propertyGroup undefined. Skipping it here used to cache diamonds with no + // source animation behind them: the collapsed row drew keyframes the + // expanded lanes could not render. + const animation: GsapAnimation = { + id: "mixed-box", + targetSelector: "#box", + method: "to", + position: 0, + properties: { x: 100, opacity: 0 }, + duration: 1, + resolvedStart: 0, + }; + usePlayerStore.setState({ + elements: [{ id: "box-clip", domId: "box", tag: "div", start: 0, duration: 1, track: 0 }], + }); + + updateKeyframeCacheFromParsed([animation], "scene.html", "box", {}); + + expect(cache().has("scene.html#box")).toBe(true); + expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]); + }); + it("does not cache a flat tween without animatable numeric properties", () => { const animation: GsapAnimation = { id: "flat-box", diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index 58da5c7aa..d65ef1b98 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -5,7 +5,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; import { toAbsoluteTime } from "./gsapShared"; -import { synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; +import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( animations: GsapAnimation[], @@ -23,9 +23,12 @@ export function updateKeyframeCacheFromParsed( anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? []; if (!id || kfSource.length === 0) continue; idsWithKeyframes.add(id); - if (anim.propertyGroup) { - sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); - } + // Every tween that fed keyframeCache also lands in gsapAnimations, group or + // not: a mixed-group tween (`{ x, opacity }` classifies to undefined) used to + // cache diamonds with no source animation behind them, so the collapsed row + // drew keyframes the expanded lanes couldn't render. Lane consumers do the + // group filtering themselves (animationContributesLane). + 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. @@ -51,28 +54,10 @@ export function updateKeyframeCacheFromParsed( const existing = merged.get(id); if (existing) { - const byPct = new Map(); - for (const kf of [...existing.keyframes, ...clipKeyframes]) { - const prev = byPct.get(kf.percentage); - if (prev) { - prev.properties = { ...prev.properties, ...kf.properties }; - // Mirror deduplicateKeyframes: a same-% collision across different - // source animations is an ambiguous merged segment (the button can - // only target one arbitrary animation). Flag it so the collapsed row - // suppresses the inline ease button there. - if ( - prev.animationId !== undefined && - kf.animationId !== undefined && - prev.animationId !== kf.animationId - ) { - prev.easeAmbiguous = true; - } - if (kf.ease) prev.ease = kf.ease; - } else { - byPct.set(kf.percentage, { ...kf, properties: { ...kf.properties } }); - } - } - existing.keyframes = Array.from(byPct.values()).sort((a, b) => a.percentage - b.percentage); + // deduplicateKeyframes owns the same-% merge (including the easeAmbiguous + // flag downstream lanes read); a second copy of that rule here is how the + // two writers drift. + existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]); } else { merged.set(id, { ...anim.keyframes, diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 6a8e9d548..feac2efac 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -329,9 +329,9 @@ export function useGsapAnimationsForElement( // fallow-ignore-next-line complexity useEffect(() => { if (!elementId) return; + // No property-group filter: ungrouped tweens are recorded here as well. const sourceAnimations = animations.filter( - (animation) => - animation.propertyGroup && (animation.keyframes || synthesizeFlatTweenKeyframes(animation)), + (animation) => animation.keyframes || synthesizeFlatTweenKeyframes(animation), ); if (sourceAnimations.length > 0) writeGsapAnimationsForElement(sourceFile, elementId, sourceAnimations); @@ -493,10 +493,10 @@ export function usePopulateKeyframeCacheForFile( // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { // kfData is already resolved (real keyframes OR a synthesized flat - // tween), so a grouped flat tween joins the store like a keyframed one. - if (anim.propertyGroup) { - sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); - } + // tween), so a flat tween joins the store like a keyframed one. No + // property-group filter: this map must cover every tween the cache + // 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);