refactor(studio): share keyframe clip-% precision across lane writers

Review follow-ups on the expanded-lanes tip:

- TimelinePropertyLanes.groupKeyframes re-derived the clip-relative
  percentage inline and skipped toClipKeyframes' rounding, the one
  precision every keyframe-cache writer has to agree on (selection keys
  embed the number). It now loops the shared helper per animation with
  the group filter and only overrides the lane's own group and ease.
- TimelineClipDiamonds rebuilt and re-sorted each tween's sibling row
  inside the marker loop, so a row of N diamonds allocated N sorted
  copies of itself on every playhead tick. Built once per render instead,
  keyed by animation id.
- Add the missing symmetry test at the real callback boundary: the
  diamond test mocks onMoveKeyframe, so nothing proved the rapid-second
  retime resolves a pending clip-% the keyframe cache has not caught up
  to. The new test asserts the identity-carrying target retimes and that
  the same drag without identity fields cannot.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-28 00:40:47 +02:00
parent e317f1fbe3
commit 3181657147
3 changed files with 97 additions and 23 deletions
@@ -499,6 +499,63 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
view.unmount();
});
// The diamond's rapid-second-retime path reports the PENDING clip-% (where the
// first drag put the keyframe), which the keyframe cache has not caught up to.
// TimelineClipDiamonds' own test mocks onMoveKeyframe, so only this one proves
// the real callback resolves that stale-cache position off the identity fields
// instead of failing the lookup.
it("retimes from a pending position the keyframe cache has not caught up to", async () => {
const authored = authoredInteriorAnimation();
mocks.animations = [authored];
usePlayerStore.setState({
elements: [element],
gsapAnimations: new Map([["index.html#box", [authored]]]),
// Still the pre-drag positions: 75% is not in here.
keyframeCache: new Map([
[
"index.html#box",
{
format: "percentage" as const,
keyframes: [
{ percentage: 0, properties: { x: 0 } },
{ percentage: 50, properties: { x: 210 } },
{ percentage: 100, properties: { x: 420 } },
],
},
],
]),
});
const view = renderCallbacks();
await expect(
view.callbacks.onMoveKeyframe?.(
"index.html#box",
{
percentage: 75,
propertyGroup: "position",
tweenPercentage: 50,
animationId: authored.id,
},
85,
),
).resolves.toBe(true);
expect(mocks.actions.handleGsapMoveKeyframe).toHaveBeenCalledWith(
authored.id,
50,
85,
mocks.selection,
);
// Control: the same drag WITHOUT the identity fields falls back to the cache
// lookup, finds nothing at 75%, and cannot retime.
mocks.actions.handleGsapMoveKeyframe.mockClear();
await expect(
view.callbacks.onMoveKeyframe?.("index.html#box", { percentage: 75 }, 85),
).resolves.toBe(false);
expect(mocks.actions.handleGsapMoveKeyframe).not.toHaveBeenCalled();
view.unmount();
});
it("uses the clip timing basis when retiming a duration-less tween", async () => {
const durationless = {
...authoredInteriorAnimation(),
@@ -149,6 +149,29 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
keyframe.animationId === undefined
? sorted
: sorted.filter((k) => k.animationId === keyframe.animationId);
// Compose each sibling's pending destination in first: clamping against
// cached positions while the dragged keyframe reads its pending one let a
// second drag cross a neighbour that had already moved past it. Built once
// per render, keyed by tween: every diamond of a row needs the same row, and
// rebuilding + re-sorting it inside the marker loop below made this
// O(keyframes squared) allocations on every playhead tick.
const pendingClipPctOf = (keyframe: TimelineDiamondKeyframe) =>
pendingRetimes.get(timelineKeyframeSelectionKey(elementId, keyframeTarget(keyframe)))
?.clipPct ?? keyframe.percentage;
const siblingRows = new Map<
string | undefined,
{ keyframes: TimelineDiamondKeyframe[]; clipPcts: number[] }
>();
for (const keyframe of sorted) {
if (siblingRows.has(keyframe.animationId)) continue;
const row = siblingRowOf(keyframe)
.map((k) => ({ keyframe: k, clipPct: pendingClipPctOf(k) }))
.sort((a, b) => a.clipPct - b.clipPct);
siblingRows.set(keyframe.animationId, {
keyframes: row.map((s) => s.keyframe),
clipPcts: row.map((s) => s.clipPct),
});
}
const centerXOf = (percentage: number) =>
Math.max(0, Math.min(clipWidthPx, (percentage / 100) * clipWidthPx));
// One record per diamond, carrying its own geometry, so the connector and
@@ -202,19 +225,9 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({
const target = keyframeTarget(kf);
const kfKey = timelineKeyframeSelectionKey(elementId, target);
// Clamp against this keyframe's own tween, not the whole merged row.
// Compose each sibling's pending destination in first: clamping against
// cached positions while the dragged keyframe reads its pending one let
// a second drag cross a neighbour that had already moved past it.
const siblingRow = siblingRowOf(kf)
.map((k) => ({
keyframe: k,
clipPct:
pendingRetimes.get(timelineKeyframeSelectionKey(elementId, keyframeTarget(k)))
?.clipPct ?? k.percentage,
}))
.sort((a, b) => a.clipPct - b.clipPct);
const siblingClipPcts = siblingRow.map((s) => s.clipPct);
const siblingIndex = siblingRow.findIndex((s) => s.keyframe === kf);
const siblingRow = siblingRows.get(kf.animationId);
const siblingClipPcts = siblingRow?.clipPcts ?? [];
const siblingIndex = siblingRow?.keyframes.indexOf(kf) ?? -1;
// While dragging this diamond, render it at the live preview clip-%.
const renderPct = preview?.kfKey === kfKey ? preview.clipPct : kf.percentage;
// Center the marker's non-overlapping hit region ON its keyframe %, so
@@ -4,7 +4,7 @@ import {
type GsapAnimation,
type PropertyGroupName,
} from "@hyperframes/core/gsap-parser";
import { toAbsoluteTime } from "../../hooks/gsapShared";
import { toClipKeyframes } from "../../hooks/gsapShared";
import { synthesizeFlatTweenKeyframes } from "../../hooks/gsapTweenSynth";
import { TimelineDiamondLane, type TimelineDiamondKeyframe } from "./TimelineClipDiamonds";
import { LANE_H, getTimelineLaneTop } from "./timelineLayout";
@@ -67,6 +67,13 @@ function keyframeEase(keyframe: { ease?: string }, animation: GsapAnimation): st
return keyframe.ease ?? animation.keyframes?.easeEach ?? animation.ease;
}
/**
* One lane row per keyframe of `group`. The clip-% re-basing goes through the
* shared toClipKeyframes so lane rows land on the exact same percentage the
* keyframe cache writes: this file used to derive it inline and skipped that
* helper's rounding, which is the one precision every keyframe-cache writer has
* to agree on (selection keys embed the number).
*/
function groupKeyframes(
animations: readonly GsapAnimation[],
group: PropertyGroupName,
@@ -75,18 +82,15 @@ function groupKeyframes(
): TimelineDiamondKeyframe[] {
const keyframes: TimelineDiamondKeyframe[] = [];
for (const animation of animations) {
const tweenStart =
animation.resolvedStart ?? (typeof animation.position === "number" ? animation.position : 0);
const tweenDuration = animation.duration ?? clipDuration;
for (const keyframe of animationKeyframes(animation)) {
if (!hasGroupProperty(keyframe.properties, group)) continue;
const absoluteTime = toAbsoluteTime(tweenStart, tweenDuration, keyframe.percentage);
const inGroup = animationKeyframes(animation).filter((keyframe) =>
hasGroupProperty(keyframe.properties, group),
);
for (const keyframe of toClipKeyframes(inGroup, animation, clipStart, clipDuration)) {
keyframes.push({
...keyframe,
percentage: ((absoluteTime - clipStart) / clipDuration) * 100,
tweenPercentage: keyframe.percentage,
// The LANE's group, not the tween's own classification: a mixed-property
// tween classifies to undefined yet still feeds every group it touches.
propertyGroup: group,
animationId: animation.id,
ease: keyframeEase(keyframe, animation),
});
}