Skip to content

feat(studio): on-canvas motion-path overlay#1610

Open
miguel-heygen wants to merge 2 commits into
feat/studio-motionpath-helpersfrom
feat/studio-motionpath-overlay
Open

feat(studio): on-canvas motion-path overlay#1610
miguel-heygen wants to merge 2 commits into
feat/studio-motionpath-helpersfrom
feat/studio-motionpath-overlay

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR adds an on-canvas motion-path overlay to Studio. When you select an element that has positional motion (GSAP x/y keyframes or a motionPath), the overlay draws that motion as a dashed path directly over the preview canvas, with a draggable node at each keyframe / waypoint. You can drag nodes to reshape the motion, click on the path to insert a new stop, right-click a keyframe node to delete it, and — for an element with no motion yet — double-click the canvas to author a brand-new motionPath. Every interaction commits to source and is undoable.

This is the visible, interactive layer on top of the geometry/commit/selection helpers landed in #1609.

What's changed

MotionPathOverlay.tsx (new, ~580 LOC) — the overlay itself

An absolute, z-40 SVG over the preview, drawn in declared composition coordinates (viewBox = 0 0 width height) so the path stays pinned to the element as GSAP applies transforms.

  • Live data tracking (useMotionPathData) — tracks the iframe rect every requestAnimationFrame, positioned relative to the [data-preview-pan-surface] wrapper (queried via the light DOM, since the composition iframe lives in the player's shadow DOM) so the SVG is clipped to the canvas under zoom/pan. Polls the GSAP runtime every 250ms (readRuntimeKeyframesbuildMotionPathGeometry) to rebuild geometry, with a points-equality guard. Resolves the target element live from the current iframe document each tick (soft reloads detach captured nodes). Tracks visibleInPreview via isElementVisibleInPreview.
  • Element anchoring (elementHome) — computes the element's layout-home center by walking offsetLeft/offsetTop to (not including) the composition root; GSAP x/y and motionPath coords are offsets from this. Folds in the manual CSS path offset (--hf-studio-offset-x/y) so a gesture on a manually-dragged element isn't drawn shifted.
  • Cross-realm check (isPreviewHtmlElement) — preview nodes are instances of the iframe window's HTMLElement, so this checks against the iframe realm's constructor.
  • Node/handle rendering (MotionPathNode.tsx, new) — each node is a diamond matching the timeline keyframe shape; selection enlarges it. Wider transparent grab target + hover × delete badge; vectorEffect="non-scaling-stroke"; themeable ACCENT applied inline (SVG attrs reject var()).
  • Drag interactions — pointer-down captures + starts a draft; move updates in composition space (screenDelta / scale); up either commits the move (commitNode, then parks the playhead on the edited keyframe so it previews at that keyframe) or, on a no-move, treats it as a click (selects the keyframe + parks the playhead so the next drag modifies it).
  • Add-on-path — a wide transparent hit polyline drives a dashed ghost diamond projecting the cursor onto the nearest path point; clicking inserts a motionPath waypoint (commitAddWaypoint) for arcs or a keyframe at the interpolated tween-% (commitAddKeyframe) for linear, landing on the line so nothing jumps.
  • Remove — the × badge quick-removes a waypoint (offered only for non-cubic arcs with >2 nodes); keyframe nodes remove via right-click → the timeline's KeyframeDiamondContextMenu.
  • Create mode — when a selected element has no positional motion (not playing, plugin present), a dashed ring + "double-click to set a destination" hint draws at the home; double-click authors a new path (commitCreatePath).
  • Read-only guards — drag/add disabled while playing or when the tween isn't statically editable (editableAnimationId); renders nothing without a rect, composition size, live anchor, or positional motion.

StudioPreviewArea.tsx

Imports MotionPathOverlay + useCompositionDimensions and renders it alongside the DOM-edit overlay + snap toolbar, gated behind STUDIO_KEYFRAMES_ENABLED, fed the iframe ref, current selection, composition dimensions, and isPlaying.

useDomEditOverlayRects.ts / DomEditOverlay.tsx

useDomEditOverlayRects now uses isElementVisibleInPreview so the selection box and motion-path overlay share one "is the element actually painted?" rule. DomEditOverlay.tsx is a one-line whitespace touch.

Why

Editing motion by typing coordinates or dragging timeline diamonds is indirect — you can't see the trajectory. Drawing the path on the canvas with draggable nodes makes spatial motion directly manipulable, with insert/delete/create available without leaving the preview. Anchoring in composition coordinates and resolving the element live keeps the path glued to its target through seeks, soft-reload commits, zoom, and pan — the cases where a naive overlay drifts.

Testing

Overlay UI, exercised manually in Studio: select an element with x/y keyframes or a motionPath and confirm the path draws on it; drag nodes (motion updates + commits, undoable); click the path to insert a stop; right-click a keyframe node to delete; double-click with a motion-less element to author a new path; verify the path stays anchored through seek, play/pause, zoom, pan, and hides when covered by a later scene. The underlying helpers it builds on are covered in #1609; this PR adds the rendering/interaction layer (limited automated coverage). bun run build and bun run test pass.

Stack

Part of the GSAP keyframe/motion-path stack: #1553 → #1554 → #1555 → #1607 → #1608 → #1609 → #1610 → #1611 → #1567 → #1612 → #1613 → #1605. This PR (#1610) sits on #1609. Builds independently; the combined diff across the stack is byte-identical to the originally-reviewed work.

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review @ 0e07ce23 — on-canvas motion-path overlay sitting on #1609's helpers. Reviewed the overlay rendering, gesture handling, coord-system transforms, lifecycle, and integration with StudioPreviewArea.

Looks good

  • Coord-system handling is careful. clientToComp uses the LIVE iframe.getBoundingClientRect() rather than the cached rect (line 361, with the explanatory comment). The reasoning is right — rect is stored pan-surface-relative for absolute positioning, so subtracting it from a viewport clientX/Y would offset the projection by the surface gutter. Classic class of bug, well-handled.
  • Shadow-DOM-aware pan-surface lookup (line 156): el.ownerDocument?.querySelector('[data-preview-pan-surface]') instead of el.closest(...). The comment explains why (composition iframe is in the player's shadow DOM, closest() stops at the shadow root). Future-trap defused with a comment.
  • Cross-realm HTMLElement check (isPreviewHtmlElement, line 85): the iframe-realm constructor check is necessary and correct — node instanceof HTMLElement would silently false on preview-document nodes.
  • vectorEffect=\"non-scaling-stroke\" on the path + ghost + node strokes keeps stroke widths constant under zoom. Matches sibling overlay conventions.
  • z-index layering: z-40 over DomEditOverlay's z-10 — drag handles correctly on top of the selection box.
  • Visibility parity: uses the SAME isElementVisibleInPreview as useDomEditOverlayRects (the selection box hook). Path + selection share one truth about what's painted. Bigger win than it looks — drift here is the kind of bug that ships unnoticed for months.
  • elementHome folds in --hf-studio-offset-x/y (lines 74-77) so a gesture on a manually-dragged element draws its path at the actual rendered position, not the gsap-coord home. The bug it prevents is exactly the kind of thing canvas-overlay reviewers miss.
  • Read-only guards (interactive = Boolean(animId) && !isPlaying) gate every drag + add + create cleanly.

Concerns

  • Global dblclick listener registered on window (line 281). Effect deps: [createMode, selection, compositionSize, iframeRef, commitMutation]. commitMutation comes from useDomEditContext() — if it isn't memoized (useCallback), the listener re-attaches on every render of any consumer. Worth confirming the context value is stable. If not, both addEventListener/removeEventListener thrash per frame while in create mode. Also: a global window dblclick while in create mode means double-clicking ANYWHERE in the page (e.g. on a sidebar input to select a word) enters this handler; you bounds-check against the iframe rect, but the early-out only protects from the commit, not from the perf hit of listener evaluation on every dblclick. Consider attaching to the pan-surface element instead of window, or at least confirm the context callback is stable.
  • Drag-vs-poll race during a soft-reload. The 250ms setInterval (line 209) keeps rebuilding geometry. If a sibling edit commits during a drag and geometry.nodes reorders, draft.index now points at a stale slot — the rendered draft + the eventual commitNode(d.ref, ...) write to the right ref (good, because ref is captured at drag-start) but the visual draft node on screen may have moved to the wrong slot during the drag. Solo-canvas → unlikely; collab-canvas (if that ever happens) → real. Worth either pausing the geometry refresh while dragRef.current is set, or guarding the geometry.nodes.map((n, i) => i === draft.index ? ...) with a ref-identity match instead of index. Not a blocker today; tag it for future collab work.
  • buildMotionPathGeometry equality guard compares only points (line 206: prev?.points === next?.points). If kind flips (lineararc) but points happens to identically match (degenerate edge case), the stored geometry stays at the wrong kind → wrong MotionNodeRef discriminator → wrong commit mutation type. Almost impossible in practice (kind flip implies waypoint vs keyframe ref shape change which usually changes coords too), but a kind-comparison on the guard costs nothing.
  • createMode flickers during selector transitions. Line 256: !geometry && Boolean(selection?.element) && !isPlaying. Between a new selection landing and the next 250ms poll computing geometry, geometry is null → the dashed "double-click to set a destination" ring flashes on top of an element that actually HAS a path. Worth either an initial recompute() outside the interval (already there at line 208 — good, runs once per effect mount) plus a transient "loading" state, or just suppressing create-mode rendering for the first ~50ms after selector change.

Nits

  • Sub-pixel drag threshold (line 409): if (x === Math.round(d.initX) && y === Math.round(d.initY)) treats a 0.49-comp-px gesture as a click. At high zoom (scale > 1), that's a sub-screen-pixel threshold — fine. At low zoom (scale < 1, e.g. 50% canvas zoom), 0.49 comp-px ≈ 1 screen-px, so a real 1-screen-pixel slip suppresses the click-handler intent (select + park playhead). Real but very minor friction. (nit)
  • fallow-ignore-next-line complexity annotations on the component (line 226), the create-mode effect (line 257), and onDbl (line 262), and onUp (line 398). The component is doing real coordination work — these are justified — but the onDbl effect could split out the bounds-check + commit into a helper to drop the bypass. (nit)
  • hasMotionPathPlugin synchronous check (line 103) returns false during the brief window before the plugin async-imports in the iframe. If a user selects + double-clicks an element FAST after iframe mount, create-mode silently no-ops on the first try (user re-tries → works). Documented behavior would help, but the path is recoverable. (nit)
  • MotionPathNode's onPointerMove/onPointerUp are wired on every node (lines 564-565). Pointer capture is on the node element that received the down, so only the right node receives subsequent moves — but every other node still has those handlers attached. Cosmetic; React makes this cheap. (nit)

Questions

  • activeKeyframePct selection model: when the user clicks (no-drag) a keyframe node, you call setActiveKeyframePct(d.ref.pct) and parkPlayheadOnKeyframe. If two animations on the same element both have a keyframe at the same pct, do they both light up? MotionPathNode's selected = p.ref.type === \"keyframe\" && p.ref.pct === activeKeyframePct (line 560) is animation-blind. Confirm this is intended (the overlay only renders ONE animation's path at a time per editableAnimationId, so probably fine) or that another guard prevents cross-anim highlight bleed.
  • isPlaying re-entry: useEffect cleanup for the dblclick listener handles exit; what about a user hitting Play mid-create-mode? The effect deps include createMode which depends on !isPlaying, so toggling Play unregisters cleanly. Good — but worth a single test that mounts → plays → unmounts doesn't leak the listener. (no test in this PR — acknowledged in body.)

What I didn't verify

  • I didn't run the overlay manually in Studio (no local studio env). Trusted the PR body's manual-test description.
  • I didn't trace commitMutation's identity through useDomEditContext provider to confirm stability — see concern 1 above.
  • I didn't check that the motionPathOverlay's points-equality guard correctly handles geometry going null → non-null → null (selector changes); read says yes (prev?.points === next?.points matches undefined === undefined only if both null, otherwise re-renders). Confirmed by re-reading.

Solid PR. The geometry math + commit routing inherits the rigor from #1609; the overlay itself does the careful coordinate-system + shadow-DOM + cross-realm work this class of feature usually gets wrong. Main asks: (1) confirm commitMutation identity stability for the global dblclick listener, (2) consider a kind field on the geometry equality guard, (3) flicker handling on selector transition.

Not a stamp-blocker from my side; concerns are layered cleanly. Defer to Vai (the HF-runtime peer) for the GSAP plugin / soft-reload interaction lens.

— Rames D Jusso

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 0e07ce23. Concur with @james-russo-rames-d-jusso — the careful coordinate-system + shadow-DOM + cross-realm work is exactly what this class of feature usually gets wrong, and the overlay does it right.

Verified at HEAD:

  • Cross-realm isPreviewHtmlElement check uses iframe contentWindow.HTMLElement, NOT globalThis.HTMLElement. Correct cross-realm pattern. Used at two call sites (rect-tick + dblclick handler).
  • clientToComp uses LIVE iframe.getBoundingClientRect() rather than the cached rect — the comment explains why (rect stored pan-surface-relative; mixing coordinate spaces would offset by the gutter). Right call.
  • Shadow-DOM-aware pan-surface lookup (el.ownerDocument?.querySelector(...) instead of el.closest) — future-trap defused with a comment.
  • vectorEffect="non-scaling-stroke" keeps stroke widths constant under zoom.
  • RAF (let raf = 0cancelAnimationFrame(raf)) and 250ms setInterval (clearInterval(id)) both have cleanup. Points-equality guard prev?.points === next?.points suppresses redundant React state churn. No leak.
  • STUDIO_KEYFRAMES_ENABLED gate at StudioPreviewArea.tsx is an EXISTING flag from main (declared at manualEditingAvailability.ts, default false, 7 prior consumers + changelog entry). Not introduced by this stack — decorative-gate-pattern check N/A.

Drag-vs-click ambiguity handled in onUp via Math.round(initX + screenDx/scale) === Math.round(initX) integer comparison. Uses pointer capture on grab target. Concur with @james-russo-rames-d-jusso's NIT on the sub-pixel threshold under low zoom (real but very minor friction).

Concur with @james-russo-rames-d-jusso:

  • Global dblclick listener on window rather than the iframe / pan-surface — Rames's instinct to attach to the pan-surface is right. The bounds check protects the commit, not the perf-cost of listener evaluation on unrelated dblclicks. Confirm commitMutation identity stability through DomEditContext before optimizing.
  • Drag-vs-poll race: 250ms setInterval keeps rebuilding geometry. If a sibling edit lands during a drag and geometry.nodes reorders, draft.index points at a stale slot. Solo-canvas safe (capturing ref at drag-start), but the fix Rames proposes (ref-identity match instead of index) is the right shape.
  • prev?.points === next?.points equality guard: if kind flips with identical points (degenerate edge), wrong MotionNodeRef discriminator → wrong commit mutation type. Almost impossible practically, but a kind comparison costs nothing.
  • createMode flicker on selector transitions — the initial recompute() outside the interval helps but there's still a sub-frame window of "double-click to set destination" dashed ring over an element that has a path.

Net-new — soft NIT:

useDomEditOverlayRects widens its visibility predicate from isElementVisibleForOverlay to the stronger isElementVisibleInPreview. PR body discloses this as intentional ("share one 'is the element actually painted?' rule") and it IS the right cross-overlay parity move. But it changes existing DOM-edit selection-overlay behavior: the selection box now disappears when the selected element is occluded by a later scene. Disclosed in body, plausibly desired, but worth a changelog entry — any user currently relying on the selection box being visible through scene fades sees a regression. Not a band-aid.

Net-new — soft NIT on pointer-cancel symmetry:

onMove / onUp are wired but no onPointerCancel cleanup. If a system-level pointer-cancel fires mid-drag (browser tab switch, modal hijack), dragRef.current stays set until the next onUp. Pointer capture release on cancel should cause subsequent events to skip these handlers anyway, so more theoretical than practical — but a one-line onPointerCancel={onUp} for robustness.

Net-new — inherited cross-PR drift from #1609:

editableAnimationId is playhead-blind while readRuntimeKeyframes (which feeds the geometry) is playhead-aware. For multi-position-tween elements (rare), the writer may commit to the wrong tween. Surfaces at commit time here. See my #1609 review.

Verdict: NIT. All asks are non-blocking; the selection-overlay widening is the strongest one to disclose more explicitly than the body does today.

Review by Via

@miguel-heygen miguel-heygen force-pushed the feat/studio-motionpath-helpers branch from ec4218d to 395c6f7 Compare June 20, 2026 21:20
@miguel-heygen miguel-heygen force-pushed the feat/studio-motionpath-overlay branch from 0e07ce2 to 0392a98 Compare June 20, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants