diff --git a/packages/core/src/runtime/timeline.test.ts b/packages/core/src/runtime/timeline.test.ts index f1d08dfa7..e22c78a49 100644 --- a/packages/core/src/runtime/timeline.test.ts +++ b/packages/core/src/runtime/timeline.test.ts @@ -39,6 +39,44 @@ describe("collectRuntimeTimelinePayload", () => { expect(result.clips[0].id).toBe("hf-headline"); }); + // Regression: the authored data-track-index must round-trip verbatim, even + // when clips of DIFFERENT kinds (video vs element) share a track. The old + // mixed-kind renumber split them onto separate tracks, which made the + // written track drift from the displayed one on every editor move. + it("honors authored track indices verbatim for mixed-kind tracks", () => { + const root = document.createElement("div"); + root.setAttribute("data-composition-id", "main"); + root.setAttribute("data-duration", "20"); + document.body.appendChild(root); + + const video = document.createElement("video"); + video.id = "clip-video"; + video.setAttribute("data-start", "1"); + video.setAttribute("data-duration", "3"); + video.setAttribute("data-track-index", "1"); + root.appendChild(video); + + const caption = document.createElement("div"); + caption.id = "clip-caption"; + caption.setAttribute("data-start", "8"); + caption.setAttribute("data-duration", "3"); + caption.setAttribute("data-track-index", "1"); + root.appendChild(caption); + + const other = document.createElement("div"); + other.id = "clip-other"; + other.setAttribute("data-start", "0"); + other.setAttribute("data-duration", "3"); + other.setAttribute("data-track-index", "2"); + root.appendChild(other); + + const result = collectRuntimeTimelinePayload(defaultParams); + const trackOf = (id: string) => result.clips.find((c) => c.id === id)?.track; + expect(trackOf("clip-video")).toBe(1); + expect(trackOf("clip-caption")).toBe(1); + expect(trackOf("clip-other")).toBe(2); + }); + it("collects clips from elements with data-start and data-duration", () => { const root = document.createElement("div"); root.setAttribute("data-composition-id", "main"); diff --git a/packages/core/src/runtime/timeline.ts b/packages/core/src/runtime/timeline.ts index 0cd123fc0..23e76d4b7 100644 --- a/packages/core/src/runtime/timeline.ts +++ b/packages/core/src/runtime/timeline.ts @@ -52,57 +52,15 @@ function maxDefinedNumber(...values: Array): number | null { } /** - * When multiple content kinds share the same track number, split them - * onto separate tracks so the timeline UI shows distinct rows. - * - * Preferred kind order (top → bottom): composition, video, image, element, audio. - * Tracks that contain only one kind are left untouched. + * Parse an authored track attribute, honoring 0 (a valid top-lane index). + * `parseInt(...) || fallback` silently replaced authored track 0 with the + * synthetic fallback, so track-0 clips drifted to the bottom of the timeline. */ -const KIND_ORDER: Record = { - composition: 0, - video: 1, - image: 2, - element: 3, - audio: 4, -}; - -function normalizeTrackAssignments(clips: RuntimeTimelineClip[]): void { - if (clips.length === 0) return; - - // Group clips by their raw track number and detect which tracks have mixed kinds - const trackKinds = new Map>(); - for (const clip of clips) { - const kinds = trackKinds.get(clip.track) ?? new Set(); - kinds.add(clip.kind); - trackKinds.set(clip.track, kinds); - } - - const hasMixedTracks = Array.from(trackKinds.values()).some((kinds) => kinds.size > 1); - if (!hasMixedTracks) return; - - // Build new contiguous track numbers, splitting mixed tracks by kind - let nextTrack = 0; - const newTrackMap = new Map(); // "origTrack:kind" → newTrack - - const sortedTracks = [...trackKinds.keys()].sort((a, b) => a - b); - for (const track of sortedTracks) { - const kinds = trackKinds.get(track)!; - if (kinds.size === 1) { - newTrackMap.set(`${track}:${[...kinds][0]}`, nextTrack++); - } else { - // Split by kind in preferred order - const sorted = [...kinds].sort((a, b) => (KIND_ORDER[a] ?? 99) - (KIND_ORDER[b] ?? 99)); - for (const kind of sorted) { - newTrackMap.set(`${track}:${kind}`, nextTrack++); - } - } - } - - for (const clip of clips) { - const key = `${clip.track}:${clip.kind}`; - const newTrack = newTrackMap.get(key); - if (newTrack != null) clip.track = newTrack; - } +function parseAuthoredTrack(el: Element, fallback: number): number { + const raw = el.getAttribute("data-track-index") ?? el.getAttribute("data-track"); + if (raw == null) return fallback; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) ? parsed : fallback; } function toAbsoluteAssetUrl(rawValue: string | null | undefined): string | null { @@ -441,11 +399,7 @@ export function collectRuntimeTimelinePayload(params: { label: buildTimelineClipLabel(node, kind, clips.length), start, duration, - track: - Number.parseInt( - node.getAttribute("data-track-index") ?? node.getAttribute("data-track") ?? String(i), - 10, - ) || 0, + track: parseAuthoredTrack(node, i), zIndex: readInlineZIndex(node), stackingContextId: compositionContext.parentCompositionId ?? rootCompositionId, kind, @@ -554,11 +508,7 @@ export function collectRuntimeTimelinePayload(params: { el.id, start: range.start, duration: clampedDuration, - track: - Number.parseInt( - el.getAttribute("data-track-index") ?? el.getAttribute("data-track") ?? "", - 10, - ) || gsapTrack, + track: parseAuthoredTrack(el, gsapTrack), zIndex: readInlineZIndex(el), stackingContextId: rootCompositionIdForGsap, kind: "element", @@ -613,11 +563,7 @@ export function collectRuntimeTimelinePayload(params: { el.id, start: 0, duration: clampedDuration, - track: - Number.parseInt( - el.getAttribute("data-track-index") ?? el.getAttribute("data-track") ?? "", - 10, - ) || overlayTrack, + track: parseAuthoredTrack(el, overlayTrack), zIndex: readInlineZIndex(el), stackingContextId: rootCompositionIdForGsap, kind: "element", @@ -637,11 +583,12 @@ export function collectRuntimeTimelinePayload(params: { } } - // ── Track normalization ──────────────────────────────────────────────── - // When multiple content kinds (composition, audio, video, …) share the same - // data-track-index value, split them onto separate tracks so the timeline UI - // shows distinct rows for each kind. - normalizeTrackAssignments(clips); + // Track assignment honors the authored data-track-index verbatim: a clip stays + // on the track it was placed on, regardless of kind. (Previously mixed-kind + // tracks were split onto separate rows, but that renumbered tracks — breaking + // "drop a clip onto an existing track" and causing the written track to drift + // from the displayed one on every move. Track index is display-only; render + // never reads it, so honoring it verbatim is the correct NLE behavior.) for (const compositionNode of compositionNodes) { if (compositionNode === root) continue; diff --git a/packages/studio/src/hooks/timelineEditingHelpers.test.ts b/packages/studio/src/hooks/timelineEditingHelpers.test.ts index d76f4898e..64251f17b 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.test.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.test.ts @@ -139,6 +139,26 @@ describe("persistTimelineBatchEdit", () => { ); } + function moveMember( + id: string, + start: number, + fromTrack: number, + toTrack: number, + ): PersistTimelineBatchChange { + return { + element: el({ id, tag: "video", domId: id, start, track: fromTrack }), + buildPatches: (original, target) => + buildTimelineMoveTimingPatch(original, target, start, 5, toTrack), + }; + } + + async function runBatch(changes: PersistTimelineBatchChange[]) { + stubReadFileContent(SOURCE); + const writes: Array<[string, string]> = []; + await persistTimelineBatchEdit(batchInput(changes, writes)); + return writes; + } + afterEach(() => { vi.unstubAllGlobals(); }); @@ -147,28 +167,12 @@ describe("persistTimelineBatchEdit", () => { // A track-insert renumber can include a member whose attributes already // hold the target values — its patch is string-identical. The batch must // skip it and still persist the members that DID change. - stubReadFileContent(SOURCE); - const writes: Array<[string, string]> = []; - - await persistTimelineBatchEdit( - batchInput( - [ - { - // no-op: data-start already "1", track already 0 - element: el({ id: "a", tag: "video", domId: "a", start: 1, track: 0 }), - buildPatches: (original, target) => - buildTimelineMoveTimingPatch(original, target, 1, 5, 0), - }, - { - // real change: track 1 -> 2 - element: el({ id: "b", tag: "video", domId: "b", start: 2, track: 1 }), - buildPatches: (original, target) => - buildTimelineMoveTimingPatch(original, target, 2, 5, 2), - }, - ], - writes, - ), - ); + const writes = await runBatch([ + // no-op: data-start already "1", track already 0 + moveMember("a", 1, 0, 0), + // real change: track 1 -> 2 + moveMember("b", 2, 1, 2), + ]); expect(writes).toHaveLength(1); expect(writes[0]![0]).toBe("index.html"); @@ -176,21 +180,7 @@ describe("persistTimelineBatchEdit", () => { }); it("saves nothing when every member is a no-op", async () => { - stubReadFileContent(SOURCE); - const writes: Array<[string, string]> = []; - - await persistTimelineBatchEdit( - batchInput( - [ - { - element: el({ id: "a", tag: "video", domId: "a", start: 1, track: 0 }), - buildPatches: (original, target) => - buildTimelineMoveTimingPatch(original, target, 1, 5, 0), - }, - ], - writes, - ), - ); + const writes = await runBatch([moveMember("a", 1, 0, 0)]); expect(writes).toHaveLength(0); }); diff --git a/packages/studio/src/hooks/timelineMoveAdapter.test.ts b/packages/studio/src/hooks/timelineMoveAdapter.test.ts index 65b77c945..a64dab810 100644 --- a/packages/studio/src/hooks/timelineMoveAdapter.test.ts +++ b/packages/studio/src/hooks/timelineMoveAdapter.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it, vi } from "vitest"; import type { TimelineElement } from "../player"; import { persistTimelineMoveEditsAtomically } from "./timelineMoveAdapter"; +type MoveArgs = Parameters; + const element = (id: string, track: number): TimelineElement => ({ id, key: id, @@ -11,64 +13,66 @@ const element = (id: string, track: number): TimelineElement => ({ track, }); +const twoLaneEdits = (bTrack: number): MoveArgs[0] => [ + { element: element("a", 0), updates: { start: 1, track: 1 } }, + { element: element("b", bTrack), updates: { start: 3, track: 2 } }, +]; + +const movedPair = (edits: MoveArgs[0]) => [ + { element: edits[0].element, start: 1, track: 1 }, + { element: edits[1].element, start: 3, track: 2 }, +]; + +const runMove = async (edits: MoveArgs[0], coalesceKey: MoveArgs[1], intent: MoveArgs[2]) => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + await persistTimelineMoveEditsAtomically(edits, coalesceKey, intent, { + handleTimelineGroupMove, + }); + return handleTimelineGroupMove; +}; + describe("persistTimelineMoveEditsAtomically", () => { it("persists two vertical edits as one group with the gesture coalesce key", async () => { - const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); - const edits = [ - { element: element("a", 0), updates: { start: 1, track: 1 } }, - { element: element("b", 1), updates: { start: 3, track: 2 } }, - ]; - await persistTimelineMoveEditsAtomically(edits, "clip-lane-move:7", "track-insert", { - handleTimelineGroupMove, - }); + const edits = twoLaneEdits(1); + const handleTimelineGroupMove = await runMove(edits, "clip-lane-move:7", "track-insert"); expect(handleTimelineGroupMove).toHaveBeenCalledTimes(1); - expect(handleTimelineGroupMove).toHaveBeenCalledWith( - [ - { element: edits[0].element, start: 1, track: 1 }, - { element: edits[1].element, start: 3, track: 2 }, - ], - { coalesceKey: "clip-lane-move:7" }, - ); - }); - - it("does not persist track attrs for a single z-only lane reorder", async () => { - const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); - const edit = { element: element("a", 0), updates: { start: 1, track: 1 } }; - await persistTimelineMoveEditsAtomically([edit], "clip-lane-move:7", "lane-reorder", { - handleTimelineGroupMove, - }); - expect(handleTimelineGroupMove).toHaveBeenCalledWith([{ element: edit.element, start: 1 }], { + expect(handleTimelineGroupMove).toHaveBeenCalledWith(movedPair(edits), { coalesceKey: "clip-lane-move:7", }); }); - it("does not persist track attrs for a multi-selection lane drag", async () => { - const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); - const edits = [ - { element: element("a", 0), updates: { start: 1, track: 1 } }, - { element: element("b", 2), updates: { start: 3, track: 2 } }, - ]; - await persistTimelineMoveEditsAtomically(edits, "clip-lane-move:7", "lane-reorder", { - handleTimelineGroupMove, + it("omits track attrs for plain timing moves (keeps the SDK fast path eligible)", async () => { + const edit = { element: element("a", 0), updates: { start: 1, track: 0 } }; + const handleTimelineGroupMove = await runMove([edit], undefined, "timing"); + expect(handleTimelineGroupMove).toHaveBeenCalledWith([{ element: edit.element, start: 1 }], { + coalesceKey: undefined, }); + }); + + it("persists the track attr for a single lane reorder (stable track lanes)", async () => { + // Lane = authored data-track-index; a vertical move that never hits disk + // snaps back on the next normalize, so the lane change MUST persist. + const edit = { element: element("a", 0), updates: { start: 1, track: 1 } }; + const handleTimelineGroupMove = await runMove([edit], "clip-lane-move:7", "lane-reorder"); expect(handleTimelineGroupMove).toHaveBeenCalledWith( - [ - { element: edits[0].element, start: 1 }, - { element: edits[1].element, start: 3 }, - ], + [{ element: edit.element, start: 1, track: 1 }], { coalesceKey: "clip-lane-move:7" }, ); }); + it("persists track attrs for a multi-selection lane drag (stable track lanes)", async () => { + const edits = twoLaneEdits(2); + const handleTimelineGroupMove = await runMove(edits, "clip-lane-move:7", "lane-reorder"); + expect(handleTimelineGroupMove).toHaveBeenCalledWith(movedPair(edits), { + coalesceKey: "clip-lane-move:7", + }); + }); + it("rejects without retrying individual members when the atomic batch fails", async () => { const failure = new Error("batch failed"); const handleTimelineGroupMove = vi.fn().mockRejectedValue(failure); - const edits = [ - { element: element("a", 0), updates: { start: 1, track: 1 } }, - { element: element("b", 1), updates: { start: 3, track: 2 } }, - ]; await expect( - persistTimelineMoveEditsAtomically(edits, "clip-lane-move:7", "track-insert", { + persistTimelineMoveEditsAtomically(twoLaneEdits(1), "clip-lane-move:7", "track-insert", { handleTimelineGroupMove, }), ).rejects.toBe(failure); diff --git a/packages/studio/src/hooks/timelineMoveAdapter.ts b/packages/studio/src/hooks/timelineMoveAdapter.ts index e64c77076..7672888e6 100644 --- a/packages/studio/src/hooks/timelineMoveAdapter.ts +++ b/packages/studio/src/hooks/timelineMoveAdapter.ts @@ -28,9 +28,11 @@ export function persistTimelineMoveEditsAtomically( edits.map(({ element, updates }) => ({ element, start: updates.start, - // A single vertical edit is the z-only reorder path. Multi-edit gestures - // are track inserts/ripples and must persist every resulting lane index. - track: operation === "track-insert" ? updates.track : undefined, + // Stable track lanes: a lane is the authored data-track-index, so every + // vertical gesture (lane-reorder AND track-insert) must persist the track; + // z is paint order only and is synced separately. Plain horizontal moves + // ("timing") omit it so they stay eligible for the SDK fast path. + track: operation === "timing" ? undefined : updates.track, })), { coalesceKey }, ); diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index 5cf07b0fe..975c1d735 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -160,23 +160,27 @@ export function useTimelineEditing({ if (!startChanged) return reorderDone; - // needsExtension gates the SDK path (setTiming can't grow the root duration), - // so read the store BEFORE the readout sync below optimistically updates it. + // needsExtension gates the SDK path (setTiming can't grow the root duration), so read the store BEFORE the readout sync below optimistically updates it. const needsExtension = extendRootDurationIfNeeded(updates.start + element.duration); - // Optimistic duration readout: content-driven (grow AND shrink), read from - // the just-patched live DOM. See syncPreviewContentDuration. + // Optimistic duration readout: content-driven (grow AND shrink), from the just-patched live DOM. See syncPreviewContentDuration. syncPreviewContentDuration(previewIframeRef.current); const buildMovePatches: PersistTimelineEditInput["buildPatches"] = (original, target) => { - return buildTimelineMoveTimingPatch(original, target, updates.start, element.duration); + // Persist lane changes too — data-start-only writes let reload snap the lane back. + const track = updates.track !== element.track ? updates.track : undefined; + return buildTimelineMoveTimingPatch( + original, + target, + updates.start, + element.duration, + track, + ); }; const coalesceKey = `timeline-move:${element.hfId ?? element.id}`; const moveFallback = () => enqueueEdit(element, "Move timeline clip", buildMovePatches, coalesceKey).then(() => - // Soft-reload with the server's rewritten GSAP script instead of a full - // iframe reload — a timing-only move already patched the DOM + store, so - // swapping the script in place avoids the all-clips flash. Falls back to - // reloadPreview() when the soft path can't apply. (See timelineTimingSync.) + // Soft-reload with the server's rewritten GSAP script — the timing-only move already patched + // DOM + store, so swapping the script avoids the all-clips flash; falls back to reloadPreview(). finishClipTimingFallback({ iframe: previewIframeRef.current, reloadPreview, @@ -249,11 +253,9 @@ export function useTimelineEditing({ liveAttrs.push([liveAttr, formatTimelineAttributeNumber(updates.playbackStart)]); } patchIframeDomTiming(previewIframeRef.current, element, liveAttrs); - // needsExtension gates the SDK path (setTiming can't grow the root duration), - // so read the store BEFORE the readout sync below optimistically updates it. + // needsExtension gates the SDK path (setTiming can't grow the root duration), so read the store BEFORE the readout sync below optimistically updates it. const needsExtension = extendRootDurationIfNeeded(updates.start + updates.duration); - // Optimistic duration readout: content-driven (grow AND shrink), read from - // the just-patched live DOM. See syncPreviewContentDuration. + // Optimistic duration readout: content-driven (grow AND shrink), from the just-patched live DOM. See syncPreviewContentDuration. syncPreviewContentDuration(previewIframeRef.current); const targetPath = element.sourceFile || activeCompPath || "index.html"; const buildResizePatches: PersistTimelineEditInput["buildPatches"] = (original, target) => { diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index 2dc4179ff..1963646f1 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -77,6 +77,15 @@ function resizeCoalesceKey(changes: readonly TimelineGroupResizeChange[]): strin return `timeline-group-resize:${changes.map((change) => change.element.hfId ?? change.element.id).join(",")}`; } +function toSdkTimingChanges( + changes: readonly T[], + timingUpdate: (change: T) => { start: number; duration?: number }, +): Array<{ hfId: string; timingUpdate: { start: number; duration?: number } } | null> { + return changes.map((change) => + change.element.hfId ? { hfId: change.element.hfId, timingUpdate: timingUpdate(change) } : null, + ); +} + function resizeHasPlaybackStartAdjustment(change: TimelineGroupResizeChange): boolean { return ( change.playbackStart != null || @@ -225,11 +234,7 @@ export function useTimelineGroupEditing({ await options?.beforeTiming; const handledBySdk = await trySdkBatchPersist({ changes, - sdkChanges: changes.map((change) => - change.element.hfId - ? { hfId: change.element.hfId, timingUpdate: { start: change.start } } - : null, - ), + sdkChanges: toSdkTimingChanges(changes, (change) => ({ start: change.start })), eligible: changes.every((change) => change.track == null), needsExtension, label: "Move timeline clips", @@ -314,14 +319,10 @@ export function useTimelineGroupEditing({ await options?.beforeTiming; const handledBySdk = await trySdkBatchPersist({ changes, - sdkChanges: changes.map((change) => - change.element.hfId - ? { - hfId: change.element.hfId, - timingUpdate: { start: change.start, duration: change.duration }, - } - : null, - ), + sdkChanges: toSdkTimingChanges(changes, (change) => ({ + start: change.start, + duration: change.duration, + })), eligible: changes.every((change) => !resizeHasPlaybackStartAdjustment(change)), needsExtension, label: "Resize timeline clips", diff --git a/packages/studio/src/player/components/PlayheadIndicator.tsx b/packages/studio/src/player/components/PlayheadIndicator.tsx index d2b8a7f69..f6793f392 100644 --- a/packages/studio/src/player/components/PlayheadIndicator.tsx +++ b/packages/studio/src/player/components/PlayheadIndicator.tsx @@ -9,6 +9,8 @@ * no matter how far the tracks are scrolled. The head is OUTLINE-only at rest and * FILLED while the playhead is actively held/scrubbed (`scrubbing`). */ +import { PLAYHEAD_HEAD_W } from "./timelineLayout"; + interface PlayheadIndicatorProps { /** CSS color, defaults to the HF accent variable */ color?: string; @@ -31,8 +33,11 @@ export function PlayheadIndicator({ }: PlayheadIndicatorProps) { // Head chip dimensions — used to compute the centering offset and the // point where the vertical line starts (so it begins at the head's bottom - // edge rather than running through the hollow diamond center). - const HEAD_W = 9; + // edge rather than running through the hollow diamond center). The width is + // the shared PLAYHEAD_HEAD_W constant: getTimelinePlayheadLeft shifts the + // wrapper by -PLAYHEAD_HEAD_W/2 so the 1px line (centered at 50% of the + // wrapper) lands exactly on GUTTER + time * pps — the ruler ticks' center x. + const HEAD_W = PLAYHEAD_HEAD_W; const HEAD_H = 9; // marginTop(1) + HEAD_H = where the line should start. const HEAD_TOTAL_H = 1 + HEAD_H; diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index 600e05425..8926060c7 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -19,8 +19,10 @@ import { shouldAutoScrollTimeline, } from "./Timeline"; import { + FIT_ZOOM_HEADROOM, GUTTER, MIN_TIMELINE_EXTENT_S, + PLAYHEAD_HEAD_W, RULER_H, TRACK_H, getTimelineDisplayContentWidth, @@ -327,6 +329,60 @@ describe("generateTicks", () => { const { major } = generateTicks(180, 80); expect(major[1] - major[0]).toBe(2); }); + + it("picks 'nice' NLE steps across zoom levels (no 7s-style intervals)", () => { + // step = first nice interval whose px spacing >= 88 at that pps. + const cases: Array<[number, number]> = [ + [2, 60], // 60s * 2pps = 120px + [10, 10], // 10s * 10pps = 100px + [20, 5], // 5s * 20pps = 100px + [50, 2], // 2s * 50pps = 100px + [100, 1], // 1s * 100pps = 100px + ]; + for (const [pps, expected] of cases) { + const { major } = generateTicks(600, pps); + expect(major[1] - major[0]).toBe(expected); + } + }); + + it("uses minute/hour steps when zoomed far out instead of colliding 10m labels", () => { + // 0.05 pps → 600s step would be 30px apart (labels collide); 1800s = 90px. + const { major } = generateTicks(7200, 0.05); + expect(major[1] - major[0]).toBe(1800); + expect(major).toContain(3600); + }); + + it("does not drift on long rulers (ticks are exact multiples of the step)", () => { + const { major } = generateTicks(600, 100); // 1s step, 601 ticks + expect(major[599]).toBe(599); + }); + + describe("frame display mode (frameRate provided)", () => { + it("snaps sub-frame steps up to one whole frame (no duplicate frame labels)", () => { + // 4400 pps would pick a 0.02s step = 0.6 frames at 30fps → snapped to 1 frame. + const { major } = generateTicks(2, 4400, 30); + const frames = major.map((t) => Math.round(t * 30)); + // Frame labels are consecutive integers — no duplicates, no gaps. + frames.forEach((f, i) => expect(f).toBe(i)); + }); + + it("keeps major AND minor ticks on whole frames", () => { + // 200 pps → 0.5s step (15 frames); quarters (3.75f) are rejected in + // frame mode in favour of fifths (3f). + const { major, minor } = generateTicks(20, 200, 30); + expect(major[1]).toBeCloseTo(0.5); + expect(minor).toContain(0.1); // 3 frames + for (const t of [...major, ...minor]) { + const frames = t * 30; + expect(Math.abs(frames - Math.round(frames))).toBeLessThan(1e-3); + } + }); + + it("leaves whole-second steps unchanged", () => { + const { major } = generateTicks(60, 100, 30); + expect(major[1] - major[0]).toBe(1); + }); + }); }); describe("formatTime", () => { @@ -397,19 +453,33 @@ describe("shouldAutoScrollTimeline", () => { }); }); -describe("getTimelineFitPps (min 60s extent)", () => { +describe("getTimelineFitPps (min 60s extent + fit headroom)", () => { const viewport = 632; // usable width = 632 - GUTTER - 2 = 598 it("computes fit pps against the 60s floor for short compositions", () => { // A 10s comp maps 60s onto the viewport → the comp takes ~1/6 of the width. + // (10 * 1.2 = 12s of headroom-padded content is still under the 60s floor.) const pps = getTimelineFitPps(viewport, 10); expect(pps).toBeCloseTo((viewport - GUTTER - 2) / MIN_TIMELINE_EXTENT_S); expect(10 * pps).toBeCloseTo((viewport - GUTTER - 2) / 6); }); - it("keeps filling the viewport with the composition when it is 60s or longer", () => { - expect(getTimelineFitPps(viewport, 60)).toBeCloseTo((viewport - GUTTER - 2) / 60); - expect(getTimelineFitPps(viewport, 120)).toBeCloseTo((viewport - GUTTER - 2) / 120); + it("fits duration * FIT_ZOOM_HEADROOM (not the bare duration) for long compositions", () => { + expect(getTimelineFitPps(viewport, 60)).toBeCloseTo( + (viewport - GUTTER - 2) / (60 * FIT_ZOOM_HEADROOM), + ); + expect(getTimelineFitPps(viewport, 120)).toBeCloseTo( + (viewport - GUTTER - 2) / (120 * FIT_ZOOM_HEADROOM), + ); + }); + + it("leaves CapCut-style trailing headroom: the comp ends at 1/1.2 of the usable width", () => { + const usable = viewport - GUTTER - 2; + const pps = getTimelineFitPps(viewport, 120); + // Composition content occupies usable/1.2 px; the remaining ~17% is empty + // droppable ruler/lane surface past the end. + expect(120 * pps).toBeCloseTo(usable / FIT_ZOOM_HEADROOM); + expect(120 * pps).toBeLessThan(usable); }); it("falls back to 100 pps before the viewport is measured", () => { @@ -529,13 +599,20 @@ describe("getTimelineScrollLeftForZoomAnchor", () => { }); describe("getTimelinePlayheadLeft", () => { - it("converts time to a pixel offset from the gutter", () => { - expect(getTimelinePlayheadLeft(4, 20)).toBe(112); + it("offsets the wrapper by half the head width so the line CENTER = GUTTER + t*pps", () => { + // Wrapper left + PLAYHEAD_HEAD_W/2 (where the 1px line is centered) must + // equal GUTTER + t*pps at any zoom. + expect(getTimelinePlayheadLeft(4, 20) + PLAYHEAD_HEAD_W / 2).toBe(GUTTER + 4 * 20); + expect(getTimelinePlayheadLeft(10, 7.5) + PLAYHEAD_HEAD_W / 2).toBe(GUTTER + 75); + }); + + it("centers the line exactly on the gutter (the 00:00 tick) at t = 0", () => { + expect(getTimelinePlayheadLeft(0, 20) + PLAYHEAD_HEAD_W / 2).toBe(GUTTER); }); it("guards invalid input", () => { - expect(getTimelinePlayheadLeft(Number.NaN, 20)).toBe(32); - expect(getTimelinePlayheadLeft(4, Number.NaN)).toBe(32); + expect(getTimelinePlayheadLeft(Number.NaN, 20)).toBe(GUTTER - PLAYHEAD_HEAD_W / 2); + expect(getTimelinePlayheadLeft(4, Number.NaN)).toBe(GUTTER - PLAYHEAD_HEAD_W / 2); }); }); diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index e9568f131..8bd838e3c 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -26,6 +26,7 @@ import { getTimelineCanvasHeight, shouldShowTimelineShortcutHint, } from "./timelineLayout"; +import { STUDIO_PREVIEW_FPS } from "../lib/time"; import { useResolvedTimelineEditCallbacks } from "./useResolvedTimelineEditCallbacks"; import type { TimelineProps } from "./TimelineTypes"; @@ -95,6 +96,7 @@ export const Timeline = memo(function Timeline({ [beatAnalysis, musicElement, beatEdits], ); const duration = usePlayerStore((s) => s.duration); + const timeDisplayMode = usePlayerStore((s) => s.timeDisplayMode); const timelineReady = usePlayerStore((s) => s.timelineReady); const selectedElementId = usePlayerStore((s) => s.selectedElementId); const selectedElementIds = usePlayerStore((s) => s.selectedElementIds); @@ -156,11 +158,8 @@ export const Timeline = memo(function Timeline({ containerRef.current = el; }, []); - // Last horizontal scroll offset, tracked so it can be RESTORED across the - // post-edit iframe reload: an edit re-derives elements (and may shrink the - // content width), which the browser clamps into a scroll jump. Paired with the - // pinned zoom (which keeps pps constant so the pixel offset stays meaningful), - // restoring this keeps the user parked at the same spot after any edit. + // Last horizontal scroll offset, RESTORED across the post-edit iframe reload (which clamps into + // a scroll jump); with the pinned zoom this keeps the user parked at the same spot after edits. const lastScrollLeftRef = useRef(0); const setScrollRef = useCallback( (el: HTMLDivElement | null) => { @@ -395,9 +394,11 @@ export const Timeline = memo(function Timeline({ } }); + // Frame display mode labels ruler ticks as frame numbers — pass the fps so ticks snap to frames. + const tickFps = timeDisplayMode === "frame" ? STUDIO_PREVIEW_FPS : undefined; const { major, minor } = useMemo( - () => generateTicks(displayDuration, pps), - [displayDuration, pps], + () => generateTicks(displayDuration, pps, tickFps), + [displayDuration, pps, tickFps], ); const majorTickInterval = major.length >= 2 ? major[1] - major[0] : effectiveDuration; @@ -526,8 +527,7 @@ export const Timeline = memo(function Timeline({ const elKey = el.key ?? el.id; setSelectedElementId(elKey); onSelectElement?.(el); - // Visually select the clicked diamond (matches shift-click / motion-path - // selection); cleared above so this single-selects it. + // Select the clicked diamond (matches shift-click); cleared above so this single-selects. toggleSelectedKeyframe(`${elKey}:${pct}`); const absTime = el.start + (pct / 100) * el.duration; onSeek?.(absTime); diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index 1dc896483..f706088da 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -10,6 +10,8 @@ import { CLIP_Y, TRACKS_TOP_PAD, TRACKS_BOTTOM_PAD, + PLAYHEAD_HEAD_W, + getTimelinePlayheadLeft, getTimelineRowTop, } from "./timelineLayout"; import { usePlayerStore } from "../store/playerStore"; @@ -262,12 +264,16 @@ export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvas )} {/* Playhead — hidden while dragging a beat so its guideline doesn't - track the scrub and clutter the beat being moved. */} + track the scrub and clutter the beat being moved. Explicit width + + the half-head offset baked into getTimelinePlayheadLeft keep the + inner 1px line's CENTER exactly on GUTTER + t * pps (the ruler + ticks' center), instead of relying on shrink-wrap sizing. */}
+ {/* Each 1px tick line is shifted -0.5px so its CENTER sits exactly on + t * pps — matching the playhead line, which is also centered on + GUTTER + t * pps (see getTimelinePlayheadLeft). Without the shift + a tick spans [x, x+1) and its center is half a pixel right. */} {minor.map((t) => ( -
+
))} {major.map((t) => ( -
+
{ expect(map.b).toBeUndefined(); // the other clip is NOT rewritten }); + it("persists a lane change in AUTHORED track space when the file is sparse", () => { + // Discovery normalized authored tracks {1, 2} to display lanes {0, 1} + // (authoredTrack records the file value). Moving 'a' onto b's lane must + // write b's AUTHORED track (2) — writing the display lane (1) would target + // a's own authored row and silently no-op in the file. + const elements = [ + { ...el("a", 0, 0, 3), authoredTrack: 1 }, + { ...el("b", 1, 10, 3), authoredTrack: 2 }, + ]; + const { updateElement, onMoveElements } = runClipMove( + drag(elements[0], { previewStart: 20, previewTrack: 1, desiredTrack: 1 }), + { elements, trackOrder: [0, 1] }, + ); + // Store stays in display-lane space... + expect(updateElement).toHaveBeenCalledWith("a", { start: 20, track: 1 }); + // ...while the persist is translated to the target lane's authored track. + const map = editMap(onMoveElements.mock.calls[0][0]); + expect(map.a).toEqual({ start: 20, track: 2 }); + }); + it("multi-selection time-move shifts EVERY selected clip by the drag delta (atomic)", () => { const elements = [el("a", 0, 2, 3), el("b", 1, 10, 3), el("c", 2, 20, 3)]; // Drag 'a' +5s on its own lane while {a, b} are marquee-selected. diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index cb556933e..e320f6dc1 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -13,6 +13,13 @@ type StartTrack = Pick; export interface TimelineMoveEdit { element: TimelineElement; updates: StartTrack; + /** + * File-space track override for the persist. The store's `updates.track` is a + * DISPLAY lane; when the source file's numbering is sparse (authored tracks + * 1,2,... or gaps), the file write must target the lane's AUTHORED track or it + * silently re-targets the wrong row. Omitted → persist `updates.track` as-is. + */ + persistTrack?: number; } export interface DragCommitDeps { @@ -120,9 +127,16 @@ function persistMoveEdits( edits.map((edit) => keyOf(edit.element)), ); for (const e of edits) updateElement(keyOf(e.element), e.updates); + // The store above gets DISPLAY lanes; the file below gets the authored-space + // track when one was resolved (see TimelineMoveEdit.persistTrack). + const persistEdits = edits.map((e) => + e.persistTrack == null || e.persistTrack === e.updates.track + ? e + : { element: e.element, updates: { ...e.updates, track: e.persistTrack } }, + ); const persisted = onMoveElements - ? onMoveElements(edits, coalesceKey, operation) - : Promise.all(edits.map((e) => Promise.resolve(onMoveElement?.(e.element, e.updates)))); + ? onMoveElements(persistEdits, coalesceKey, operation) + : Promise.all(persistEdits.map((e) => Promise.resolve(onMoveElement?.(e.element, e.updates)))); return Promise.resolve(persisted).then( () => true, (error) => { @@ -143,6 +157,24 @@ function persistMoveEdits( * then compacts it to a distinct integer lane between its neighbours, and the * clips at/below the insert shift down by one — the sanctioned index-renumber. */ +/** + * Translate a DISPLAY lane into the AUTHORED (source-file) track to persist. + * The lane's occupants all share one authored track by construction (lane = + * authored track after normalizeToZones; overlap sub-lane spills are display-only + * and never a lane-move target), so any occupant answers. A lane with no other + * occupant falls back to the lane value itself — for already-contiguous files the + * two spaces coincide, and edge-created lanes (min-1 / max+1) route through the + * insert path, never here. + */ +function authoredTrackForLane( + lane: number, + elements: TimelineElement[], + excludeKey: string, +): number { + const occupant = elements.find((e) => e.track === lane && keyOf(e) !== excludeKey); + return occupant ? (occupant.authoredTrack ?? occupant.track) : lane; +} + function insertTrackValue(trackOrder: number[], insertRow: number): number { if (trackOrder.length === 0) return 0; if (insertRow <= 0) return trackOrder[0] - 0.5; @@ -251,6 +283,7 @@ export function commitDraggedClipMove(drag: DraggedClipState, deps: DragCommitDe const dragEdit: TimelineMoveEdit = { element: drag.element, updates: { start: drag.previewStart, track: drag.previewTrack }, + persistTrack: authoredTrackForLane(drag.previewTrack, elements, dragKey), }; const coalesceKey = isVertical ? `clip-lane-move:${laneChangeGestureSeq++}` : undefined; @@ -265,9 +298,13 @@ export function commitDraggedClipMove(drag: DraggedClipState, deps: DragCommitDe // The drop-intent set for the z-sync: the dragged clip at its new lane, others // as-is. Reasoning on this (not a re-normalize) keeps the sync seeing the user's // move; computeStackingPatches only compares lanes relatively. - const candidate = elements.map((e) => - keyOf(e) === dragKey ? { ...e, start: drag.previewStart, track: drag.previewTrack } : e, - ); + const candidate = elements.map((e) => { + if (keyOf(e) === dragKey) return { ...e, start: drag.previewStart, track: drag.previewTrack }; + // Selection members shift in time with the drag — the z-sync must reason on + // their POST-move overlap sets, same as the insert branch's candidate. + if (multi?.keys.has(keyOf(e))) return { ...e, start: multi.movedStart(e) }; + return e; + }); const multiKeys = multi ? multi.keys : null; void persistMoveEdits(edits, deps, coalesceKey, "lane-reorder").then((moved) => { if (moved && isVertical) { @@ -292,6 +329,7 @@ export function commitDraggedClipMove(drag: DraggedClipState, deps: DragCommitDe * The whole affected set is persisted atomically (single undo), and the deliberate * vertical move syncs the dragged clip's stacking afterwards. */ +// fallow-ignore-next-line complexity function commitTrackInsert( drag: DraggedClipState, deps: DragCommitDeps, @@ -314,12 +352,25 @@ function commitTrackInsert( // shifts the at/below clips down by one — the sanctioned +1 index renumber. const normalized = normalizeToZones(candidate); const bySrc = new Map(elements.map((e) => [keyOf(e), e])); + // The renumber is only correct as a WHOLE-SET write: skipping an unwritable + // clip whose lane shifts leaves its track colliding with a renumbered + // neighbour, and the next normalize merges the two lanes. If any shifted clip + // can't be written, refuse the insert instead of persisting a broken layout. + for (const norm of normalized) { + const src = bySrc.get(keyOf(norm)); + if (src && !canMoveElement(src) && norm.track !== src.track) { + console.warn( + `[Timeline] Track insert refused: locked clip ${keyOf(src)} would need renumbering`, + ); + return; + } + } const edits: TimelineMoveEdit[] = []; for (const norm of normalized) { const src = bySrc.get(keyOf(norm)); if (!src) continue; - // Capabilities gate: never write a locked/implicit clip, even one only swept - // along by the renumber (not just a marquee member). + // Capabilities gate (unchanged-lane clips only reach here now): never write + // a locked/implicit clip. if (!canMoveElement(src)) continue; const start = keyOf(norm) === dragKey || multi?.keys.has(keyOf(norm)) @@ -391,6 +442,7 @@ function syncStackingForEdit( zIndex: readZIndex(el), isAudio: classifyZone(el) === "audio", domIndex, + stackingContextId: el.stackingContextId ?? null, })); const editedKeys = [dragKey]; diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index ce7cfbbea..b5b08e6a3 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -67,51 +67,105 @@ export const DRAG_EXTEND_MARGIN_PX = 160; * with 60s of ruler after it. */ export const MIN_TIMELINE_EXTENT_S = 60; +/** + * Fit-mode headroom (CapCut-style): "fit" maps `duration * 1.2` — not the bare + * duration — onto the viewport, so the composition ends at ~83% of the width + * and the trailing ~17% stays empty ruler + droppable lane surface (room to + * drag clips past the current end without first zooming out). Applied ONLY + * inside {@link getTimelineFitPps}, the single fit-pps source, so the ruler, + * lanes, playhead, marquee, and drag math all inherit it consistently. Manual + * zoom percentages stay defined relative to this fit basis (100% == fit). + */ +export const FIT_ZOOM_HEADROOM = 1.2; /* ── Tick generation ──────────────────────────────────────────────── */ -function getMajorTickInterval(duration: number, pixelsPerSecond?: number): number { - const zoomIntervals = [0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 15, 30, 60, 120, 300, 600]; +// fallow-ignore-next-line complexity +function getMajorTickInterval( + duration: number, + pixelsPerSecond?: number, + frameRate?: number, +): number { + // "Nice" NLE steps: 1-2-5 sub-second decades, then 1s/2s/5s/10s/15s/30s, + // minute multiples, and 15m/30m/1h so ultra-zoomed-out long comps still get + // readable (non-colliding) labels instead of the old 10m fallback everywhere. + const zoomIntervals = [ + 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 15, 30, 60, 120, 300, 600, 900, 1800, 3600, + ]; + let interval: number; if (Number.isFinite(pixelsPerSecond) && (pixelsPerSecond ?? 0) > 0) { const targetMajorPx = 88; - return ( - zoomIntervals.find((interval) => interval * (pixelsPerSecond ?? 0) >= targetMajorPx) ?? 600 - ); + interval = + zoomIntervals.find((candidate) => candidate * (pixelsPerSecond ?? 0) >= targetMajorPx) ?? + 3600; + } else { + const durationIntervals = [0.25, 0.5, 1, 2, 5, 10, 15, 30, 60]; + const target = duration / 6; + interval = durationIntervals.find((candidate) => candidate >= target) ?? 60; } - const durationIntervals = [0.25, 0.5, 1, 2, 5, 10, 15, 30, 60]; - const target = duration / 6; - return durationIntervals.find((interval) => interval >= target) ?? 60; + // Frame display mode: labels are frame numbers, so a major step must be a + // WHOLE number of frames — sub-frame steps produce duplicate/uneven labels + // (e.g. 0.02s at 30fps is 0.6 frames → "0, 1, 1, 2, 2…"). Snap UP (ceil) so + // the label spacing never drops below the readability target. + if (Number.isFinite(frameRate) && (frameRate ?? 0) > 0) { + const fps = frameRate ?? 0; + return Math.max(1, Math.ceil(interval * fps - 1e-6)) / fps; + } + return interval; } // How many equal parts to split each major interval into for minor ticks. Prefer // quarters (4) so the midpoint stays a minor tick; fall back to halves (2) then -// none (0) as ticks get too dense to read (< ~8px apart). -function getMinorSubdivisions(majorInterval: number, pixelsPerSecond?: number): number { +// none (0) as ticks get too dense to read (< ~8px apart). In frame display mode +// the subdivision must also keep minor ticks on WHOLE frames (a minor tick at a +// sub-frame time is not a seekable position), so only divisors of the major +// step's frame count qualify — quarters, then fifths (15/30-frame majors), +// thirds, halves. +// fallow-ignore-next-line complexity +function getMinorSubdivisions( + majorInterval: number, + pixelsPerSecond?: number, + frameRate?: number, +): number { const pps = Number.isFinite(pixelsPerSecond) ? (pixelsPerSecond ?? 0) : 0; if (pps <= 0) return 4; // no zoom info (duration-fit mode): quarter ticks - if ((majorInterval / 4) * pps >= 8) return 4; - if ((majorInterval / 2) * pps >= 8) return 2; + const fps = Number.isFinite(frameRate) ? (frameRate ?? 0) : 0; + const majorFrames = fps > 0 ? Math.round(majorInterval * fps) : 0; + const candidates = fps > 0 ? [4, 5, 3, 2] : [4, 2]; + for (const parts of candidates) { + if (fps > 0 && majorFrames % parts !== 0) continue; + if ((majorInterval / parts) * pps >= 8) return parts; + } return 0; } +// Ticks are exact multiples of the interval (multiplied per index, never +// accumulated with `+=`, so long rulers don't drift), then rounded to 1µs to +// keep values/keys clean without disturbing frame-exact positions like 2/30s. +function roundTickValue(t: number): number { + return Math.round(t * 1e6) / 1e6; +} + export function generateTicks( duration: number, pixelsPerSecond?: number, + frameRate?: number, ): { major: number[]; minor: number[] } { - if (duration <= 0 || !Number.isFinite(duration) || duration > 7200) + if (duration <= 0 || !Number.isFinite(duration) || duration > 14400) return { major: [], minor: [] }; - const majorInterval = getMajorTickInterval(duration, pixelsPerSecond); - const subdivisions = getMinorSubdivisions(majorInterval, pixelsPerSecond); + const majorInterval = getMajorTickInterval(duration, pixelsPerSecond, frameRate); + const subdivisions = getMinorSubdivisions(majorInterval, pixelsPerSecond, frameRate); const minorInterval = subdivisions > 0 ? majorInterval / subdivisions : 0; const major: number[] = []; const minor: number[] = []; const maxTicks = 2000; // Safety cap to prevent runaway tick generation - for (let t = 0; t <= duration + 0.001 && major.length < maxTicks; t += majorInterval) { - const rounded = Math.round(t * 100) / 100; - major.push(rounded); + for (let i = 0; major.length < maxTicks; i++) { + const t = i * majorInterval; + if (t > duration + 0.001) break; + major.push(roundTickValue(t)); // Emit the (subdivisions - 1) minor ticks between this major and the next. for (let k = 1; k < subdivisions && major.length + minor.length < maxTicks; k++) { - const m = Math.round((t + k * minorInterval) * 100) / 100; - if (m <= duration + 0.001) minor.push(m); + const m = t + k * minorInterval; + if (m <= duration + 0.001) minor.push(roundTickValue(m)); } } return { major, minor }; @@ -144,15 +198,17 @@ export function formatTimelineTickLabel(time: number, duration: number, majorInt /* ── Width / duration derivation ──────────────────────────────────── */ /** - * Fit-mode pixels-per-second: fill the viewport with the composition, but - * never map fewer than MIN_TIMELINE_EXTENT_S seconds onto it — a short comp - * takes a fraction of the width and the remaining ruler runs to 1:00. + * Fit-mode pixels-per-second: fill the viewport with the composition plus + * FIT_ZOOM_HEADROOM trailing headroom (CapCut-style — the comp never slams + * into the right edge), and never map fewer than MIN_TIMELINE_EXTENT_S + * seconds onto it — a short comp takes a fraction of the width and the + * remaining ruler runs to 1:00. * Manual zoom multiplies this base, so the floor only anchors the default. */ export function getTimelineFitPps(viewportWidth: number, effectiveDuration: number): number { const safeDuration = Number.isFinite(effectiveDuration) && effectiveDuration > 0 ? effectiveDuration : 0; - const span = Math.max(safeDuration, MIN_TIMELINE_EXTENT_S); + const span = Math.max(safeDuration * FIT_ZOOM_HEADROOM, MIN_TIMELINE_EXTENT_S); if (!Number.isFinite(viewportWidth) || viewportWidth <= GUTTER) return 100; return (viewportWidth - GUTTER - 2) / span; } @@ -227,9 +283,26 @@ export function getTimelineScrollLeftForZoomAnchor(input: { } /* ── Playhead / canvas ────────────────────────────────────────────── */ +/** + * Width of the playhead wrapper element (== the diamond head chip's layout + * width, which the wrapper shrink-wraps to). The 1px vertical line inside + * PlayheadIndicator is centered at 50% of this wrapper, so the wrapper must be + * shifted LEFT by half this width for the line's center to land exactly on + * `GUTTER + time * pps` — see {@link getTimelinePlayheadLeft}. + */ +export const PLAYHEAD_HEAD_W = 9; + +/** + * The `left` for the playhead WRAPPER such that the vertical line's CENTER + * sits exactly on `GUTTER + time * pps` (the same x the ruler ticks center + * on) at every zoom level. Without the half-head offset the line sat + * `PLAYHEAD_HEAD_W / 2` px to the right of its ruler tick. + */ export function getTimelinePlayheadLeft(time: number, pixelsPerSecond: number): number { - if (!Number.isFinite(time) || !Number.isFinite(pixelsPerSecond)) return GUTTER; - return GUTTER + Math.max(0, time) * Math.max(0, pixelsPerSecond); + if (!Number.isFinite(time) || !Number.isFinite(pixelsPerSecond)) { + return GUTTER - PLAYHEAD_HEAD_W / 2; + } + return GUTTER + Math.max(0, time) * Math.max(0, pixelsPerSecond) - PLAYHEAD_HEAD_W / 2; } export function getTimelineCanvasHeight(trackCount: number): number { diff --git a/packages/studio/src/player/components/timelineStackingSync.test.ts b/packages/studio/src/player/components/timelineStackingSync.test.ts index 925787e77..599bc3502 100644 --- a/packages/studio/src/player/components/timelineStackingSync.test.ts +++ b/packages/studio/src/player/components/timelineStackingSync.test.ts @@ -19,6 +19,66 @@ function patchMap(elements: StackingElement[], edited: string[]): Record { + it("never compares or patches across stacking contexts", () => { + // X lives in sub-comp context "scene-1" with a high leaf z; Y is a root clip + // with a lower leaf z, overlapping in time. Their leaf z values are NOT + // comparable (the ancestors' z decides paint order), so moving X's lane above + // Y must not reason on Y or patch either based on the 10-vs-5 comparison. + const x: StackingElement = { + key: "x", + track: 0, + start: 0, + duration: 5, + zIndex: 10, + isAudio: false, + stackingContextId: "scene-1", + }; + const y: StackingElement = { + key: "y", + track: 1, + start: 0, + duration: 5, + zIndex: 5, + isAudio: false, + stackingContextId: null, + }; + // X edited: only same-context neighbours participate — none here, so X keeps + // its z (nothing to fix WITHIN its context) and Y is never touched. + expect(patchMap([x, y], ["x"])).toEqual({}); + }); + + it("still resolves within the edited clip's own context", () => { + const a: StackingElement = { + key: "a", + track: 1, + start: 0, + duration: 5, + zIndex: 1, + isAudio: false, + stackingContextId: "scene-1", + }; + const b: StackingElement = { + key: "b", + track: 0, + start: 0, + duration: 5, + zIndex: 5, + isAudio: false, + stackingContextId: "scene-1", + }; + // 'a' moved BELOW b's lane... a (track 1) is under b (track 0): a's z (1) + // is already below b's (5) — consistent, no patch. Move a ABOVE (track -1 + // relative ordering) is covered by existing suites; here we just prove the + // same-context pair still participates (no patch ≠ no participation: verify + // by flipping z so a MUST be lifted). + const aWrong = { ...a, track: 0, zIndex: 1 }; + const bLow = { ...b, track: 1, zIndex: 5 }; + const patches = patchMap([aWrong, bLow], ["a"]); + expect(patches.a).toBeGreaterThan(5); + }); +}); + describe("laneIsAbove", () => { it("lower track renders above (top of timeline wins)", () => { expect(laneIsAbove({ track: 0 }, { track: 1 })).toBe(true); diff --git a/packages/studio/src/player/components/timelineStackingSync.ts b/packages/studio/src/player/components/timelineStackingSync.ts index 4362879be..2cc22ea1c 100644 --- a/packages/studio/src/player/components/timelineStackingSync.ts +++ b/packages/studio/src/player/components/timelineStackingSync.ts @@ -43,6 +43,13 @@ export interface StackingElement { zIndex: number; /** Audio clips have no visual stacking and are excluded from the computation. */ isAudio: boolean; + /** + * CSS stacking context the clip's node lives in (TimelineElement.stackingContextId). + * Leaf z-indexes are only comparable WITHIN one context — across contexts the + * ancestors' z decides paint order — so the sync partitions by this key and + * never patches across contexts. Null/undefined ⇒ the root context. + */ + stackingContextId?: string | null; /** * Discovery / DOM document position (optional). Two clips with EQUAL z paint by * DOM order — the one LATER in the DOM paints ON TOP. When supplied, "is A above @@ -284,7 +291,16 @@ export function computeStackingPatches( // z=0 would enter the boundary math as a phantom neighbour at the z-floor. An // unresolved clip is neither a neighbour nor resolvable as an edit, so it is // excluded outright (item 13). - const resolved = elements.filter((e) => Number.isFinite(e.zIndex)); + const allResolved = elements.filter((e) => Number.isFinite(e.zIndex)); + + // Leaf z is only meaningful within ONE stacking context: across contexts the + // ancestor contexts' z decides paint order, so comparing (or patching) leaf + // values across contexts is nonsense. Restrict the computation to the edited + // clips' own context(s); cross-context lane relations are out of scope. + const editedContexts = new Set( + allResolved.filter((e) => editedSet.has(e.key)).map((e) => e.stackingContextId ?? null), + ); + const resolved = allResolved.filter((e) => editedContexts.has(e.stackingContextId ?? null)); // Mutable z snapshot so edits + cascaded bumps see each other's applied z. const byKey = new Map(resolved.map((e) => [e.key, { ...e }])); @@ -304,8 +320,12 @@ export function computeStackingPatches( // The full live set, so the transitive cascade can reach clips that overlap a // LIFTED neighbour without overlapping the edited clip itself (#2198). const all = [...byKey.values()]; + const sameContext = (a: MutZ, b: MutZ) => + (a.stackingContextId ?? null) === (b.stackingContextId ?? null); const overlappersOf = (clip: MutZ): MutZ[] => - all.filter((o) => o.key !== clip.key && !o.isAudio && overlapsInTime(clip, o)); + all.filter( + (o) => o.key !== clip.key && !o.isAudio && sameContext(clip, o) && overlapsInTime(clip, o), + ); for (const clip of edited) { resolveEditedZ(clip, overlappersOf(clip), overlappersOf, patchZ); diff --git a/packages/studio/src/player/components/timelineZones.ts b/packages/studio/src/player/components/timelineZones.ts index 515555b3e..5cc2d8af9 100644 --- a/packages/studio/src/player/components/timelineZones.ts +++ b/packages/studio/src/player/components/timelineZones.ts @@ -128,7 +128,10 @@ export function normalizeToZones(elements: TimelineElement[]): TimelineElement[] const lane = laneOf.get(keyOf(el)); if (lane == null || lane === el.track) return el; changed = true; - return { ...el, track: lane }; + // Record the source-file track the first time a clip is remapped so lane + // edits can persist in AUTHORED space (see TimelineElement.authoredTrack). + // Re-normalizing already-remapped elements must keep the original value. + return { ...el, track: lane, authoredTrack: el.authoredTrack ?? el.track }; }); return changed ? remapped : elements; } diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index 8a7cdcff0..7cd76f610 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -29,6 +29,13 @@ export interface TimelineElement { start: number; duration: number; track: number; + /** + * The data-track-index as written in the source file, when it differs from + * the display lane in `track` (normalizeToZones packs sparse authored tracks + * onto contiguous display lanes). Lane edits must persist THIS space — writing + * a display-lane number into a sparse file re-targets the wrong track. + */ + authoredTrack?: number; /** Resolved z-index for stacking-aware timeline ordering. */ zIndex?: number; /** True when the effective z-index was authored inline or through CSS, not auto. */