diff --git a/src/components/ai-edition/Modals.tsx b/src/components/ai-edition/Modals.tsx index cc077efd1..9ac9d1079 100644 --- a/src/components/ai-edition/Modals.tsx +++ b/src/components/ai-edition/Modals.tsx @@ -704,7 +704,7 @@ interface EditClipModalProps extends BaseModalProps { // a draggable dual-handle range over the asset's full source duration — // replaces the old numeric-input-only form. Trim range AND crop are both // per-clip and both edited here (see clipSchema.cropRegion / useTimeline's -// updateClipSourceRange + updateClipCrop) — crop used to be a document-wide +// applyClipEdit, which composes the two into one save) — crop used to be a document-wide // setting behind its own facet-rail button; it's a framing choice for one // piece of footage, so it belongs with the rest of this clip's edits. export function EditClipModal({ diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index 8c9ad63b5..4026266cb 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -1331,8 +1331,14 @@ export function NewEditorShell() { videoSources={videoSources} onApply={(sStart, sEnd, cropRegion) => { if (!editClipTarget) return; - void tl.updateClipSourceRange(editClipTarget.id, sStart, sEnd); - if (cropRegion !== undefined) void tl.updateClipCrop(editClipTarget.id, cropRegion); + const clipId = editClipTarget.id; + // One user action, one document, one save. This used to be two calls — + // `updateClipSourceRange` then `updateClipCrop` — each building its next + // document from the same pre-Apply one, so the second write clobbered the + // first and one of the two edits vanished silently (#355). It goes on the + // shared write queue for the same reason every other timeline edit does: + // so it can't clobber, or be clobbered by, a save already in flight. + void enqueueTimelineWrite(() => tl.applyClipEdit(clipId, sStart, sEnd, cropRegion)); setEditClipTarget(null); }} /> diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx index 280853bef..f3a067bca 100644 --- a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -63,14 +63,19 @@ function clip(startSec: number, endSec: number) { }; } +/** The asset every clip above points at. No `cameraTrack`: this recording has no webcam, + * which is what the Full Camera button is gated on. */ +const NO_CAMERA_ASSET = { id: "a1", label: "rec", durationSec: TOTAL_SEC }; + /** By default one 30-minute clip carrying a single one-second annotation. */ function renderTimeline( clips = [clip(0, TOTAL_SEC)], annotation = { id: "ann1", startMs: 10_000, endMs: 11_000 }, + assets: Array> = [NO_CAMERA_ASSET], ) { const tl = { clips, - assets: [{ id: "a1", label: "rec", durationSec: TOTAL_SEC }], + assets, annotationRegions: [annotation], speedRegions: [], cameraFullscreenRegions: [], @@ -215,6 +220,46 @@ describe("V4Timeline create-from-toolbar", () => { fireEvent.click(screen.getByTitle("buttons.addZoom")); expect(durationOf(tl)).toBeCloseTo(0.25, 3); }); + + // #353. A camera-fullscreen region grows the webcam overlay, so with no webcam on the + // timeline it renders nothing in the preview and nothing in the export — the region is + // stored and forgotten. `addCameraFullscreen` now refuses to write one; the button says + // so before it is clicked instead of looking like it worked. + it("disables Add Full Camera when no clip on the timeline has a camera", () => { + renderTimeline(); + expect(screen.getByTitle("buttons.addCameraFullscreen")).toBeDisabled(); + }); + + it("enables Add Full Camera as soon as a clip's asset carries one", () => { + renderTimeline(undefined, undefined, [ + { + ...NO_CAMERA_ASSET, + cameraTrack: { sourcePath: "/tmp/cam.webm", startMs: 0, offsetMs: 0, visible: true }, + }, + ]); + expect(screen.getByTitle("buttons.addCameraFullscreen")).toBeEnabled(); + }); + + // The disabled button is only half the promise: an empty lane advertises the shortcut + // that fills it, so on a camera-less project it was still inviting a `C` press that + // `addCameraFullscreen` now refuses. It borrows the Layout pane's "No Webcam" wording + // instead, so the two surfaces agree about the same project. + it("does not advertise the C shortcut on a lane that cannot be filled", () => { + renderTimeline(); + expect(screen.getByText("layout.noWebcam")).toBeInTheDocument(); + expect(screen.queryByText("hints.pressCameraFullscreen")).not.toBeInTheDocument(); + }); + + it("advertises it again once a camera is on the timeline", () => { + renderTimeline(undefined, undefined, [ + { + ...NO_CAMERA_ASSET, + cameraTrack: { sourcePath: "/tmp/cam.webm", startMs: 0, offsetMs: 0, visible: true }, + }, + ]); + expect(screen.getByText("hints.pressCameraFullscreen")).toBeInTheDocument(); + expect(screen.queryByText("layout.noWebcam")).not.toBeInTheDocument(); + }); }); describe("V4Timeline clip row", () => { diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 0872d7f87..b6e5f34ea 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -39,6 +39,7 @@ import { useTimelineTranscriptGate } from "@/lib/ai-edition/store/transcriptionS import { useChatPromptBus } from "@/lib/ai-edition/store/useChatPromptBus"; import { useEditorSettings } from "@/lib/ai-edition/store/useEditorSettings"; import type { useTimeline } from "@/lib/ai-edition/store/useTimeline"; +import { hasAnyClipWithCamera } from "@/lib/ai-edition/timeline/camera"; import { formatSec } from "@/lib/ai-edition/timeline/format"; import { newRegionDurationSec, @@ -396,6 +397,9 @@ export function V4Timeline({ onEditClip: (clip: AxcutClip) => void; }) { const t = useScopedT("timeline"); + // The camera lane borrows the Layout pane's "No Webcam" wording when there is no + // camera to grow, so the two surfaces say the same thing about the same project. + const ts = useScopedT("settings"); const tracksRef = useRef(null); // The transformed canvas is the true timeline coordinate frame — clips, pills // and the playhead are all positioned inside it. Time↔x math must measure THIS @@ -465,6 +469,12 @@ export function V4Timeline({ : t("toolbar.smartCutsNeedsTranscript"); const clips = tl.clips; + // A camera-fullscreen region grows the webcam overlay, so on a project with no webcam + // it renders nothing in the preview and nothing in the export. `addCameraFullscreen` + // refuses to write one (see useTimeline) — this makes the control say so before it is + // clicked instead of looking like it worked. Same question, same helper as the Layout + // pane: is a camera attached anywhere on this timeline? + const hasAnyCamera = useMemo(() => hasAnyClipWithCamera(tl.assets, clips), [tl.assets, clips]); const total = useMemo( () => Math.max( @@ -1394,6 +1404,8 @@ export function V4Timeline({ className={styles.tlToolBtn} title={t("buttons.addCameraFullscreen")} aria-label={t("buttons.addCameraFullscreen")} + disabled={!hasAnyCamera} + style={!hasAnyCamera ? { opacity: 0.55, cursor: "not-allowed" } : undefined} onClick={() => void tl.addCameraFullscreen(newRegionDurationSec())} > @@ -1555,7 +1567,13 @@ export function V4Timeline({
{renderPills(trimPills, t("hints.pressTrim"))}
{renderPills(zoomPills, t("hints.pressZoom"))}
- {renderPills(cameraFullscreenPills, t("hints.pressCameraFullscreen"))} + {/* Advertising "Press C" on a project with no webcam invites a keystroke + that `addCameraFullscreen` now refuses (#353). The toolbar button is + already disabled; this keeps the lane from contradicting it. */} + {renderPills( + cameraFullscreenPills, + hasAnyCamera ? t("hints.pressCameraFullscreen") : ts("layout.noWebcam"), + )}
) : null} diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index cc51b3a01..939fc7c9d 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -341,7 +341,7 @@ describe("useTimeline backfills missing source dimensions on load", () => { }); }); -describe("useTimeline.updateClipSourceRange (Edit-clip modal)", () => { +describe("useTimeline.applyClipEdit (Edit-clip modal)", () => { const anchoredZoom = (id: string, s: number, e: number) => ({ id, startMs: s * 1000, @@ -380,7 +380,7 @@ describe("useTimeline.updateClipSourceRange (Edit-clip modal)", () => { const { result } = renderTimeline(); // Trim the 10s clip down to its first 4s of source. await act(async () => { - await result.current.updateClipSourceRange("clip_a", 0, 4); + await result.current.applyClipEdit("clip_a", 0, 4); }); const clip = useProjectStore.getState().document?.timeline.clips[0]; expect(clip).toMatchObject({ sourceStartSec: 0, sourceEndSec: 4 }); @@ -392,7 +392,7 @@ describe("useTimeline.updateClipSourceRange (Edit-clip modal)", () => { it("drops a pill sitting over the truncated tail and keeps the one that survives", async () => { const { result } = renderTimeline(); await act(async () => { - await result.current.updateClipSourceRange("clip_a", 0, 4); + await result.current.applyClipEdit("clip_a", 0, 4); }); const zooms = useProjectStore.getState().document?.zoomRanges ?? []; // z_keep (source 2-3) stays; z_drop (source 6-8) is entirely past the new 4s end. @@ -414,7 +414,7 @@ describe("useTimeline.updateClipSourceRange (Edit-clip modal)", () => { }); const { result } = renderTimeline(); await act(async () => { - await result.current.updateClipSourceRange("clip_a", 0, 5); + await result.current.applyClipEdit("clip_a", 0, 5); }); const zooms = useProjectStore.getState().document?.zoomRanges ?? []; expect(zooms).toHaveLength(1); @@ -426,6 +426,123 @@ describe("useTimeline.updateClipSourceRange (Edit-clip modal)", () => { endMs: 5000, }); }); + + // #355. Apply used to fire `updateClipSourceRange` and `updateClipCrop` as two + // concurrent saves, each built from the same pre-Apply document — so the second + // write clobbered the first and one of the two edits vanished with no error and no + // toast. Which one survived depended on IPC timing, which is why it read as "the app + // randomly forgets my crop". + it("keeps BOTH the source range and the crop when Apply changes them together", async () => { + const { result } = renderTimeline(); + const crop = { x: 0.1, y: 0.2, width: 0.5, height: 0.5 }; + await act(async () => { + await result.current.applyClipEdit("clip_a", 0, 4, crop); + }); + const clip = useProjectStore.getState().document?.timeline.clips[0]; + expect(clip).toMatchObject({ sourceStartSec: 0, sourceEndSec: 4, cropRegion: crop }); + // The width still followed the range edit — the crop is applied to the + // RESEQUENCED clips, not to a stale copy of them. + expect(clip?.timelineEndSec).toBe(4); + // One user action, one document, one write: two saves is the race itself. + expect(bridgeMocks.save).toHaveBeenCalledTimes(1); + }); + + it("clears the crop on an explicit null and leaves it alone on undefined", async () => { + useProjectStore.setState({ + document: { + ...sampleDoc, + timeline: { + ...sampleDoc.timeline, + clips: [ + { ...sampleDoc.timeline.clips[0], cropRegion: { x: 0, y: 0, width: 0.5, height: 1 } }, + ], + }, + }, + }); + const { result } = renderTimeline(); + // `undefined` is the modal's "crop section untouched" — the stored region stays. + await act(async () => { + await result.current.applyClipEdit("clip_a", 0, 6); + }); + expect(useProjectStore.getState().document?.timeline.clips[0].cropRegion).toEqual({ + x: 0, + y: 0, + width: 0.5, + height: 1, + }); + // `null` is "reset to no crop", stored as an absent field rather than the + // identity region. + await act(async () => { + await result.current.applyClipEdit("clip_a", 0, 6, null); + }); + expect(useProjectStore.getState().document?.timeline.clips[0].cropRegion).toBeUndefined(); + }); +}); + +// #353. The toolbar button and the `C` shortcut both used to write a region on a +// project with no webcam: it persists into `legacyEditor.cameraFullscreenRegions`, +// renders nothing in the preview (PreviewCanvas short-circuits on a missing +// `webcamRect`) and nothing in the export, forever, with no feedback. The gate lives +// in the shared mutation so both entry points — and any future one — are covered. +describe("useTimeline.addCameraFullscreen (camera gate)", () => { + const cameraAsset = { + ...sampleDoc.assets[0], + cameraTrack: { + sourcePath: "/tmp/camera.webm", + startMs: 0, + offsetMs: 0, + visible: true, + // Dimensions filled in so the hook's backfill probe has nothing to do — an + // unprobed camera would fire its own `saveDocument` alongside this test's. + width: 1280, + height: 720, + }, + }; + + beforeEach(() => { + useProjectStore.getState().clear(); + for (const mock of Object.values(bridgeMocks)) mock.mockReset(); + bridgeMocks.save.mockImplementation(async (doc: typeof sampleDoc) => ({ + success: true, + document: doc, + })); + useProjectStore.setState({ + projectId: "proj_test", + document: sampleDoc, + currentTimeSec: 1, + revision: 1, + status: "ready", + error: null, + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("writes nothing when no clip on the timeline has a camera", async () => { + // sampleDoc's only asset carries `cameraTrack: null`. + const { result } = renderTimeline(); + await act(async () => { + await result.current.addCameraFullscreen(); + }); + expect(bridgeMocks.save).not.toHaveBeenCalled(); + expect(useProjectStore.getState().document?.legacyEditor).toBeNull(); + expect(result.current.cameraFullscreenRegions).toEqual([]); + }); + + it("still writes a region when a clip's asset carries a camera", async () => { + useProjectStore.setState({ document: { ...sampleDoc, assets: [cameraAsset] } }); + const { result } = renderTimeline(); + await act(async () => { + await result.current.addCameraFullscreen(); + }); + const legacy = useProjectStore.getState().document?.legacyEditor as Record; + const regions = legacy.cameraFullscreenRegions as Array<{ startMs: number; endMs: number }>; + expect(regions).toHaveLength(1); + // 2s at the playhead (currentTimeSec = 1), the shared default. + expect(regions[0]).toMatchObject({ startMs: 1000, endMs: 3000 }); + }); }); describe("useTimeline.addAnnotation", () => { diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index 716b4003a..788a8a4fc 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -20,6 +20,7 @@ import { setClipSourceRange, } from "../document/timeline"; import type { AxcutClipCropRegion, AxcutDocument } from "../schema"; +import { hasAnyClipWithCamera } from "../timeline/camera"; import { probeVideoDimensions, probeVideoDuration } from "../timeline/duration"; import { anchorRegionsWithDerivedMs, @@ -394,9 +395,20 @@ export function useTimeline() { // Full Camera: a plain time span (no value) during which the preview/export // grows the webcam overlay to (almost) fill the canvas and eases it back. + // + // With no webcam anywhere on the timeline there is nothing to grow, so the region + // renders nothing in the preview (`PreviewCanvas.effectiveLayout` short-circuits on + // a missing `webcamRect`) and nothing in the export — it just sits in + // `legacyEditor.cameraFullscreenRegions` forever. The agent's `addCameraFullscreen` + // tool already refuses this and says why (electron/ai-edition/agent-tools.ts, + // `noCameraUnderSpan`); the gate lives HERE rather than at each button so both UI + // entry points — the toolbar and the `C` shortcut — and any future one are covered + // by construction. `hasAnyClipWithCamera` is the consolidated answer to "does this + // project have a camera at all", used the same way by the Layout pane. const addCameraFullscreen = useCallback( async (durationSec = DEFAULT_NEW_REGION_SEC) => { if (!document) return; + if (!hasAnyClipWithCamera(document.assets, document.timeline.clips)) return; const timeMs = Math.round(playheadSec() * 1000); const endMs = timeMs + Math.round(durationSec * 1000); const legacy = (document.legacyEditor as Record) ?? {}; @@ -858,38 +870,56 @@ export function useTimeline() { setClipSelection(null); }, []); - // Axcut-consistent clip trim: only the source range is user-editable (the - // Edit Clip dialog's draggable track). Changing it changes the clip's - // effective duration, so every clip is resequenced back-to-back afterward — - // same invariant as insertClipAt/moveClip/removeClip — instead of leaving - // downstream clips at their old timeline positions (which would overlap). - // The whole recipe (resequence width + clamp/rederive pills) lives in the one - // pure `setClipSourceRange`, shared with the op dispatcher and the LLM tool. - const updateClipSourceRange = useCallback( - async (clipId: string, sourceStartSec: number, sourceEndSec: number) => { - if (!document) return; - await saveDocument(setClipSourceRange(document, clipId, sourceStartSec, sourceEndSec)); - }, - [document, saveDocument], - ); - - // Crop is a per-clip framing, not a document-wide setting — two clips - // (even from the same asset) can reasonably want different crops. Passing - // `null` clears it back to "no crop" (full frame) instead of storing the - // identity region explicitly. - const updateClipCrop = useCallback( - async (clipId: string, region: AxcutClipCropRegion | null) => { - if (!document) return; - const arr = document.timeline.clips.map((c) => - c.id === clipId ? { ...c, cropRegion: region ?? undefined } : c, - ); - const next: AxcutDocument = { - ...document, - timeline: { ...document.timeline, clips: arr }, - }; + // The Edit Clip dialog's Apply, as ONE document and ONE save. + // + // Source range and crop are two edits made in a single user action, and they used to + // be two independent saves fired back to back. Both built their next document from + // the SAME pre-Apply one — the crop write never saw the source-range change — so + // whichever IPC write landed last silently dropped the other edit, with no error and + // no toast (#355). Composing them means the crop is applied to the *resequenced* + // clips, which is also the only order that can be right. + // + // Axcut-consistent clip trim: only the source range is user-editable (the dialog's + // draggable track). Changing it changes the clip's effective duration, so every clip + // is resequenced back-to-back afterward — same invariant as + // insertClipAt/moveClip/removeClip — instead of leaving downstream clips at their old + // timeline positions (which would overlap). That whole recipe (resequence width + + // clamp/rederive pills) lives in the one pure `setClipSourceRange`, shared with the op + // dispatcher and the LLM tool. + // + // Crop is a per-clip framing, not a document-wide setting — two clips (even from the + // same asset) can reasonably want different crops. `undefined` means the dialog's crop + // section was never touched (leave the stored value alone); `null` clears it back to + // "no crop" (full frame) rather than storing the identity region explicitly. + // + // The document is read from the store, not off the render closure, so this composes + // with `useSequentialTimelineOps`: queued behind another timeline write, it still sees + // what that write committed. Same reason as `setTrimEntries` / `insertClipAt`. + const applyClipEdit = useCallback( + async ( + clipId: string, + sourceStartSec: number, + sourceEndSec: number, + cropRegion?: AxcutClipCropRegion | null, + ) => { + const doc = useProjectStore.getState().document; + if (!doc) return; + const ranged = setClipSourceRange(doc, clipId, sourceStartSec, sourceEndSec); + const next: AxcutDocument = + cropRegion === undefined + ? ranged + : { + ...ranged, + timeline: { + ...ranged.timeline, + clips: ranged.timeline.clips.map((c) => + c.id === clipId ? { ...c, cropRegion: cropRegion ?? undefined } : c, + ), + }, + }; await saveDocument(next); }, - [document, saveDocument], + [saveDocument], ); // Background probe: read the asset's actual duration and patch the @@ -1111,8 +1141,7 @@ export function useTimeline() { removeRegions, selectRegion, clearSelection, - updateClipSourceRange, - updateClipCrop, + applyClipEdit, insertClipAt, moveClip, duplicateClip,