From 1a5bb477396e35c98ca28f767e72be239c8a89b7 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 26 Jun 2026 22:22:49 -0400 Subject: [PATCH] fix(studio): keyframe commit routing for 3D and cross-group edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pickBestAnimation is group-aware: a rotation/3D edit no longer merges into a position tween — a fresh same-group tween with a 0% baseline is created instead - editing at a playhead past the tween extends it and keyframes there (matches drag) - update-keyframe MERGES into the existing keyframe instead of overwriting, so editing one property no longer drops z/transformPerspective (the lens then animated from 0 and the element popped) - dragging a keyframed element with a constant position tween keyframes rather than writing a static set --- packages/core/src/runtime/init.ts | 17 ++ packages/parsers/src/gsapParser.ts | 17 ++ packages/parsers/src/gsapWriterAcorn.ts | 18 ++- .../components/editor/MotionPathOverlay.tsx | 32 +++- .../src/components/editor/PropertyPanel.tsx | 65 ++++---- .../src/components/editor/Transform3DCube.tsx | 30 +++- .../src/components/editor/manualOffsetDrag.ts | 25 ++- .../editor/propertyPanel3dTransform.tsx | 110 +++++++------ .../components/editor/useMotionPathData.ts | 48 +++++- .../studio/src/hooks/gsapRuntimeBridge.ts | 11 +- .../src/hooks/useAnimatedPropertyCommit.ts | 152 +++++++++++++++--- .../src/hooks/useEnableKeyframes.test.ts | 35 ++++ .../studio/src/hooks/useEnableKeyframes.ts | 30 +++- 13 files changed, 466 insertions(+), 124 deletions(-) diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index f02de66d51..8fedc633b4 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -97,6 +97,23 @@ export function initSandboxRuntimeModular(): void { swallow("runtime.init.site1", err); } } + // `_auto` is a Studio-internal keyframe marker (an auto-tracked endpoint the + // parser reads back), NOT an animatable property. Register it as a no-op GSAP + // plugin so GSAP doesn't log "Invalid property _auto" on every tween build — + // that per-frame warning destabilizes the preview and makes the selection + // overlay stop tracking the pointer. Idempotent + best-effort. + const ensureAutoMarkerNoop = (): void => { + const g = window.gsap as { registerPlugin?: (plugin: unknown) => void } | undefined; + const w = window as Window & { __hfAutoNoopRegistered?: boolean }; + if (!g?.registerPlugin || w.__hfAutoNoopRegistered) return; + try { + g.registerPlugin({ name: "_auto", init: () => false }); + w.__hfAutoNoopRegistered = true; + } catch { + // a stray warning is preferable to a broken runtime + } + }; + ensureAutoMarkerNoop(); // Normalize html/body so browser defaults (8px margin, white background) never // bleed into renders as white bars. Runs in both preview and render contexts, // eliminating the preview/render parity gap that existed when only the React diff --git a/packages/parsers/src/gsapParser.ts b/packages/parsers/src/gsapParser.ts index 96ffd43e4f..4702a0d4f7 100644 --- a/packages/parsers/src/gsapParser.ts +++ b/packages/parsers/src/gsapParser.ts @@ -2348,6 +2348,23 @@ export function updateKeyframeInScript( // `properties` would wipe the primitive — leave the keyframe untouched. return script; } + // MERGE edited props into the existing keyframe, preserving props not in this edit + // (z, transformPerspective, rotation, …). A whole-value rebuild drops them, so editing + // one prop at the 0% keyframe strips z/transformPerspective and the element pops. + // Mirrors acorn updateKeyframeInScript; parity-locked by gsapWriterParity.corpus. + const existing = match.prop.value; + if (existing?.type === "ObjectExpression") { + const props = (existing.properties ?? []) as AstNode[]; + const upsert = (key: string, valueCode: string) => { + const idx = props.findIndex((p: AstNode) => isObjectProperty(p) && propKeyName(p) === key); + const node = parseExpr(`({ ${safeKey(key)}: ${valueCode} })`).properties[0]; + if (idx >= 0) props[idx] = node; + else props.push(node); + }; + for (const [k, v] of Object.entries(properties)) upsert(k, valueToCode(v)); + if (ease !== undefined) upsert("ease", JSON.stringify(ease)); + return recast.print(loc.parsed.ast).code; + } match.prop.value = buildKeyframeValueNode(properties, ease); return recast.print(loc.parsed.ast).code; } diff --git a/packages/parsers/src/gsapWriterAcorn.ts b/packages/parsers/src/gsapWriterAcorn.ts index b29b77c32e..4927be9ae7 100644 --- a/packages/parsers/src/gsapWriterAcorn.ts +++ b/packages/parsers/src/gsapWriterAcorn.ts @@ -780,10 +780,22 @@ export function updateKeyframeInScript( const match = findKfPropByPct(kfPropNode.value, percentage); if (!match) return script; - const record: Record = { ...properties }; - if (ease) record.ease = ease; const ms = new MagicString(script); - ms.overwrite(match.prop.value.start, match.prop.value.end, recordToCode(record)); + // MERGE the edited props into the existing keyframe, preserving properties already + // keyframed at this percentage (z, transformPerspective, rotation, …). A whole-value + // overwrite DROPS every prop not in this edit — e.g. editing rotationY at the 0% + // keyframe would strip z / transformPerspective, so the lens then animates from 0 and + // the element pops. Mirrors addKeyframeToScript's merge-into-existing branch. + if (match.prop.value?.type === "ObjectExpression") { + for (const [k, v] of Object.entries(properties)) { + upsertProp(ms, match.prop.value, k, v); + } + if (ease !== undefined) upsertProp(ms, match.prop.value, "ease", ease); + } else { + const record: Record = { ...properties }; + if (ease) record.ease = ease; + ms.overwrite(match.prop.value.start, match.prop.value.end, recordToCode(record)); + } return ms.toString(); } diff --git a/packages/studio/src/components/editor/MotionPathOverlay.tsx b/packages/studio/src/components/editor/MotionPathOverlay.tsx index 4a27cfaca3..038538717c 100644 --- a/packages/studio/src/components/editor/MotionPathOverlay.tsx +++ b/packages/studio/src/components/editor/MotionPathOverlay.tsx @@ -21,6 +21,7 @@ import { elementHome, hasMotionPathPlugin, isPreviewHtmlElement, + transformWDivisor, useMotionPathData, } from "./useMotionPathData"; @@ -39,6 +40,7 @@ type DragState = { initX: number; initY: number; scale: number; + pScale: number; ref: MotionNodeRef; }; @@ -71,7 +73,7 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ handleGsapRemoveKeyframe, handleGsapDeleteAllForElement, } = useDomEditContext(); - const { rect, geometry, geometryResolved, visibleInPreview, home } = useMotionPathData( + const { rect, geometry, geometryResolved, visibleInPreview, home, pScale } = useMotionPathData( iframeRef, selectorFor(selection), ); @@ -156,8 +158,12 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ e.preventDefault(); const sc = r.width / compW; const elHome = elementHome(live); - const px = Math.round((e.clientX - r.left) / sc - elHome.x); - const py = Math.round((e.clientY - r.top) / sc - elHome.y); + // De-magnify: the click lands on the projected (1/m44-magnified) path, so + // divide the home-relative offset by the perspective factor to recover the + // stored composition offset (inverse of the `* pScale` applied at draw). + const ps = 1 / transformWDivisor(live); + const px = Math.round(((e.clientX - r.left) / sc - elHome.x) / ps); + const py = Math.round(((e.clientY - r.top) / sc - elHome.y) / ps); const t = Math.round(usePlayerStore.getState().currentTime * 100) / 100; void commitCreatePath(createSelector, t, px, py, commitMutation); setMotionPathArmed(false); @@ -232,7 +238,16 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ : geometry.nodes; // ax/ay = absolute composition position (home + offset) for drawing; n.x/n.y // stay offsets so the drag commit writes the right tween values. - const abs = nodes.map((n) => ({ ...n, ax: home.x + n.x, ay: home.y + n.y })); + // Magnify the animated offsets by the element's perspective factor (1/m44, via + // pScale) so the path tracks the *projected* element. `home` is the projection + // pivot (transform-origin), so it stays put; only the offsets foreshorten. 2D + // elements have pScale = 1 (no change). Inverse (de-magnify) applied wherever a + // pointer position is mapped back to a stored offset (create + node drag). + const abs = nodes.map((n) => ({ + ...n, + ax: home.x + n.x * pScale, + ay: home.y + n.y * pScale, + })); const points = abs.map((p) => `${p.ax},${p.ay}`).join(" "); // Map a VIEWPORT pointer to composition space. Use the iframe's LIVE viewport // rect, not `rect` — `rect.left/top` are stored pan-surface-relative (for the @@ -264,6 +279,7 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ initX: x, initY: y, scale, + pScale, ref, }; setDraft({ index, x, y }); @@ -273,8 +289,8 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ if (!d) return; setDraft({ index: d.index, - x: d.initX + (e.clientX - d.startX) / d.scale, - y: d.initY + (e.clientY - d.startY) / d.scale, + x: d.initX + (e.clientX - d.startX) / d.scale / d.pScale, + y: d.initY + (e.clientY - d.startY) / d.scale / d.pScale, }); }; // fallow-ignore-next-line complexity @@ -286,8 +302,8 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ if (!animId) return; const screenDx = e.clientX - d.startX; const screenDy = e.clientY - d.startY; - const x = Math.round(d.initX + screenDx / d.scale); - const y = Math.round(d.initY + screenDy / d.scale); + const x = Math.round(d.initX + screenDx / d.scale / d.pScale); + const y = Math.round(d.initY + screenDy / d.scale / d.pScale); // Click-vs-drag is decided in SCREEN space, not composition px: the old guard // compared rounded comp-px, which at high zoom (scale ≫ 1) swallowed real // multi-px screen drags whose sub-comp-px delta rounds to 0 → the node would diff --git a/packages/studio/src/components/editor/PropertyPanel.tsx b/packages/studio/src/components/editor/PropertyPanel.tsx index a082ac67f1..9aeba6a547 100644 --- a/packages/studio/src/components/editor/PropertyPanel.tsx +++ b/packages/studio/src/components/editor/PropertyPanel.tsx @@ -148,6 +148,19 @@ export const PropertyPanel = memo(function PropertyPanel({ // eslint-disable-next-line react-hooks/exhaustive-deps [gsapRuntimeValues, gsapAnimations, element, currentTime], ); + // The 3D Transform panel should be reachable on ANY element, not only ones GSAP is + // already animating — otherwise you can't add depth/rotation to a fresh static + // element (the panel never appears, the classic chicken-and-egg). Default to + // identity when there are no runtime values yet; the first edit creates the + // gsap.set via commitStaticSet, after which real runtime values flow in. + const gsap3dValues: Record = gsapRuntimeValues ?? { + rotationX: 0, + rotationY: 0, + rotationZ: 0, + z: 0, + scale: 1, + transformPerspective: 0, + }; if (!element) { return ( @@ -490,33 +503,31 @@ export const PropertyPanel = memo(function PropertyPanel({ )} - {gsapRuntimeValues && ( - { - const iframe = iframeRef.current; - const win = iframe?.contentWindow as - | { gsap?: { set: (t: Element, v: Record) => void } } - | null - | undefined; - const sel = el.id ? `#${el.id}` : el.selector; - const node = sel ? iframe?.contentDocument?.querySelector(sel) : null; - if (win?.gsap && node) win.gsap.set(node, props); - }} - /> - )} + { + const iframe = iframeRef.current; + const win = iframe?.contentWindow as + | { gsap?: { set: (t: Element, v: Record) => void } } + | null + | undefined; + const sel = el.id ? `#${el.id}` : el.selector; + const node = sel ? iframe?.contentDocument?.querySelector(sel) : null; + if (win?.gsap && node) win.gsap.set(node, props); + }} + />
Stacking diff --git a/packages/studio/src/components/editor/Transform3DCube.tsx b/packages/studio/src/components/editor/Transform3DCube.tsx index adf7de058d..d0bb4ffd9b 100644 --- a/packages/studio/src/components/editor/Transform3DCube.tsx +++ b/packages/studio/src/components/editor/Transform3DCube.tsx @@ -29,6 +29,7 @@ const pxToProjPersp = (px: number) => (px > 0 ? Math.max(2.2, Math.min(14, px / export function Transform3DCube({ pose, perspective = 0, + defaultPerspective = 0, z = 0, onPoseDraft, onPoseCommit, @@ -41,6 +42,8 @@ export function Transform3DCube({ pose: CubePose; /** Element's transformPerspective (px); drives the cube's foreshortening. */ perspective?: number; + /** Comp-derived lens used for depth feedback before a perspective is committed. */ + defaultPerspective?: number; /** Element's translateZ (px) — "depth", adjusted by scrolling over the cube. */ z?: number; /** Fires on every drag move with the in-progress pose (parent live-previews). */ @@ -68,8 +71,12 @@ export function Transform3DCube({ // studio's "scroll = z depth" gesture-recording convention. A non-passive // listener is required so preventDefault can stop the panel from scrolling. const svgRef = useRef(null); - const depthRef = useRef({ z, onDepthDraft, onDepthCommit }); - depthRef.current = { z, onDepthDraft, onDepthCommit }; + // Perspective lens (committed, else the comp-derived default the panel will + // apply). Drives the cube's depth-scale feedback AND clamps the scroll so depth + // can't cross the lens. Defined here so the wheel handler can read it via the ref. + const lens = perspective > 0 ? perspective : defaultPerspective; + const depthRef = useRef({ z, onDepthDraft, onDepthCommit, lens }); + depthRef.current = { z, onDepthDraft, onDepthCommit, lens }; useEffect(() => { const el = svgRef.current; if (!el) return; @@ -81,7 +88,14 @@ export function Transform3DCube({ e.preventDefault(); // ponytail: 0.25 px of Z per wheel-delta unit (~25px per notch); tune if // it feels too fast/slow. Scroll up (deltaY < 0) pushes toward the viewer. - pending = Math.round((pending ?? depthRef.current.z) - e.deltaY * 0.25); + let next = Math.round((pending ?? depthRef.current.z) - e.deltaY * 0.25); + // Clamp depth in front of the perspective lens. At z ≥ lens the element sits + // at/behind the virtual camera and the projection lens/(lens−z) blows up or + // inverts — that's the runaway "Z = 3195px past a 1080 lens". Cap just short + // of the lens; allow pushing well back (smaller) but not absurdly far. + const L = depthRef.current.lens; + if (L > 0) next = Math.max(Math.min(next, Math.round(L * 0.85)), Math.round(-L * 4)); + pending = next; draft?.(pending); setDepthDraft(pending); // live-scale the cube while scrolling if (timer) clearTimeout(timer); @@ -100,15 +114,15 @@ export function Transform3DCube({ // Depth feedback: the cube scales like the element would — translateZ(z) under // a perspective lens P appears scaled by P/(P-z). Closer (z>0) reads bigger, - // farther (z<0) smaller. Fall back to the default lens so depth always reads in - // the gizmo even before a perspective is set. - const lens = perspective > 0 ? perspective : 800; - const depthScale = Math.max(0.4, Math.min(2.2, lens / (lens - shownZ))); + // farther (z<0) smaller. Use the committed perspective, else the comp-derived + // lens the panel is about to apply — same value in both, so the cube doesn't + // jump when the commit lands. If neither is known, skip the scale (no lens). + const depthScale = lens > 0 ? Math.max(0.4, Math.min(2.2, lens / (lens - shownZ))) : 1; const projOpts = { cx: CX, cy: CY, r: RADIUS * depthScale, - persp: pxToProjPersp(perspective), + persp: pxToProjPersp(lens), }; // The element lives in CSS's screen-Y-down space; the cube projects Y-up. RotateX // and RotateZ act in planes that contain Y, so they read inverted in the gizmo diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index 1bd3599340..6151a09790 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -209,6 +209,22 @@ export function applyManualOffsetDragMatrix(matrix: ManualOffsetDragMatrix, poin }; } +/** + * The perspective w-divisor (matrix3d m44) of the element's current transform. + * For a plain `translateZ(z)` under `perspective(p)`, m44 = (p - z) / p, so the + * element renders 1/m44× larger and a translate of `d` composition px moves + * `d / m44` px on screen. Returns 1 for 2D transforms (no foreshortening). Used + * to keep the drag offset → screen-movement mapping correct for depth elements, + * which the flat-scale fast path below would otherwise get wrong by 1/m44. + */ +function readTransformWDivisor(element: HTMLElement): number { + const t = element.ownerDocument.defaultView?.getComputedStyle(element).transform; + if (!t || !t.startsWith("matrix3d(")) return 1; + const parts = t.slice("matrix3d(".length, -1).split(","); + const w = Number.parseFloat(parts[15] ?? ""); + return Number.isFinite(w) && w > 0 ? w : 1; +} + export function measureManualOffsetDragScreenToOffsetMatrix( element: HTMLElement, initialOffset: { x: number; y: number }, @@ -221,7 +237,11 @@ export function measureManualOffsetDragScreenToOffsetMatrix( ) { const sx = options.scaleX || 1; const sy = options.scaleY || 1; - return { ok: true, matrix: { a: 1 / sx, b: 0, c: 0, d: 1 / sy } }; + // Fold in the perspective foreshortening: a depth element (z≠0) moves + // 1/m44× faster on screen than its flat scale implies, so the screen→offset + // matrix must scale by m44 or the element outruns the pointer/overlay. + const w = readTransformWDivisor(element); + return { ok: true, matrix: { a: w / sx, b: 0, c: 0, d: w / sy } }; } const probeSize = options.probeSize ?? DEFAULT_OFFSET_PROBE_PX; @@ -360,6 +380,7 @@ export function createManualOffsetDragMember(input: { // drag is acceptable — the final committed position is always exact. const scaleX = input.rect.editScaleX || 1; const scaleY = input.rect.editScaleY || 1; + const w = readTransformWDivisor(input.element); return { ok: true, member: { @@ -370,7 +391,7 @@ export function createManualOffsetDragMember(input: { baseGsap, initialPathOffset, gestureToken, - screenToOffset: { a: 1 / scaleX, b: 0, c: 0, d: 1 / scaleY }, + screenToOffset: { a: w / scaleX, b: 0, c: 0, d: w / scaleY }, originRect: input.rect, }, }; diff --git a/packages/studio/src/components/editor/propertyPanel3dTransform.tsx b/packages/studio/src/components/editor/propertyPanel3dTransform.tsx index 22b1aea83b..ad29d80d29 100644 --- a/packages/studio/src/components/editor/propertyPanel3dTransform.tsx +++ b/packages/studio/src/components/editor/propertyPanel3dTransform.tsx @@ -6,9 +6,19 @@ import { KeyframeNavigation } from "./KeyframeNavigation"; import { formatPxMetricValue, parsePxMetricValue, RESPONSIVE_GRID } from "./propertyPanelHelpers"; import { Transform3DCube, type CubePose } from "./Transform3DCube"; -// Default perspective (px) applied when depth is first set, so translateZ is -// visible. ~800px is a moderate lens — closer = stronger foreshortening. -const DEFAULT_DEPTH_PERSPECTIVE = 800; +// translateZ only foreshortens under a perspective lens. Rather than hardcode one +// (an arbitrary px value reads wrong at different canvas sizes), derive it from the +// element's composition: perspective = composition height puts the virtual camera +// one comp-height back, a natural ~53° vertical FOV that looks the same whether the +// canvas is 720p or 4K. Falls back to the element's own height only if the comp size +// can't be read (detached/unmeasured), never to a fixed magic number. +function naturalDepthPerspective(el: HTMLElement | null | undefined): number { + if (!el) return 0; + const root = el.closest("[data-hf-inner-root],[data-composition-id]") as HTMLElement | null; + const compHeight = root?.offsetHeight || el.ownerDocument?.documentElement?.clientHeight || 0; + if (compHeight > 0) return Math.round(compHeight); + return Math.round((el.offsetHeight || 0) * 4) || 0; +} type KeyframeEntry = Array<{ percentage: number; @@ -42,17 +52,10 @@ interface PropertyPanel3dTransformProps { onLivePreviewProps?: (element: DomEditSelection, props: Record) => void; } -type CommitAnimatedProperty = ( - element: DomEditSelection, - property: string, - value: number, -) => Promise; - /** The draggable cube + its commit/recenter/live-preview wiring. */ function Cube3dControl({ element, gsapRuntimeValues, - onCommitAnimatedProperty, onCommitAnimatedProperties, onLivePreviewProps, onKeyframe, @@ -60,8 +63,7 @@ function Cube3dControl({ }: { element: DomEditSelection; gsapRuntimeValues: Record; - onCommitAnimatedProperty: CommitAnimatedProperty; - onCommitAnimatedProperties?: ( + onCommitAnimatedProperties: ( element: DomEditSelection, props: Record, ) => Promise; @@ -74,6 +76,15 @@ function Cube3dControl({ rotationY: gsapRuntimeValues.rotationY ?? 0, rotationZ: gsapRuntimeValues.rotationZ ?? 0, }; + // Comp-derived lens (see naturalDepthPerspective) applied the first time depth is + // set, so the scene's foreshortening scales with the canvas instead of a magic 800. + const depthPerspective = naturalDepthPerspective(element.element); + // A gentle, fixed "depth pose" tilt (degrees) dropped on a flat element the first + // time it gets depth, so translateZ reads as 3D foreshortening instead of a plain + // resize — small enough to look like a premium card, not a flip. + const DEPTH_POSE_X = 10; + const DEPTH_POSE_Y = -15; + const isFlat = Math.round(pose.rotationX) === 0 && Math.round(pose.rotationY) === 0; // Commit only the rotation axes the drag actually changed (each rounded to a // whole degree). Reuses the keyframe-aware animated-property commit, so a drag // at the playhead writes/updates a keyframe just like the numeric fields. @@ -86,13 +97,8 @@ function Cube3dControl({ const axes = Object.keys(changedProps); if (axes.length === 0) return; // ONE keyframe for the whole pose change — avoids per-axis commits racing into - // adjacent duplicate keyframes. Fall back to per-axis if no batched commit. - if (onCommitAnimatedProperties) { - void onCommitAnimatedProperties(element, changedProps); - } else { - for (const [axis, v] of Object.entries(changedProps)) - onCommitAnimatedProperty(element, axis, v); - } + // adjacent duplicate keyframes. + void onCommitAnimatedProperties(element, changedProps); }; const recenter = () => { // ONE commit for the whole reset — six per-axis commits meant six soft-reloads @@ -105,15 +111,10 @@ function Cube3dControl({ scale: 1, transformPerspective: 0, }; - if (onCommitAnimatedProperties) { - void onCommitAnimatedProperties(element, identity); - } else { - for (const [prop, v] of Object.entries(identity)) - void onCommitAnimatedProperty(element, prop, v); - } + void onCommitAnimatedProperties(element, identity); }; // Immediate element feedback while dragging — set the live transform without a - // source write; the release commits via onCommitAnimatedProperty. + // source write; the release commits via commitPose. const livePreview = (next: CubePose) => onLivePreviewProps?.(element, { rotationX: next.rotationX, @@ -127,29 +128,47 @@ function Cube3dControl({ - onLivePreviewProps?.( - element, - gsapRuntimeValues.transformPerspective - ? { z } - : { z, transformPerspective: DEFAULT_DEPTH_PERSPECTIVE }, - ) - } + onDepthDraft={(z) => { + // Preview WITH a lens so depth is visible while scrolling — the same + // default the commit applies, so the element doesn't snap on release. + const preview: Record = gsapRuntimeValues.transformPerspective + ? { z } + : { z, transformPerspective: depthPerspective }; + // Depth-pose preview: a flat element only scales under Z, so mirror the + // commit and preview the gentle tilt that makes the depth read as 3D. + if (isFlat) { + preview.rotationX = DEPTH_POSE_X; + preview.rotationY = DEPTH_POSE_Y; + } + onLivePreviewProps?.(element, preview); + }} onDepthCommit={(z) => { - // translateZ is invisible without a perspective lens — apply a sensible - // default the first time depth is set so scrolling visibly moves the - // element. The user can still fine-tune via the Perspective field. - if (!gsapRuntimeValues.transformPerspective) { - void onCommitAnimatedProperty( - element, - "transformPerspective", - DEFAULT_DEPTH_PERSPECTIVE, - ); + // Best-UX depth: scroll moves Z, and a 3D transform always has a lens — + // like an After Effects camera. translateZ is invisible without a + // perspective, so the FIRST time depth is added (Perspective still 0) we + // set a sensible comp-derived lens ONCE. Every later scroll touches Z + // only, and Perspective stays an independent, editable field. The cube's + // scroll is clamped in front of the lens, so Z can't run away past it. + const props: Record = { z }; + if (!gsapRuntimeValues.transformPerspective && depthPerspective > 0) { + props.transformPerspective = depthPerspective; + } + // Depth-pose: a flat element (no tilt) only scales under Z — it can't read + // as depth. So the first time depth lands on a flat element, also drop a + // gentle fixed tilt; the foreshortening makes depth read as 3D IN PLACE + // (no screen travel, per-element lens unchanged). Once the element has any + // tilt, depth scrolls touch Z only. Reset tilt to 0 to go flat again. + if (isFlat) { + props.rotationX = DEPTH_POSE_X; + props.rotationY = DEPTH_POSE_Y; } - void onCommitAnimatedProperty(element, "z", z); + // One commit for all props so the writes can't race read-modify-write on + // the same script (which dropped a prop and reverted after a seek). + void onCommitAnimatedProperties(element, props); }} onRecenter={recenter} onKeyframe={onKeyframe} @@ -308,11 +327,10 @@ export function PropertyPanel3dTransform({ {collapsed ? null : ( <> - {onCommitAnimatedProperty && ( + {onCommitAnimatedProperties && ( 0 ? w : 1; +} + export function elementHome(el: HTMLElement): { x: number; y: number } { let left = 0; let top = 0; @@ -12,6 +44,14 @@ export function elementHome(el: HTMLElement): { x: number; y: number } { while (node) { left += node.offsetLeft; top += node.offsetTop; + // Ancestor transforms (e.g. a group wrapper moved via GSAP) shift where the + // element actually renders, so the path must anchor on top of them. The element's + // OWN transform is excluded — that's the animated offset the path itself draws. + if (node !== el) { + const t = transformTranslate(node); + left += t.x; + top += t.y; + } const parent = node.offsetParent as HTMLElement | null; if (!parent || parent.hasAttribute("data-composition-id")) break; node = parent; @@ -62,6 +102,7 @@ export function useMotionPathData( geometryResolved: boolean; visibleInPreview: boolean; home: { x: number; y: number } | null; + pScale: number; } { const [rect, setRect] = useState(null); const [geometry, setGeometry] = useState(null); @@ -69,6 +110,9 @@ export function useMotionPathData( const geometryResolved = resolvedForRef.current === selector; const [visibleInPreview, setVisibleInPreview] = useState(true); const [home, setHome] = useState<{ x: number; y: number } | null>(null); + // Perspective magnification (1/m44) of the selected element — applied to the + // path's offset points so depth (translateZ) elements' paths track on screen. + const [pScale, setPScale] = useState(1); useEffect(() => { if (!selector) { @@ -105,6 +149,8 @@ export function useMotionPathData( setHome((prev) => prev && Math.abs(prev.x - h.x) < 0.5 && Math.abs(prev.y - h.y) < 0.5 ? prev : h, ); + const ps = 1 / transformWDivisor(live); + setPScale((p) => (Math.abs(p - ps) < 0.001 ? p : ps)); } } raf = requestAnimationFrame(tick); @@ -132,5 +178,5 @@ export function useMotionPathData( return () => window.clearInterval(id); }, [selector, iframeRef]); - return { rect, geometry, geometryResolved, visibleInPreview, home }; + return { rect, geometry, geometryResolved, visibleInPreview, home, pScale }; } diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index 6efd2ce3f4..55c2e50804 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -241,8 +241,15 @@ export async function tryGsapDragIntercept( // place (idempotent), else add a new one. This also covers the stale-cache // phantom — committing a set is correct because the element genuinely has no live motion. const hasNonHold = hasNonHoldTweenForElement(iframe, selector); - - if (!hasNonHold) { + // A KEYFRAMED position tween — even one that's currently a flat constant ("hold", + // e.g. 0% and 100% identical) — is still an animation the user is building, so a + // drag must add/update a keyframe, NOT fall back to a static `set`. Without this, + // dragging an element whose position tween is constant writes a `gsap.set` that + // fights the tween (the "drag didn't create a keyframe / didn't persist" bug). The + // static path is only for elements with NO keyframed position tween (truly static, + // or just a leftover position-hold `set`). + const hasKeyframedPosTween = !!posAnim?.keyframes; + if (!hasNonHold && !hasKeyframedPosTween) { const existingSet = posAnim && posAnim.method === "set" && posAnim.targetSelector === selector ? posAnim diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts index eb49b1eb94..28999ee6f3 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts @@ -15,6 +15,8 @@ import { usePlayerStore } from "../player/store/playerStore"; import { readAllAnimatedProperties, readGsapProperty } from "./gsapRuntimeBridge"; import type { SetPatchProps } from "./gsapRuntimePatch"; import { selectorFromSelection, computeElementPercentage } from "./gsapShared"; +import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; +import { roundTo3 } from "../utils/rounding"; interface CommitAnimatedPropertyDeps { selectedGsapAnimations: GsapAnimation[]; @@ -45,14 +47,22 @@ function pickBestAnimation( selector: string | null, property?: string, ): GsapAnimation | undefined { - if (animations.length <= 1) return animations[0]; - const currentTime = usePlayerStore.getState().currentTime; const targetGroup = property ? classifyPropertyGroup(property) : undefined; - - // fallow-ignore-next-line complexity - const scored = animations.map((a) => { + // Group-aware: never hand back a tween from a DIFFERENT property group. The old + // `animations.length <= 1` early return merged a rotation/3D edit into the element's + // only tween even when that was a `position` tween — contaminating it and leaving the + // new property with no clean keyframe baseline. When a target group is known, only + // same-group tweens are candidates; if none exist we return undefined and the caller + // creates a fresh same-group tween. + const candidates = + targetGroup !== undefined + ? animations.filter((a) => a.propertyGroup === targetGroup) + : animations; + if (candidates.length === 0) return undefined; + if (candidates.length === 1) return candidates[0]; + const currentTime = usePlayerStore.getState().currentTime; + const scored = candidates.map((a) => { let score = 0; - if (targetGroup && a.propertyGroup === targetGroup) score += 20; if (a.keyframes) score += 10; if (selector && a.targetSelector === selector) score += 5; else if (a.targetSelector.includes(",")) score -= 3; @@ -196,14 +206,15 @@ async function commitKeyframeProps( iframe: HTMLIFrameElement | null, commit: Commit, ): Promise { - if (!anim.keyframes) { + const wasKeyframed = !!anim.keyframes; + if (!wasKeyframed) { await commit( selection, { type: "convert-to-keyframes", animationId: anim.id }, { label: "Convert to keyframes", skipReload: true }, ); } - const pct = computeElementPercentage(usePlayerStore.getState().currentTime, selection, anim); + const ct = usePlayerStore.getState().currentTime; const runtimeProps = selector ? readAllAnimatedProperties(iframe, selector, anim) : {}; const properties: Record = { ...runtimeProps, ...props }; @@ -216,6 +227,52 @@ async function commitKeyframeProps( backfillDefaults[property] = value; } + // Playhead OUTSIDE the keyframe tween's time range → EXTEND the tween to reach it + // and add a keyframe there, exactly like manual drag's extendTweenAndAddKeyframe. + // The add-keyframe below only writes WITHIN the existing range, so without this a + // depth edit past the tween end just overwrites the last keyframe (the bug: no new + // diamond appears at a playhead beyond the tween). Only for an already-keyframed + // tween — a freshly-converted set has no prior range worth remapping. + const kfs = anim.keyframes?.keyframes; + const ts = resolveTweenStart(anim); + const td = resolveTweenDuration(anim); + const hasSelectedKeyframe = usePlayerStore.getState().activeKeyframePct != null; + const playheadOutside = ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); + const willExtend = wasKeyframed && !!kfs && playheadOutside && !hasSelectedKeyframe; + if (willExtend && kfs && ts !== null) { + const newStart = Math.min(ct, ts); + const newEnd = Math.max(ct, ts + td); + const newDuration = Math.max(0.01, newEnd - newStart); + const remapped = kfs.map((kf) => { + const absTime = ts + (kf.percentage / 100) * td; + const newPct = Math.round(((absTime - newStart) / newDuration) * 1000) / 10; + const p: Record = { ...kf.properties }; + for (const k of Object.keys(properties)) { + if (!(k in p) && backfillDefaults[k] != null) p[k] = backfillDefaults[k]; + } + return { percentage: newPct, properties: p }; + }); + remapped.push({ + percentage: Math.round(((ct - newStart) / newDuration) * 1000) / 10, + properties, + }); + remapped.sort((a, b) => a.percentage - b.percentage); + await commit( + selection, + { + type: "replace-with-keyframes", + animationId: anim.id, + targetSelector: anim.targetSelector, + position: roundTo3(newStart), + duration: roundTo3(newDuration), + keyframes: remapped, + }, + { label: `Edit ${primaryProp} (extended keyframe)`, softReload: true }, + ); + return; + } + + const pct = computeElementPercentage(ct, selection, anim); const existingKf = anim.keyframes?.keyframes.some((kf) => Math.abs(kf.percentage - pct) < 0.05); // Rebuild the live keyframe tween in place so the edit shows instantly (no flash); // rebuildKeyframeTween declines → soft reload if the tween can't be safely rebuilt. @@ -275,8 +332,30 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { // so the rejection doesn't escape as an uncaught promise, and bump the cache // so selectedGsapAnimations re-syncs and the user's next edit self-heals. try { - // Existing static hold — merge the props into the `set`, then auto-keyframe - // ONLY if the element is already animated (maybeAutoKeyframeSet no-ops if not). + // Animated element → keyframe at the playhead, EXACTLY like manual drag / + // resize / rotate: if the picked anim is still a static `set`, + // commitKeyframeProps converts it to keyframes first, then writes the new + // value as a keyframe at the current time — so the 3D animates instead of + // holding a flat constant. This MUST come before the `set`-update path below, + // or a 3D `set` would short-circuit to an in-place update and the playhead + // keyframe would never land (the bug: scrolling depth on a keyframed element + // just changed the constant instead of dropping a keyframe). + if (elementHasKeyframes && anim) { + await commitKeyframeProps( + selection, + anim, + props, + propEntries, + primaryProp, + selector, + iframe, + gsapCommitMutation, + ); + return; + } + + // Existing static hold on a NON-animated element — merge the props into the + // `set` in place (maybeAutoKeyframeSet no-ops when nothing else is keyframed). if (anim?.method === "set") { await commitSetProps( selection, @@ -289,8 +368,8 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { return; } - // Static element — persist as a `tl.set`, never keyframes (incl. the - // no-animation case, which now creates a set instead of a keyframed tween). + // Static element (no keyframes anywhere) — persist as a `tl.set`, never + // keyframes (incl. the no-animation case, which creates a fresh set). if (!elementHasKeyframes) { await commitStaticSet( selection, @@ -302,22 +381,43 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { return; } - // Animated element — write ALL props into ONE keyframe so a multi-axis cube - // edit doesn't race into adjacent duplicates. - if (!anim) { - bumpGsapCache(); + // Animated element but NO same-group tween exists (e.g. the FIRST rotation/3D + // keyframe on an element that only has a position tween). Create a fresh + // same-group keyframed tween WITH a 0% baseline at the playhead, instead of + // contaminating a foreign-group tween. Mirror an existing keyframed tween's + // time range so the new group animates over the same span. The 0% baseline is + // an `_auto` endpoint so it tracks the nearest keyframe as you add more. + if (selector) { + const template = selectedGsapAnimations.find((a) => !!a.keyframes); + const tStart = template ? (resolveTweenStart(template) ?? 0) : 0; + const tDur = template ? resolveTweenDuration(template) || 1 : 1; + const ct = usePlayerStore.getState().currentTime; + const pct = + tDur > 0 + ? Math.max(0, Math.min(100, Math.round(((ct - tStart) / tDur) * 1000) / 10)) + : 0; + const newProps = Object.fromEntries(propEntries); + const keyframes = + pct <= 0.05 + ? [{ percentage: 0, properties: newProps }] + : [ + { percentage: 0, properties: { ...newProps, _auto: 1 } }, + { percentage: pct, properties: newProps }, + ]; + await gsapCommitMutation( + selection, + { + type: "add-with-keyframes", + targetSelector: selector, + position: roundTo3(tStart), + duration: roundTo3(tDur), + keyframes, + }, + { label: `Add ${primaryProp} keyframe`, softReload: true }, + ); return; } - await commitKeyframeProps( - selection, - anim, - props, - propEntries, - primaryProp, - selector, - iframe, - gsapCommitMutation, - ); + bumpGsapCache(); } catch { bumpGsapCache(); } diff --git a/packages/studio/src/hooks/useEnableKeyframes.test.ts b/packages/studio/src/hooks/useEnableKeyframes.test.ts index 3594a57983..47826780ac 100644 --- a/packages/studio/src/hooks/useEnableKeyframes.test.ts +++ b/packages/studio/src/hooks/useEnableKeyframes.test.ts @@ -167,4 +167,39 @@ describe("promoteSetToKeyframes — auto endpoint", () => { expect(kfs[1].percentage).toBe(100); expect(kfs[1].auto).toBeUndefined(); }); + + it("playhead AT the set (t <= setStart) drops a single 0% keyframe, not a no-op", async () => { + // Regression: enabling keyframes on a `gsap.set` element at t=0 (set start 0) + // returned early (`t <= setStart`) → nothing created. Must give a 0% keyframe. + let committed: Record | undefined; + const session = { + commitMutation: async (mutation: Record) => { + committed = mutation; + }, + } as unknown as EnableKeyframesSession; + const sel = { + id: "box", + selector: "#box", + sourceFile: "index.html", + element: { isConnected: true } as unknown as HTMLElement, + } as unknown as DomEditSelection; + const iframe = { + contentWindow: { gsap: { getProperty: () => -1091 } }, + } as unknown as HTMLIFrameElement; + const setAnim = anim({ + id: "#box-set-0-position", + targetSelector: "#box", + method: "set", + global: true, + resolvedStart: 0, + properties: { x: -1091, y: 280 }, + }); + + await promoteSetToKeyframes(session, sel, setAnim, 0, iframe); + + const kfs = committed?.keyframes as Array<{ percentage: number }>; + expect(committed?.type).toBe("replace-with-keyframes"); + expect(kfs).toHaveLength(1); + expect(kfs[0].percentage).toBe(0); + }); }); diff --git a/packages/studio/src/hooks/useEnableKeyframes.ts b/packages/studio/src/hooks/useEnableKeyframes.ts index 634f545e8a..3c9558ab1a 100644 --- a/packages/studio/src/hooks/useEnableKeyframes.ts +++ b/packages/studio/src/hooks/useEnableKeyframes.ts @@ -253,7 +253,35 @@ export async function promoteSetToKeyframes( ): Promise { const selector = selectorFromSelection(sel); const setStart = resolveTweenStart(setAnim) ?? 0; - if (!selector || !session.commitMutation || t <= setStart) return; + if (!selector || !session.commitMutation) return; + // Playhead at or before the set → there's no forward range to promote into. + // Instead of doing nothing (which read as "can't add a keyframe at 0"), replace + // the set with a single keyframe at the playhead holding its value, matching the + // no-animation branch: one diamond the user can build motion from. + if (t <= setStart) { + const position = readElementPosition(iframe, sel, setAnim); + if (Object.keys(position).length === 0) { + for (const key of Object.keys(setAnim.properties ?? {})) { + const held = setAnim.properties?.[key]; + if (typeof held === "number") position[key] = held; + } + } + if (Object.keys(position).length === 0) return; + const range = resolveNewTweenRange(sel.dataAttributes?.start, sel.dataAttributes?.duration, t); + await session.commitMutation( + { + type: "replace-with-keyframes", + animationId: setAnim.id, + targetSelector: selector, + position: roundTo3(range.start), + duration: roundTo3(range.duration), + keyframes: [{ percentage: 0, properties: position }], + ease: setAnim.ease, + }, + { label: "Enable keyframes", softReload: true }, + ); + return; + } const endPosition = readElementPosition(iframe, sel, setAnim); if (Object.keys(endPosition).length === 0) return; const startPosition: Record = {};