From bbd6ed5e07fb1c1584d1c3493f291f775c7184c7 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 1 Jul 2026 16:23:06 +0800 Subject: [PATCH] feat(keyframes): time ruler + cross-property snapping + yellow snap line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port upstream KeyframesLane's editor affordances (Inspector/Keyframes/ KeyframesLane.swift). Dragging a keyframe diamond previously snapped only to the playhead; now it snaps to the nearest of {playhead, clip start/end, every OTHER property's keyframes} within ±5 frames (snapTargets, :201-216), and a panel-wide yellow DASHED guide line shows while snapped (snapOverlay, :301-313, matching the main timeline's SnapIndicator). The static tinted strip becomes a real tick ruler reusing the shared chooseTicks/formatTimecode logic (as upstream's RulerView reuses TimelineRuler). - keyframeSnap.ts: pure snapFrame(candidate, targets, threshold) — nearest-wins, deterministic ties, null when outside threshold. Unit-tested. - KeyframesLaneRow: collects cross-property + clip-bound targets (excludes the dragged property's own track, 1:1 with `where p != property`); reports the live snapped frame up via onSnapChange; clears it on mouseup + mid-drag unmount. - KeyframesPanel: lifts the snap-frame state (the line must span all rows); renders the yellow dashed overlay; swaps the tinted strip for . - KeyframesRuler.tsx: ResizeObserver-measured tick ruler, clip-relative frames. - tokens.css: --status-warning (SwiftUI .yellow / systemYellow). Preserved: playhead snap (now one target among many), moveKeyframe commit, live diamond preview, context menu, stamp/clear, drag-cleanup. Frontend only. Verified: pnpm build (tsc) clean; pnpm test 218 passed (incl. new snapFrame tests). --- .../components/inspector/KeyframesLaneRow.tsx | 65 ++++++++++- .../components/inspector/KeyframesPanel.tsx | 65 +++++++---- .../components/inspector/KeyframesRuler.tsx | 106 ++++++++++++++++++ web/src/lib/keyframeSnap.test.ts | 46 ++++++++ web/src/lib/keyframeSnap.ts | 42 +++++++ web/src/styles/tokens.css | 3 + 6 files changed, 299 insertions(+), 28 deletions(-) create mode 100644 web/src/components/inspector/KeyframesRuler.tsx create mode 100644 web/src/lib/keyframeSnap.test.ts create mode 100644 web/src/lib/keyframeSnap.ts diff --git a/web/src/components/inspector/KeyframesLaneRow.tsx b/web/src/components/inspector/KeyframesLaneRow.tsx index 758dfdf..ce6f1e6 100644 --- a/web/src/components/inspector/KeyframesLaneRow.tsx +++ b/web/src/components/inspector/KeyframesLaneRow.tsx @@ -6,8 +6,11 @@ * Interaction model: * - Click empty track area → seek the playhead to that frame. * - Click a diamond → starts a drag (window mousemove/mouseup); the diamond - * follows the cursor in real time, snaps to the playhead within ±5 frames, - * and commits via `edit.moveKeyframe` on mouseup. + * follows the cursor in real time, snaps to the nearest of {playhead, clip + * start/end, every OTHER property's keyframes} within ±5 frames (1:1 port + * of upstream `KeyframesLaneRow.applySnap` / `snapTargets`, + * Inspector/Keyframes/KeyframesLane.swift:177-216), and commits via + * `edit.moveKeyframe` on mouseup. * - Right-click a diamond → context menu (delete / set interpolation). * - Stamp button → `edit.stampKeyframe` at the current playhead. * - Clear button → `edit.setKeyframes` with an empty keyframe array. @@ -22,6 +25,7 @@ import { useState, useRef, useCallback, useEffect } from "react"; import { useEditorUiStore } from "../../store/uiStore"; import * as edit from "../../store/editActions"; +import { snapFrame } from "../../lib/keyframeSnap"; import type { AnimPair, Clip, @@ -43,14 +47,49 @@ type AnyKeyframeTrack = | KeyframeTrack | KeyframeTrack; +/** All animatable properties, used to collect cross-property snap targets + * regardless of which subset (video vs audio) is currently rendered as rows. */ +const ALL_PROPERTIES: KeyframeProperty[] = [ + "opacity", + "volume", + "rotation", + "position", + "scale", + "crop", +]; + +/** Absolute-frame snap targets from every OTHER property's keyframes on this + * clip, plus the clip's own start/end. Excludes `property` (the row being + * dragged) so a keyframe never snaps to a sibling on its own track — matches + * upstream's `for p in AnimatableProperty.allCases where p != property` + * (KeyframesLane.swift:210-214). Playhead is added separately by the caller + * since it isn't a track-derived target. */ +function crossPropertyAndBoundTargets(clip: Clip, property: KeyframeProperty): number[] { + const targets: number[] = [clip.startFrame, clip.startFrame + clip.durationFrames]; + for (const p of ALL_PROPERTIES) { + if (p === property) continue; + const otherTrack = getTrack(clip, p); + if (!otherTrack) continue; + for (const kf of otherTrack.keyframes) { + targets.push(kf.frame + clip.startFrame); + } + } + return targets; +} + export function KeyframesLaneRow({ clip, property, t, + onSnapChange, }: { clip: Clip; property: KeyframeProperty; t: TFunction; + /** Reports the absolute frame currently snapped to during a drag (or null + * when not snapped / not dragging) so KeyframesPanel can render a + * panel-wide yellow snap-guide line. */ + onSnapChange?: (absFrame: number | null) => void; }) { const activeFrame = useEditorUiStore((s) => s.activeFrame); const setActiveFrame = useEditorUiStore((s) => s.setActiveFrame); @@ -62,12 +101,16 @@ export function KeyframesLaneRow({ * Cleared on unmount via the useEffect below to prevent leaks. */ const dragCleanupRef = useRef<(() => void) | null>(null); - // Unmount safety: remove any active drag listeners. + // Unmount safety: remove any active drag listeners and clear any snap line + // the panel may be showing on our behalf (otherwise a mid-drag unmount — + // e.g. deselecting the clip — would leave a stale yellow line onscreen). useEffect(() => { return () => { dragCleanupRef.current?.(); dragCleanupRef.current = null; + onSnapChange?.(null); }; + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); const startFrame = clip.startFrame; @@ -119,19 +162,31 @@ export function KeyframesLaneRow({ setDragging({ fromFrame: absFrame, currentFrame: absFrame }); // Clamp to [startFrame, startFrame + duration - 1] (half-open clip range). const lastFrame = startFrame + duration - 1; + // Cross-property + clip-bound targets are stable for the whole drag (they + // don't depend on the cursor). Playhead is read fresh per-move below since + // it's the one target that could (in theory) change during a drag. + const boundTargets = crossPropertyAndBoundTargets(clip, property); const onMove = (ev: globalThis.MouseEvent) => { const rel = xToFrame(ev.clientX); let newFrame = startFrame + rel; // Clamp to valid clip range. newFrame = Math.max(startFrame, Math.min(lastFrame, newFrame)); - // Snap to playhead when within threshold. - if (Math.abs(newFrame - activeFrame) <= SNAP_FRAMES) newFrame = activeFrame; + // Snap to the nearest of {playhead, clip start/end, other properties' + // keyframes} within SNAP_FRAMES (upstream KeyframesLane.swift:177-216). + const { frame: snapped, snappedTo } = snapFrame( + newFrame, + [...boundTargets, activeFrame], + SNAP_FRAMES, + ); + newFrame = snapped; + onSnapChange?.(snappedTo); setDragging((d) => (d ? { ...d, currentFrame: newFrame } : d)); }; const onUp = () => { window.removeEventListener("mousemove", onMove); window.removeEventListener("mouseup", onUp); dragCleanupRef.current = null; + onSnapChange?.(null); setDragging((d) => { if (d && d.fromFrame !== d.currentFrame) { void edit.moveKeyframe(clip.id, property, d.fromFrame, d.currentFrame); diff --git a/web/src/components/inspector/KeyframesPanel.tsx b/web/src/components/inspector/KeyframesPanel.tsx index bb3257a..7bd9f1c 100644 --- a/web/src/components/inspector/KeyframesPanel.tsx +++ b/web/src/components/inspector/KeyframesPanel.tsx @@ -2,28 +2,41 @@ * KeyframesPanel (SPEC §6.4). Inspector sub-panel that renders one row per * animatable property for the selected clip, each row a `KeyframesLaneRow` with * draggable diamond markers. A panel-wide red playhead line overlays every row - * so the user can see the current frame against every track at once. + * so the user can see the current frame against every track at once. A second, + * yellow, panel-wide line appears only while a keyframe drag is actively + * snapped (cross-property / playhead / clip-bound target) — 1:1 port of + * upstream's `KeyframesPanel.snapOverlay` (Inspector/Keyframes/KeyframesLane.swift:301-313). * * Layout: the outer div carries the top border + padding (visual spacing from * the Inspector edges). An inner `position: relative` wrapper (no padding) - * holds the ruler, the rows, and the playhead overlay. Both the playhead + * holds the ruler, the rows, and the overlays. The playhead/snap overlays * (`left: X%`) and the keyframe diamonds inside each row (`left: X%`) resolve * against the same width — the inner wrapper's content box — so they align * exactly at every frame. (If the overlay were positioned against the padded * outer box, the playhead would drift from the diamonds by up to the padding * width at the clip's start/end.) + * + * Snap-line state lives here (lifted from the individual rows) because the + * line must span every row, not just the one being dragged — each + * `KeyframesLaneRow` reports its live snap frame via `onSnapChange`, which + * only ever writes to this state while that row owns the active drag. */ +import { useState } from "react"; import { useEditorUiStore } from "../../store/uiStore"; +import { useProjectStore } from "../../store/projectStore"; import type { TFunction } from "../../i18n"; import type { Clip, KeyframeProperty } from "../../lib/types"; import { KeyframesLaneRow } from "./KeyframesLaneRow"; +import { KeyframesRuler } from "./KeyframesRuler"; const VIDEO_PROPERTIES: KeyframeProperty[] = ["position", "scale", "rotation", "opacity", "crop"]; const AUDIO_PROPERTIES: KeyframeProperty[] = ["volume"]; export function KeyframesPanel({ clip, t }: { clip: Clip; t: TFunction }) { const activeFrame = useEditorUiStore((s) => s.activeFrame); + const fps = useProjectStore((s) => s.timeline.fps); + const [snappedFrame, setSnappedFrame] = useState(null); const properties = clip.mediaType === "audio" ? AUDIO_PROPERTIES : VIDEO_PROPERTIES; const startFrame = clip.startFrame; const endFrame = clip.startFrame + clip.durationFrames; @@ -31,6 +44,10 @@ export function KeyframesPanel({ clip, t }: { clip: Clip; t: TFunction }) { // Playhead position within the clip (0..1), clamped to the clip's span. const playheadRatio = Math.max(0, Math.min(1, (activeFrame - startFrame) / duration)); + const snapRatio = + snappedFrame === null + ? null + : Math.max(0, Math.min(1, (snappedFrame - startFrame) / duration)); return (
- {/* Ruler bar — a thin tinted strip showing the clip's span. */} -
-
-
+ {/* Real tick ruler spanning the clip's frame range. */} + {/* Property rows. */} {properties.map((prop) => ( - + ))} {/* Panel-wide playhead overlay — spans the full height of the inner @@ -89,6 +88,26 @@ export function KeyframesPanel({ clip, t }: { clip: Clip; t: TFunction }) { zIndex: 5, }} /> + + {/* Panel-wide yellow snap-guide overlay — only visible while a + keyframe drag is actively snapped to a target. Dashed, matching + both upstream's `StrokeStyle(dash: [4, 4])` and the main + timeline's own SnapIndicator dashed line. */} + {snapRatio !== null && ( +
+ )}
); diff --git a/web/src/components/inspector/KeyframesRuler.tsx b/web/src/components/inspector/KeyframesRuler.tsx new file mode 100644 index 0000000..d86f515 --- /dev/null +++ b/web/src/components/inspector/KeyframesRuler.tsx @@ -0,0 +1,106 @@ +/** + * KeyframesRuler (SPEC §6.4, T2-8 part 3). Real tick ruler for the top of + * KeyframesPanel, replacing the old static tinted strip. 1:1 reuse of the + * main timeline's tick-interval logic — upstream's `ClipRulerBlock` wraps a + * `RulerView` that itself just calls the shared `TimelineRuler.draw` + * (Inspector/Keyframes/KeyframesLane.swift:335-362); OpenTake's equivalent + * shared logic is `chooseTicks` (lib/ruler.ts, already a 1:1 port of + * `TimelineRuler.swift`) plus `formatTimecode` for the major-tick labels, + * which is exactly what the main timeline's own ruler painter + * (components/timeline/rulerCanvas.ts) uses. + * + * Rendered as absolutely-positioned divs (not canvas) to match this panel's + * existing JSX/CSS-var idiom (diamonds and the playhead overlay are divs too). + * Ticks are clip-relative: frame 0 = clip start, so `pixelsPerFrame = width / + * duration` — unlike the scrolling main-timeline ruler, there is no + * `scrollLeft` to account for. + */ + +import { useLayoutEffect, useRef, useState } from "react"; +import { chooseTicks } from "../../lib/ruler"; +import { formatTimecode } from "../../lib/geometry"; + +const RULER_HEIGHT = 18; // upstream KeyframesMetrics.rulerHeight (KeyframesLane.swift:5) + +export function KeyframesRuler({ duration, fps }: { duration: number; fps: number }) { + const ref = useRef(null); + const [width, setWidth] = useState(0); + + useLayoutEffect(() => { + const el = ref.current; + if (!el) return; + const observer = new ResizeObserver((entries) => { + const w = entries[0]?.contentRect.width; + if (typeof w === "number") setWidth(w); + }); + observer.observe(el); + setWidth(el.getBoundingClientRect().width); + return () => observer.disconnect(); + }, []); + + const pixelsPerFrame = width > 0 ? width / duration : 0; + const ticks = pixelsPerFrame > 0 ? buildTicks(duration, fps, pixelsPerFrame) : []; + + return ( +
+ {ticks.map((tick) => ( +
+ {tick.major && tick.label && ( + + {tick.label} + + )} +
+ ))} +
+ ); +} + +interface Tick { + frame: number; + major: boolean; + label: string | null; +} + +/** Build clip-relative ticks spanning [0, duration] using the shared + * major/minor interval selection (chooseTicks, a 1:1 port of + * TimelineRuler.swift's own interval logic). */ +function buildTicks(duration: number, fps: number, pixelsPerFrame: number): Tick[] { + const { majorInterval, minorSubdivisions } = chooseTicks(pixelsPerFrame, fps); + const ticks: Tick[] = []; + const minorInterval = majorInterval / minorSubdivisions; + for (let f = 0; f <= duration; f += minorInterval) { + const frame = Math.round(f); + const isMajor = frame % majorInterval === 0; + ticks.push({ frame, major: isMajor, label: isMajor ? formatTimecode(frame, fps) : null }); + } + return ticks; +} diff --git a/web/src/lib/keyframeSnap.test.ts b/web/src/lib/keyframeSnap.test.ts new file mode 100644 index 0000000..35606d5 --- /dev/null +++ b/web/src/lib/keyframeSnap.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; +import { snapFrame } from "./keyframeSnap"; + +describe("snapFrame", () => { + it("snaps to the nearest target within threshold", () => { + expect(snapFrame(100, [90, 103, 120], 5)).toEqual({ frame: 103, snappedTo: 103 }); + }); + + it("returns the candidate unchanged with snappedTo=null when nothing is within threshold", () => { + expect(snapFrame(100, [50, 200], 5)).toEqual({ frame: 100, snappedTo: null }); + }); + + it("returns the candidate unchanged with snappedTo=null for an empty target list", () => { + expect(snapFrame(100, [], 5)).toEqual({ frame: 100, snappedTo: null }); + }); + + it("resolves equal-distance ties deterministically by picking the first target encountered", () => { + // 98 and 102 are both distance 2 from candidate 100. + expect(snapFrame(100, [98, 102], 5)).toEqual({ frame: 98, snappedTo: 98 }); + // Reversed order still picks the first-encountered tie. + expect(snapFrame(100, [102, 98], 5)).toEqual({ frame: 102, snappedTo: 102 }); + }); + + it("snaps exactly at the threshold boundary (inclusive)", () => { + expect(snapFrame(100, [105], 5)).toEqual({ frame: 105, snappedTo: 105 }); + }); + + it("does not snap just past the threshold boundary", () => { + expect(snapFrame(100, [106], 5)).toEqual({ frame: 100, snappedTo: null }); + }); + + it("snaps when the candidate already equals a target (distance 0)", () => { + expect(snapFrame(100, [100, 90], 5)).toEqual({ frame: 100, snappedTo: 100 }); + }); + + it("considers playhead, cross-property, and clip-bound targets together, nearest wins", () => { + // Simulates: playhead=50, clip start=10, clip end=90, other-property kf=97. + const targets = [50, 10, 90, 97]; + expect(snapFrame(95, targets, 5)).toEqual({ frame: 97, snappedTo: 97 }); + }); + + it("ignores negative or non-finite thresholds gracefully (no snap)", () => { + expect(snapFrame(100, [100], 0)).toEqual({ frame: 100, snappedTo: 100 }); + expect(snapFrame(100, [101], 0)).toEqual({ frame: 100, snappedTo: null }); + }); +}); diff --git a/web/src/lib/keyframeSnap.ts b/web/src/lib/keyframeSnap.ts new file mode 100644 index 0000000..cba801e --- /dev/null +++ b/web/src/lib/keyframeSnap.ts @@ -0,0 +1,42 @@ +/** + * Pure keyframe snap resolver for the Inspector KeyframesPanel (SPEC §6.4). + * Mirrors upstream `KeyframesLaneRow.applySnap` (Inspector/Keyframes/KeyframesLane.swift) + * at the "which target wins" level: nearest target within threshold wins, no + * per-kind threshold weighting (unlike the main-timeline `lib/snap.ts` port, + * which gives the playhead a larger threshold multiplier — upstream's keyframe + * lane treats playhead/clip-edge/cross-property targets uniformly, see + * KeyframesLane.swift:177-216 where all `SnapTarget`s share one + * `snapThresholdPixels` call). + */ + +export interface KeyframeSnapResult { + /** The candidate frame, replaced by the nearest in-threshold target if any. */ + frame: number; + /** The target frame snapped to, or null if the candidate was left unchanged. */ + snappedTo: number | null; +} + +/** + * Snap `candidate` to the nearest value in `targets` that is within + * `threshold` frames. Ties resolve to whichever target is encountered first + * in `targets` (deterministic, order-dependent — callers control tie-break + * priority via target ordering). Returns the unchanged candidate with + * `snappedTo: null` when no target is within threshold (including an empty + * target list or a non-positive threshold). + */ +export function snapFrame( + candidate: number, + targets: number[], + threshold: number, +): KeyframeSnapResult { + let best: number | null = null; + let bestDist = Number.POSITIVE_INFINITY; + for (const target of targets) { + const dist = Math.abs(candidate - target); + if (dist <= threshold && dist < bestDist) { + bestDist = dist; + best = target; + } + } + return best === null ? { frame: candidate, snappedTo: null } : { frame: best, snappedTo: best }; +} diff --git a/web/src/styles/tokens.css b/web/src/styles/tokens.css index 9d445d7..1bc723c 100644 --- a/web/src/styles/tokens.css +++ b/web/src/styles/tokens.css @@ -34,6 +34,9 @@ --accent-primary: rgb(245, 239, 228); --accent-spotlight: rgb(255, 69, 69); --status-error: rgb(229, 79, 79); + /* SwiftUI .yellow (matches ACCENT.systemYellow in theme.ts / the main + timeline's SnapIndicator dashed snap-guide line) */ + --status-warning: rgb(255, 204, 0); --glass-tint: rgba(245, 239, 228, 0.05); --ai-gradient: linear-gradient(135deg, #fff 0%, #c7c7c7 45%, #999 55%, #fff 100%); --spotlight-gradient: linear-gradient(135deg, rgb(255, 87, 77), rgb(242, 38, 71), rgb(255, 122, 56));