diff --git a/packages/studio/src/hooks/useRazorSplit.test.ts b/packages/studio/src/hooks/useRazorSplit.test.ts index 5d230fabb2..73f05109c4 100644 --- a/packages/studio/src/hooks/useRazorSplit.test.ts +++ b/packages/studio/src/hooks/useRazorSplit.test.ts @@ -9,6 +9,7 @@ import { createPersistentEditHistoryStore } from "./usePersistentEditHistory"; import { createEmptyEditHistory } from "../utils/editHistory"; import type { EditHistoryStorageAdapter } from "../utils/editHistoryStorage"; import { createSplitFetchMock, mountProbe } from "./useRazorSplit.testHelpers"; +import type { PlacementFold } from "../player/components/timelinePlacementCommit"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -49,11 +50,19 @@ interface SplitRequest { type SingleSplit = (element: TimelineElement, splitTime: number) => Promise; type SplitAll = (splitTime: number) => Promise; +type PlacementSplit = ( + element: TimelineElement, + splitTime: number, + fold: PlacementFold, +) => Promise; interface Harness { splitRequests: SplitRequest[]; singleRef: { current: SingleSplit | undefined }; allRef: { current: SplitAll | undefined }; + placementRef: { current: PlacementSplit | undefined }; + reloadPreview: ReturnType; + forceReloadSdkSession: ReturnType; root: ReturnType; } @@ -76,9 +85,12 @@ function mountRazorSplit(): Harness { const singleRef: { current: SingleSplit | undefined } = { current: undefined }; const allRef: { current: SplitAll | undefined } = { current: undefined }; + const placementRef: { current: PlacementSplit | undefined } = { current: undefined }; + const reloadPreview = vi.fn(); + const forceReloadSdkSession = vi.fn(); function Component() { - const { handleRazorSplit, handleRazorSplitAll } = useRazorSplit({ + const { handleRazorSplit, handleRazorSplitAll, handlePlacementSplit } = useRazorSplit({ projectId: "p1", activeCompPath: ROOT_FILE, showToast: () => {}, @@ -86,15 +98,25 @@ function mountRazorSplit(): Harness { disk[path] = content; }, recordEdit: async () => {}, - reloadPreview: () => {}, + reloadPreview, + forceReloadSdkSession, }); singleRef.current = handleRazorSplit; allRef.current = handleRazorSplitAll; + placementRef.current = handlePlacementSplit; return null; } const root = mountProbe(Component); - return { splitRequests, singleRef, allRef, root }; + return { + splitRequests, + singleRef, + allRef, + placementRef, + reloadPreview, + forceReloadSdkSession, + root, + }; } afterEach(() => { @@ -162,6 +184,37 @@ describe("useRazorSplit — sub-comp coordinate rebasing", () => { }); }); +// A drop reloads the preview once, at its end; an interior split must not reload on its own. +describe("useRazorSplit — preview reload folded into a drop", () => { + let harness: Harness; + beforeEach(() => { + harness = mountRazorSplit(); + }); + afterEach(() => { + act(() => harness.root.unmount()); + }); + + it("skips reloading the preview for a split folded into a drop, but still refreshes the sdk session", async () => { + await act(async () => { + await harness.placementRef.current!(rootElement, 4, { + coalesceKey: "clip-overwrite:1", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + + expect(harness.reloadPreview).not.toHaveBeenCalled(); + expect(harness.forceReloadSdkSession).toHaveBeenCalledTimes(1); + }); + + it("still reloads the preview for a plain (non-drop) split", async () => { + await act(async () => { + await harness.singleRef.current!(rootElement, 4); + }); + + expect(harness.reloadPreview).toHaveBeenCalledTimes(1); + }); +}); + // ── Bug 1: split must resync the SDK session so undo isn't refused ──────────── const memoryStorage = (): EditHistoryStorageAdapter => { diff --git a/packages/studio/src/hooks/useRazorSplit.ts b/packages/studio/src/hooks/useRazorSplit.ts index 9bd1fe902f..ba1b41d67b 100644 --- a/packages/studio/src/hooks/useRazorSplit.ts +++ b/packages/studio/src/hooks/useRazorSplit.ts @@ -36,20 +36,28 @@ export function useRazorSplit({ const projectIdRef = useRef(projectId); projectIdRef.current = projectId; - const synchronize = useCallback(() => { - let failure: unknown; - try { - forceReloadSdkSession?.(); - } catch (error) { - failure = error; - } - try { - reloadPreview(); - } catch (error) { - failure ??= error; - } - if (failure) throw failure; - }, [forceReloadSdkSession, reloadPreview]); + // skipPreviewReload: true when this cut is one step folded into a drop, whose + // own single reload (after every step lands) replaces this one — otherwise the + // preview would show this step's DOM before the next step makes it stale. + const synchronize = useCallback( + (skipPreviewReload: boolean) => { + let failure: unknown; + try { + forceReloadSdkSession?.(); + } catch (error) { + failure = error; + } + if (!skipPreviewReload) { + try { + reloadPreview(); + } catch (error) { + failure ??= error; + } + } + if (failure) throw failure; + }, + [forceReloadSdkSession, reloadPreview], + ); const runCut = useCallback( async ( @@ -75,7 +83,7 @@ export function useRazorSplit({ writeProjectFile, recordEdit, observeProjectFileVersion, - synchronize, + synchronize: () => synchronize(Boolean(fold)), }); trackStudioRazorSplit({ mode, count: result.splitCount }); if (result.syncFailed) { diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index 16c6f3c922..e71b974875 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -59,6 +59,8 @@ describe("useTimelineDeleteOps: ripple undo label", () => { handleTimelineGroupMove: DeleteOpsOptions["handleTimelineGroupMove"]; showToast?: DeleteOpsOptions["showToast"]; recordEdit?: DeleteOpsOptions["recordEdit"]; + reloadPreview?: DeleteOpsOptions["reloadPreview"]; + forceReloadSdkSession?: DeleteOpsOptions["forceReloadSdkSession"]; }) { const elements = [el("hf-a", 0, 2), el("hf-b", 2, 2), el("hf-c", 4, 2)]; let hook: ReturnType | null = null; @@ -70,7 +72,8 @@ describe("useTimelineDeleteOps: ripple undo label", () => { showToast: overrides.showToast ?? vi.fn(), writeProjectFile: vi.fn().mockResolvedValue(undefined), recordEdit: overrides.recordEdit ?? vi.fn().mockResolvedValue(undefined), - reloadPreview: vi.fn(), + reloadPreview: overrides.reloadPreview ?? vi.fn(), + forceReloadSdkSession: overrides.forceReloadSdkSession, previewIframeRef: { current: null }, handleTimelineGroupMove: overrides.handleTimelineGroupMove, }); @@ -145,4 +148,38 @@ describe("useTimelineDeleteOps: ripple undo label", () => { expect.objectContaining({ coalesceKey: "clip-overwrite:3", coalesceMs: Infinity }), ); }); + + // A drop reloads the preview once, at its end; an interior remove must not reload on its own. + it("skips reloading the preview for an overwrite delete, but still refreshes the sdk session", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const reloadPreview = vi.fn(); + const forceReloadSdkSession = vi.fn(); + const { b, getHook } = mountDeleteHarness({ + handleTimelineGroupMove, + reloadPreview, + forceReloadSdkSession, + }); + + await act(async () => { + await getHook().deleteTimelineElements([b], { + coalesceKey: "clip-overwrite:4", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(forceReloadSdkSession).toHaveBeenCalledTimes(1); + }); + + it("still reloads the preview for a plain (non-drop) delete", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const reloadPreview = vi.fn(); + const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove, reloadPreview }); + + await act(async () => { + await getHook().handleTimelineElementDelete(b); + }); + + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index a1d2178bab..9b015611fa 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -223,7 +223,10 @@ export function useTimelineDeleteOps({ usePlayerStore.getState().setSelectedElementIds(new Set()); } forceReloadSdkSession?.(); - reloadPreview(); + // Folded into a drop: the drop's own single reload (after every step + // lands) replaces this one, so an interior remove never shows the + // preview a state the next step is about to make stale. + if (!overwrite) reloadPreview(); // A failed ripple already showed its own toast above; the user did one // thing (delete), so they get one message, not this generic follow-up too. if (!rippleFailed && !overwrite) { diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index cba4f45eb6..063291f2ef 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -452,8 +452,9 @@ export function useTimelineEditing({ split: handlePlacementSplit, remove: deleteTimelineElements, toast: (message) => showToast(message, "error"), + reloadPreview, }), - [handlePlacementSplit, deleteTimelineElements, showToast], + [handlePlacementSplit, deleteTimelineElements, showToast, reloadPreview], ); return { diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index 6b7f615e60..71dc85e770 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -117,6 +117,8 @@ export function persistMoveEdits( coalesceKey?: string, operation: TimelineMoveOperation = "timing", coalesceMs?: number, + /** False when the caller already wrote the end state to the store (a placement drop does). */ + updateStore = true, ): Promise { if (edits.length === 0) return Promise.resolve(true); const { updateElement, onMoveElement, onMoveElements } = deps; @@ -150,7 +152,7 @@ export function persistMoveEdits( writtenTrack == null ? e.updates : { ...e.updates, authoredTrack: writtenTrack }, ); }; - for (const e of edits) applyEdit(e); + if (updateStore) for (const e of edits) applyEdit(e); // 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) => @@ -167,6 +169,7 @@ export function persistMoveEdits( // restore the preview manifest's pre-gesture lane. Reassert the durable // result after persistence, but only while this remains the latest // optimistic gesture so an older save can never clobber a newer drag. + if (!updateStore) return true; for (const e of edits) { const key = keyOf(e.element); if (isLatestTimelineOptimisticGesture(updateElement, revision, key)) applyEdit(e); @@ -174,9 +177,15 @@ export function persistMoveEdits( return true; }, (error) => { - for (const p of prev) { - if (isLatestTimelineOptimisticGesture(updateElement, revision, p.key)) { - updateElement(p.key, { start: p.start, track: p.track, authoredTrack: p.authoredTrack }); + if (updateStore) { + for (const p of prev) { + if (isLatestTimelineOptimisticGesture(updateElement, revision, p.key)) { + updateElement(p.key, { + start: p.start, + track: p.track, + authoredTrack: p.authoredTrack, + }); + } } } console.error("[Timeline] Failed to persist clip edits", error); @@ -262,6 +271,7 @@ export function commitDraggedClipMove(rawDrag: DraggedClipState, deps: DragCommi fold.coalesceKey, edits.some((e) => e.updates.track !== e.element.track) ? "lane-reorder" : "timing", fold.coalesceMs, + false, ), deps, ); diff --git a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts index d389ff92e9..2e7540e193 100644 --- a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts +++ b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts @@ -189,6 +189,7 @@ describe("timeline clip drag gesture lifecycle", () => { split: vi.fn(async () => true), remove: vi.fn(async () => true), toast: vi.fn(), + reloadPreview: vi.fn(), }, }, }); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 6ac4207c85..c6ad394cf7 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -1,29 +1,13 @@ import { describe, expect, it, vi } from "vitest"; -import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; -import type { TimelineElement } from "../store/playerStore"; -import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; -import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { TimelineMoveEdit } from "./timelineClipDragCommit"; import type { PlaceClipResult } from "./timelinePlacement"; import { buildPlacementSteps, commitPlacementDrop, placementRefusal, runPlacementSteps, - type PlacementFold, - type PlacementOps, } from "./timelinePlacementCommit"; -import { - buildEditHistoryEntry, - createEmptyEditHistory, - hashEditHistoryContent, - pushEditHistoryEntry, - undoEditHistory, - type EditHistoryState, -} from "../../utils/editHistory"; - -function clip(id: string, start: number, duration: number, extra: Partial = {}) { - return { id, domId: id, tag: "video", start, duration, track: 1, ...extra } as TimelineElement; -} +import { clip, createFakeProject, depsOf, dragOf, type Doc } from "./timelinePlacementTestHarness"; const dragged = clip("d", 20, 2); const draggedEdit = (start: number): TimelineMoveEdit => ({ @@ -223,144 +207,6 @@ describe("placementRefusal", () => { }); }); -interface Doc { - [id: string]: { start: number; duration: number; playbackStart?: number }; -} - -/** A tiny stand-in for the project file plus the history recorder, driven by the real reducer. */ -function createFakeProject(initial: Doc) { - let doc: Doc = structuredClone(initial); - let history: EditHistoryState = createEmptyEditHistory(); - let clock = 0; - const serialize = (d: Doc) => - JSON.stringify(Object.entries(d).sort(([x], [y]) => (x < y ? -1 : 1))); - const record = (mutate: () => void, label: string, fold: PlacementFold) => { - const before = serialize(doc); - mutate(); - history = pushEditHistoryEntry( - history, - buildEditHistoryEntry({ - id: `e${clock}`, - projectId: "p", - label, - kind: "timeline", - coalesceKey: fold.coalesceKey, - coalesceMs: fold.coalesceMs, - now: (clock += 1000), - files: { "index.html": { before, after: serialize(doc) } }, - }), - ); - }; - const current = (element: TimelineElement) => { - const found = doc[element.id]; - if (!found) throw new Error(`${element.id} is not in the document`); - // The element a step hands over must describe what the document holds right now. - expect({ start: element.start, duration: element.duration }).toEqual({ - start: found.start, - duration: found.duration, - }); - return found; - }; - const ops: PlacementOps = { - split: async (element, at, fold) => { - record( - () => { - const target = current(element); - const end = target.start + target.duration; - doc[`${element.id}-split`] = { - start: at, - duration: end - at, - playbackStart: (target.playbackStart ?? 0) + (at - target.start), - }; - target.duration = at - target.start; - }, - "split", - fold, - ); - return true; - }, - remove: async (elements, fold) => { - record( - () => { - for (const element of elements) { - current(element); - delete doc[element.id]; - } - }, - "delete", - fold, - ); - return true; - }, - toast: vi.fn(), - }; - const resize = async (changes: TimelineGroupResizeChange[], fold: PlacementFold) => { - record( - () => { - for (const change of changes) { - const target = current(change.element); - target.start = change.start; - target.duration = change.duration; - if (change.playbackStart != null) target.playbackStart = change.playbackStart; - } - }, - "resize", - fold, - ); - }; - const move = async (edits: TimelineMoveEdit[], fold: PlacementFold) => { - record( - () => { - for (const edit of edits) { - const found = doc[edit.element.id]; - if (found) found.start = edit.updates.start; - else - doc[edit.element.id] = { start: edit.updates.start, duration: edit.element.duration }; - } - }, - "move", - fold, - ); - return true; - }; - return { - ops, - resize, - move, - doc: () => doc, - history: () => history, - undo: () => { - const undone = undoEditHistory( - history, - { "index.html": hashEditHistoryContent(serialize(doc)) }, - 0, - ); - if (!undone.ok) throw new Error(`undo refused: ${undone.reason}`); - return undone.filesToWrite["index.html"]; - }, - serializeInitial: () => serialize(initial), - }; -} - -function dragOf(element: TimelineElement, previewStart: number, previewTrack = 1) { - return { - element, - previewStart, - previewTrack, - insertRow: null, - } as unknown as DraggedClipState; -} - -function depsOf(elements: TimelineElement[], project: ReturnType) { - return { - elements, - trackOrder: [1], - updateElement: vi.fn(), - placementOps: project.ops, - onResizeElements: project.resize, - } as unknown as DragCommitDeps; -} - describe("commitPlacementDrop: one undo step for the move and every cut", () => { // Lane 1: a [0,4) and b [4,8) video (b reads its source from 1s at rate 1), dragged d [20,22). const a = clip("a", 0, 4); @@ -487,9 +333,10 @@ describe("runPlacementSteps failures", () => { it("names a partial apply and stops when a later step fails", async () => { const toast = vi.fn(); await runPlacementSteps(steps, { - ops: { split: async () => true, remove: async () => true, toast }, + ops: { split: async () => true, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move: async () => false, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); }); @@ -498,9 +345,10 @@ describe("runPlacementSteps failures", () => { const toast = vi.fn(); const move = vi.fn(async () => true); await runPlacementSteps(steps, { - ops: { split: async () => false, remove: async () => true, toast }, + ops: { split: async () => false, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite failed, nothing changed"); expect(move).not.toHaveBeenCalled(); @@ -510,12 +358,95 @@ describe("runPlacementSteps failures", () => { const toast = vi.fn(); vi.spyOn(console, "error").mockImplementation(() => undefined); await runPlacementSteps(steps, { - ops: { split: async () => true, remove: async () => true, toast }, + ops: { split: async () => true, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move: async () => { throw new Error("write failed"); }, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); }); }); + +// A drop reloads the preview exactly once, after every step lands, never once per step. +describe("runPlacementSteps: one reload per drop, after every step lands", () => { + it("reloads exactly once, after the move step, when the drop contains a remove", async () => { + const calls: string[] = []; + const reloadPreview = vi.fn(() => calls.push("reload")); + const remove = vi.fn(async () => { + calls.push("remove"); + return true; + }); + const move = vi.fn(async () => { + calls.push("move"); + return true; + }); + await runPlacementSteps( + [ + { kind: "remove", elements: [clip("a", 0, 4)] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move, + applyToStore: vi.fn(), + }, + ); + expect(calls).toEqual(["remove", "move", "reload"]); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("reloads exactly once when the drop contains a split", async () => { + const reloadPreview = vi.fn(); + const split = vi.fn(async () => true); + await runPlacementSteps( + [ + { kind: "split", element: clip("a", 0, 4), at: 2 }, + { kind: "move", edits: [] }, + ], + { + ops: { split, remove: async () => true, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("never reloads a move/resize-only drop (a plain trim stays flash-free)", async () => { + const reloadPreview = vi.fn(); + await runPlacementSteps( + [ + { kind: "resize", changes: [] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove: async () => true, toast: vi.fn(), reloadPreview }, + resize: vi.fn(async () => undefined), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("reloads once when a step failed, because the store was already told the end state", async () => { + const reloadPreview = vi.fn(); + await runPlacementSteps( + [ + { kind: "remove", elements: [clip("a", 0, 4)] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove: async () => false, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index 05e4146eaa..f3a51300df 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -1,4 +1,5 @@ -import type { TimelineElement } from "../store/playerStore"; +import { usePlayerStore, type TimelineElement } from "../store/playerStore"; +import { holdTimelineManifests } from "../lib/timelineManifestHold"; import type { DraggedClipState } from "./timelineClipDragTypes"; import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; import { canMoveTimelineElement } from "./timelineAuthoredMoveTarget"; @@ -21,6 +22,8 @@ export interface PlacementOps { split: (element: TimelineElement, at: number, fold: PlacementFold) => Promise; remove: (elements: TimelineElement[], fold: PlacementFold) => Promise; toast: (message: string) => void; + /** The one full-reload point for a drop; split/remove skip their own while folded. */ + reloadPreview: () => void; } export type PlacementStep = @@ -148,6 +151,8 @@ interface PlacementRunner { ops: PlacementOps; resize: (changes: TimelineGroupResizeChange[], fold: PlacementFold) => Promise | void; move: (edits: TimelineMoveEdit[], fold: PlacementFold) => Promise; + /** Puts the drop's end state in the store before any write starts. */ + applyToStore: (steps: readonly PlacementStep[]) => void; } function runStep(step: PlacementStep, fold: PlacementFold, run: PlacementRunner) { @@ -163,7 +168,87 @@ function runStep(step: PlacementStep, fold: PlacementFold, run: PlacementRunner) } } -/** Each step awaits the previous: the history fold needs every write to start from the last one's output. */ +/** The clip a split will add, under a stand-in id until the reload reports the real one. */ +function pendingSplitTail(el: TimelineElement, at: number): TimelineElement { + const playbackStart = hasSourcePlaybackOffset(el) + ? round3((el.playbackStart ?? 0) + (at - el.start) * (el.playbackRate ?? 1)) + : el.playbackStart; + return { + ...el, + id: `${el.id}~tail`, + key: `${keyOf(el)}~tail`, + domId: undefined, + start: at, + duration: round3(el.start + el.duration - at), + playbackStart, + }; +} + +function moveUpdate(edit: TimelineMoveEdit): Partial { + const written = + edit.persistTrack ?? + (edit.updates.track !== edit.element.track ? edit.updates.track : undefined); + return written == null ? edit.updates : { ...edit.updates, authoredTrack: written }; +} + +function resizeUpdate(change: TimelineGroupResizeChange): Partial { + return { + start: change.start, + duration: change.duration, + ...(change.playbackStart != null ? { playbackStart: change.playbackStart } : {}), + }; +} + +/** What one step changes in the store: keyed updates, removed keys, and clips it adds. */ +function stepEffects(step: PlacementStep): { + updates: Array<[string, Partial]>; + removed: string[]; + added: TimelineElement[]; +} { + switch (step.kind) { + case "remove": + return { updates: [], removed: step.elements.map(keyOf), added: [] }; + case "move": + return { + updates: step.edits.map((edit) => [keyOf(edit.element), moveUpdate(edit)]), + removed: [], + added: [], + }; + case "resize": + return { + updates: step.changes.map((change) => [keyOf(change.element), resizeUpdate(change)]), + removed: [], + added: [], + }; + case "split": + return { + updates: [[keyOf(step.element), { duration: round3(step.at - step.element.start) }]], + removed: [], + added: [pendingSplitTail(step.element, step.at)], + }; + } +} + +/** The drop's end state, written to the store in one go so no in-between state is ever shown. */ +function applyPlacementToStore(steps: readonly PlacementStep[]): void { + const removed = new Set(); + const updates = new Map>(); + const tails: TimelineElement[] = []; + for (const effects of steps.map(stepEffects)) { + for (const key of effects.removed) removed.add(key); + for (const [key, next] of effects.updates) updates.set(key, { ...updates.get(key), ...next }); + tails.push(...effects.added); + } + const { elements, setElements } = usePlayerStore.getState(); + setElements([ + ...elements + .filter((el) => !removed.has(keyOf(el))) + .map((el) => ({ ...el, ...updates.get(keyOf(el)) })), + ...tails, + ]); +} + +/** Steps run in order, each from the last write; preview manifests stay held until the final reload. */ export async function runPlacementSteps( steps: readonly PlacementStep[], run: PlacementRunner, @@ -172,20 +257,32 @@ export async function runPlacementSteps( coalesceKey: `clip-overwrite:${placementGestureSeq++}`, coalesceMs: Number.POSITIVE_INFINITY, }; - for (const [index, step] of steps.entries()) { - let applied = false; - try { - applied = (await runStep(step, fold, run)) !== false; - } catch (error) { - console.error("[Timeline] Overwrite step failed", error); + const release = holdTimelineManifests(); + try { + run.applyToStore(steps); + for (const [index, step] of steps.entries()) { + let applied = false; + try { + applied = (await runStep(step, fold, run)) !== false; + } catch (error) { + console.error("[Timeline] Overwrite step failed", error); + } + if (applied) continue; + run.ops.toast( + index > 0 + ? "Overwrite partly applied, Undo restores it" + : "Overwrite failed, nothing changed", + ); + // The store was told the drop's end state; only a reload puts back what disk holds. + run.ops.reloadPreview(); + return; } - if (applied) continue; - run.ops.toast( - index > 0 - ? "Overwrite partly applied, Undo restores it" - : "Overwrite failed, nothing changed", - ); - return; + // remove/split write to disk directly and skip their own reload while folded here. + if (steps.some((step) => step.kind === "remove" || step.kind === "split")) { + run.ops.reloadPreview(); + } + } finally { + release(); } } @@ -228,5 +325,6 @@ export function commitPlacementDrop( ops: placementOps, resize: (changes, fold) => onResizeElements(changes, fold), move, + applyToStore: applyPlacementToStore, }); } diff --git a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts new file mode 100644 index 0000000000..89ffacdc56 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts @@ -0,0 +1,155 @@ +// @vitest-environment happy-dom + +import React, { act, useEffect } from "react"; +import { createRoot } from "react-dom/client"; +import { runtimeProtocolMetadata } from "@hyperframes/core/runtime/protocol"; +import { afterEach, describe, expect, it } from "vitest"; +import { useTimelinePlayer } from "../hooks/useTimelinePlayer"; +import { usePlayerStore } from "../store/playerStore"; +import { commitPlacementDrop } from "./timelinePlacementCommit"; +import { clip, createFakeProject, depsOf, dragOf } from "./timelinePlacementTestHarness"; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +function Harness() { + useTimelinePlayer(); + useEffect(() => undefined); + return null; +} + +function mountPlayer() { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render(React.createElement(Harness))); + return () => act(() => root.unmount()); +} + +const manifestClip = (id: string, start: number, duration: number) => ({ + id, + label: id, + start, + duration, + track: 1, + kind: "element", + tagName: "div", + compositionId: null, + parentCompositionId: null, + compositionSrc: null, + assetUrl: null, +}); + +// The preview iframe keeps the pre-drop DOM until the drop's one reload, so its manifest lists the +// clip the drop already removed. It used to land mid-drop and put that clip back on the timeline. +function postStalePreviewManifest() { + window.dispatchEvent( + new MessageEvent("message", { + data: { + source: "hf-preview", + type: "timeline", + clips: [manifestClip("a", 0, 4), manifestClip("b", 4, 4), manifestClip("d", 20, 6)], + durationInFrames: 900, + fps: 30, + ...runtimeProtocolMetadata(30), + }, + }), + ); +} + +function recordStoreStates( + format: (el: { id: string; start: number; duration: number }) => string, +) { + const states: string[] = []; + const stop = usePlayerStore.subscribe((state) => + states.push(state.elements.map(format).join(" ")), + ); + return { states, stop }; +} + +afterEach(() => { + document.body.innerHTML = ""; + usePlayerStore.getState().reset(); +}); + +describe("a drop's timeline states never go back to an older one", () => { + it("keeps the removed clip gone when the old preview posts its manifest mid-drop", async () => { + const unmount = mountPlayer(); + const a = clip("a", 0, 4); + const b = clip("b", 4, 4); + const covering = clip("d", 20, 6); + const preDrop = [a, b, covering]; + usePlayerStore.getState().setElements(preDrop); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4 }, + d: { start: 20, duration: 6 }, + }); + const remove = project.ops.remove; + project.ops.remove = async (elements, fold) => { + const done = await remove(elements, fold); + act(() => postStalePreviewManifest()); + return done; + }; + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}`); + + await commitPlacementDrop( + dragOf(covering, 4), + depsOf(preDrop, project), + "overwrite", + project.move, + ); + stop(); + + expect(states.length).toBeGreaterThan(0); + expect(states.every((state) => !state.includes("b@"))).toBe(true); + expect(states.at(-1)).toContain("d@4"); + unmount(); + }); + + it("shows a split's head, the dropped clip and the new tail in the first state after the drop", async () => { + const a = clip("a", 0, 4); + const b = clip("b", 4, 4, { playbackStart: 1 }); + const dropped = clip("d", 20, 2); + const lane = [a, b, dropped]; + usePlayerStore.getState().setElements(lane); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4, playbackStart: 1 }, + d: { start: 20, duration: 2 }, + }); + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); + + await commitPlacementDrop(dragOf(dropped, 1), depsOf(lane, project), "overwrite", project.move); + stop(); + + // d [1,3) inside a [0,4): head [0,1), tail [3,4), all in the very first store write. + expect(states[0]).toBe("a@0+1 b@4+4 d@1+2 a~tail@3+1"); + }); + + it("shows an insert's pushed clips, split head and tail in the first state after the drop", async () => { + const a = clip("a", 0, 4); + const b = clip("b", 4, 4); + const dropped = clip("d", 20, 2); + const lane = [a, b, dropped]; + usePlayerStore.getState().setElements(lane); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4 }, + d: { start: 20, duration: 2 }, + }); + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); + + await commitPlacementDrop(dragOf(dropped, 2), depsOf(lane, project), "insert", project.move); + stop(); + + // d [2,4) inserted into a [0,4): head [0,2), tail [4,6), b pushed to [6,10). + expect(states[0]).toBe("a@0+2 b@6+4 d@2+2 a~tail@4+2"); + }); + + it("takes the preview's manifest again once the drop is over", () => { + const unmount = mountPlayer(); + act(() => postStalePreviewManifest()); + expect(usePlayerStore.getState().elements.map((el) => el.id)).toEqual(["a", "b", "d"]); + unmount(); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacementTestHarness.ts b/packages/studio/src/player/components/timelinePlacementTestHarness.ts new file mode 100644 index 0000000000..14fd74f834 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementTestHarness.ts @@ -0,0 +1,162 @@ +import { expect, vi } from "vitest"; +import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; +import type { TimelineElement } from "../store/playerStore"; +import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; +import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { PlacementFold, PlacementOps } from "./timelinePlacementCommit"; +import { + buildEditHistoryEntry, + createEmptyEditHistory, + hashEditHistoryContent, + pushEditHistoryEntry, + undoEditHistory, + type EditHistoryState, +} from "../../utils/editHistory"; + +export function clip( + id: string, + start: number, + duration: number, + extra: Partial = {}, +) { + return { id, domId: id, tag: "video", start, duration, track: 1, ...extra } as TimelineElement; +} + +export interface Doc { + [id: string]: { start: number; duration: number; playbackStart?: number }; +} + +/** A tiny stand-in for the project file plus the history recorder, driven by the real reducer. */ +export function createFakeProject(initial: Doc) { + let doc: Doc = structuredClone(initial); + let history: EditHistoryState = createEmptyEditHistory(); + let clock = 0; + const serialize = (d: Doc) => + JSON.stringify(Object.entries(d).sort(([x], [y]) => (x < y ? -1 : 1))); + const record = (mutate: () => void, label: string, fold: PlacementFold) => { + const before = serialize(doc); + mutate(); + history = pushEditHistoryEntry( + history, + buildEditHistoryEntry({ + id: `e${clock}`, + projectId: "p", + label, + kind: "timeline", + coalesceKey: fold.coalesceKey, + coalesceMs: fold.coalesceMs, + now: (clock += 1000), + files: { "index.html": { before, after: serialize(doc) } }, + }), + ); + }; + const current = (element: TimelineElement) => { + const found = doc[element.id]; + if (!found) throw new Error(`${element.id} is not in the document`); + // The element a step hands over must describe what the document holds right now. + expect({ start: element.start, duration: element.duration }).toEqual({ + start: found.start, + duration: found.duration, + }); + return found; + }; + const ops: PlacementOps = { + split: async (element, at, fold) => { + record( + () => { + const target = current(element); + const end = target.start + target.duration; + doc[`${element.id}-split`] = { + start: at, + duration: end - at, + playbackStart: (target.playbackStart ?? 0) + (at - target.start), + }; + target.duration = at - target.start; + }, + "split", + fold, + ); + return true; + }, + remove: async (elements, fold) => { + record( + () => { + for (const element of elements) { + current(element); + delete doc[element.id]; + } + }, + "delete", + fold, + ); + return true; + }, + toast: vi.fn(), + reloadPreview: vi.fn(), + }; + const resize = async (changes: TimelineGroupResizeChange[], fold: PlacementFold) => { + record( + () => { + for (const change of changes) { + const target = current(change.element); + target.start = change.start; + target.duration = change.duration; + if (change.playbackStart != null) target.playbackStart = change.playbackStart; + } + }, + "resize", + fold, + ); + }; + const move = async (edits: TimelineMoveEdit[], fold: PlacementFold) => { + record( + () => { + for (const edit of edits) { + const found = doc[edit.element.id]; + if (found) found.start = edit.updates.start; + else + doc[edit.element.id] = { start: edit.updates.start, duration: edit.element.duration }; + } + }, + "move", + fold, + ); + return true; + }; + return { + ops, + resize, + move, + doc: () => doc, + history: () => history, + undo: () => { + const undone = undoEditHistory( + history, + { "index.html": hashEditHistoryContent(serialize(doc)) }, + 0, + ); + if (!undone.ok) throw new Error(`undo refused: ${undone.reason}`); + return undone.filesToWrite["index.html"]; + }, + serializeInitial: () => serialize(initial), + }; +} + +export function dragOf(element: TimelineElement, previewStart: number, previewTrack = 1) { + return { + element, + previewStart, + previewTrack, + insertRow: null, + } as unknown as DraggedClipState; +} + +export function depsOf(elements: TimelineElement[], project: ReturnType) { + return { + elements, + trackOrder: [1], + updateElement: vi.fn(), + placementOps: project.ops, + onResizeElements: project.resize, + } as unknown as DragCommitDeps; +} diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 7794be1b04..f7fca7faed 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -40,6 +40,7 @@ import { normalizeToZones } from "../components/timelineZones"; import { applyPreviewAudioFlags, setPreviewPlaybackRate } from "../lib/timelineIframeHelpers"; import { scrubMusicAtSeek, stopScrubPreviewAudio } from "../lib/playbackScrub"; import { hasTimelinePerformanceFixtureLease } from "../lib/timelinePerformanceFixture"; +import { timelineManifestsHeld } from "../lib/timelineManifestHold"; import { applyCachedSourceDurations, probeMissingSourceDurations } from "../lib/mediaProbe"; import { shouldResumeForwardPlaybackAfterSeek, shouldStopAfterSeek } from "../lib/playbackSeek"; import { applyPreviewVariablesToUrl } from "../../hooks/previewVariablesStore"; @@ -64,13 +65,13 @@ export function useTimelinePlayer() { const { setIsPlaying, setCurrentTime, setDuration, requestTimelineReady, setElements } = usePlayerStore.getState(); - // The fixture lease belongs at this shared synchronization boundary so every - // iframe discovery path has the same owner for deciding whether it may write. + // The fixture lease and the drop's manifest hold belong at this shared synchronization + // boundary so every iframe discovery path has the same owner for deciding whether it may write. const syncTimelineElements = useCallback( // The lease guard adds one deliberate branch at the shared synchronization boundary. // fallow-ignore-next-line complexity (elements: TimelineElement[], nextDuration?: number) => { - if (hasTimelinePerformanceFixtureLease()) return; + if (hasTimelinePerformanceFixtureLease() || timelineManifestsHeld()) return; const state = usePlayerStore.getState(); const resolvedDuration = nextDuration ?? state.duration; // applyCachedSourceDurations re-applies the cached probe duration: re-derived diff --git a/packages/studio/src/player/lib/timelineManifestHold.ts b/packages/studio/src/player/lib/timelineManifestHold.ts new file mode 100644 index 0000000000..d1e0c5b739 --- /dev/null +++ b/packages/studio/src/player/lib/timelineManifestHold.ts @@ -0,0 +1,16 @@ +let holds = 0; + +/** While held, clip manifests from the preview are ignored; release() lets the next one in. */ +export function holdTimelineManifests(): () => void { + holds += 1; + let released = false; + return () => { + if (released) return; + released = true; + holds -= 1; + }; +} + +export function timelineManifestsHeld(): boolean { + return holds > 0; +}