From 270179d94b3ca3b82f61655b354855187f0d210c Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 20 Jul 2026 19:26:10 -0700 Subject: [PATCH] fix(studio): harden flat keyframe retiming --- .../components/editor/keyframeRetime.test.ts | 23 +++++++++++++ .../src/components/editor/keyframeRetime.ts | 14 +++++--- .../nle/useTimelineEditCallbacks.test.ts | 32 ++++++++++++++++++- .../nle/useTimelineEditCallbacks.ts | 26 +++++++-------- 4 files changed, 76 insertions(+), 19 deletions(-) diff --git a/packages/studio/src/components/editor/keyframeRetime.test.ts b/packages/studio/src/components/editor/keyframeRetime.test.ts index 5c1d9eb08..876c5129a 100644 --- a/packages/studio/src/components/editor/keyframeRetime.test.ts +++ b/packages/studio/src/components/editor/keyframeRetime.test.ts @@ -58,6 +58,29 @@ describe("resolveKeyframeRetime — move (within the tween window)", () => { expect(r.duration).toBe(3); expect(r.pctRemap).toEqual([]); }); + + it("no-ops an unexpected interior percentage on a flat tween", () => { + expect( + resolveKeyframeRetime({ + ...WINDOW, + keyframes: [], + draggedTweenPct: 50, + dropAbsTime: 5, + }).kind, + ).toBe("noop"); + }); + + it("no-ops instead of rounding a sub-10ms flat tween to zero", () => { + expect( + resolveKeyframeRetime({ + tweenStart: 2, + tweenDuration: 4, + keyframes: [], + draggedTweenPct: 100, + dropAbsTime: 2.0004, + }).kind, + ).toBe("noop"); + }); }); describe("resolveKeyframeRetime — resize (past the tween boundary)", () => { diff --git a/packages/studio/src/components/editor/keyframeRetime.ts b/packages/studio/src/components/editor/keyframeRetime.ts index 477cae600..f235b418e 100644 --- a/packages/studio/src/components/editor/keyframeRetime.ts +++ b/packages/studio/src/components/editor/keyframeRetime.ts @@ -51,6 +51,8 @@ export interface KeyframeRetimeResult { const NOOP_EPSILON_PCT = 0.1; /** Slack (seconds) for the within-tween boundary test. */ const EPSILON_TIME = 1e-4; +/** Smallest authored tween window; avoids sub-millisecond/round-to-zero durations. */ +const MIN_TWEEN_DURATION = 0.01; const round3 = (n: number) => Math.round(n * 1000) / 1000; const round1 = (n: number) => Math.round(n * 10) / 10; // 0.1% precision @@ -65,16 +67,20 @@ function resolveFlatTweenBoundaryRetime(opts: { dropAbsTime: number; }): KeyframeRetimeResult | null { const { keyframeCount, draggedTweenPct, tweenStart, tweenEnd, dropAbsTime } = opts; - if (keyframeCount > 0 || (draggedTweenPct !== 0 && draggedTweenPct !== 100)) return null; + if (keyframeCount > 0) return null; + // Flat tweens have only synthesized boundary diamonds. Never delegate an + // unexpected interior percentage to the authored-keyframe move path. + if (draggedTweenPct !== 0 && draggedTweenPct !== 100) return { kind: "noop" }; const draggedTime = draggedTweenPct === 0 ? tweenStart : tweenEnd; if (Math.abs(dropAbsTime - draggedTime) <= EPSILON_TIME) return { kind: "noop" }; const newStart = draggedTweenPct === 0 ? dropAbsTime : tweenStart; const newEnd = draggedTweenPct === 100 ? dropAbsTime : tweenEnd; - if (newEnd <= newStart + EPSILON_TIME) return { kind: "noop" }; + const newDuration = newEnd - newStart; + if (newDuration < MIN_TWEEN_DURATION) return { kind: "noop" }; return { kind: "resize", position: round3(newStart), - duration: round3(newEnd - newStart), + duration: round3(newDuration), pctRemap: [], }; } @@ -127,7 +133,7 @@ export function resolveKeyframeRetime(opts: { const newStart = Math.min(dropAbsTime, tweenStart); const newEnd = Math.max(dropAbsTime, tweenEnd); - const newDuration = Math.max(0.01, newEnd - newStart); + const newDuration = Math.max(MIN_TWEEN_DURATION, newEnd - newStart); // The dragged keyframe is the one whose tween-% is closest to draggedTweenPct. let draggedIdx = 0; diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.ts index f8dbc70d4..662234daa 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.ts @@ -40,7 +40,37 @@ describe("resolveTimelineKeyframeTarget", () => { ).toBeNull(); }); - it("prefers an explicit property-group match", () => { + it("keeps a keyframed and flat tween in the same property group unresolved", () => { + expect( + resolveTimelineKeyframeTarget( + 50, + [{ percentage: 50, tweenPercentage: 25, propertyGroup: "position" }], + [ + { id: "position-keyframed", propertyGroup: "position", keyframes: {} }, + { id: "position-flat", propertyGroup: "position" }, + ], + ), + ).toBeNull(); + }); + + it("keeps multiple ungrouped keyframed tweens unresolved", () => { + expect( + resolveTimelineKeyframeTarget( + 50, + [{ percentage: 50, tweenPercentage: 25 }], + [ + { id: "ungrouped-a", keyframes: {} }, + { id: "ungrouped-b", keyframes: {} }, + ], + ), + ).toBeNull(); + }); + + it("does not infer an animation when the rendered diamond misses the cache", () => { + expect(resolveTimelineKeyframeTarget(60, [], [FLAT_ANIMATION])).toBeNull(); + }); + + it("resolves the sole candidate in an explicit property group", () => { expect( resolveTimelineKeyframeTarget( 50, diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index a90028083..76122eb47 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -51,9 +51,10 @@ interface TimelineCachedKeyframe { /** * Resolve a rendered timeline diamond back to the animation that authored it. * Flat tweens use synthesized diamonds, so a mixed flat tween may have neither - * a property group nor real keyframes. It is safe to fall back only when that - * flat tween is the selection's sole animation; multiple candidates remain - * unresolved rather than retiming an arbitrary tween. + * a property group nor real keyframes. The cache currently carries a property + * group, not an animation id, so resolution is safe only when that group has a + * single candidate. Ambiguous candidates remain unresolved rather than + * retiming an arbitrary tween. */ export function resolveTimelineKeyframeTarget( pct: number, @@ -61,19 +62,13 @@ export function resolveTimelineKeyframeTarget( animations: ReadonlyArray, ): { animId: string; tweenPct: number } | null { const kf = keyframes.find((item) => Math.abs(item.percentage - pct) < 0.2); + if (!kf) return null; const group = kf?.propertyGroup; - const groupedCandidates = group + const candidates = group ? animations.filter((animation) => animation.propertyGroup === group) - : []; - const groupedKeyframed = groupedCandidates.find((animation) => animation.keyframes); - const soleGroupedFlat = - groupedCandidates.length === 1 && !groupedCandidates[0]?.keyframes - ? groupedCandidates[0] - : undefined; - const keyframed = animations.find((animation) => animation.keyframes); - const soleFlat = animations.length === 1 && !animations[0]?.keyframes ? animations[0] : undefined; - const animation = groupedKeyframed ?? soleGroupedFlat ?? keyframed ?? soleFlat; - return animation ? { animId: animation.id, tweenPct: kf?.tweenPercentage ?? pct } : null; + : animations.filter((animation) => !animation.propertyGroup); + const animation = candidates.length === 1 ? candidates[0] : undefined; + return animation ? { animId: animation.id, tweenPct: kf.tweenPercentage ?? pct } : null; } /** @@ -197,6 +192,9 @@ export function useTimelineEditCallbacks({ decision.pctRemap, ); } else { + // resize-keyframed-tween requires an authored `keyframes` AST node + // and intentionally no-ops for a flat tween. Update its real tween + // window through the metadata writer (and SDK cutover path) instead. handleGsapUpdateMeta(target.animId, { position: decision.position, duration: decision.duration,