Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/ai-edition/Modals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
10 changes: 8 additions & 2 deletions src/components/ai-edition/NewEditorShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}}
/>
Expand Down
47 changes: 46 additions & 1 deletion src/components/ai-edition/v4/V4Timeline.geometry.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<string, unknown>> = [NO_CAMERA_ASSET],
) {
const tl = {
clips,
assets: [{ id: "a1", label: "rec", durationSec: TOTAL_SEC }],
assets,
annotationRegions: [annotation],
speedRegions: [],
cameraFullscreenRegions: [],
Expand Down Expand Up @@ -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", () => {
Expand Down
20 changes: 19 additions & 1 deletion src/components/ai-edition/v4/V4Timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<HTMLDivElement | null>(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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())}
>
<Maximize2 size={15} />
Expand Down Expand Up @@ -1555,7 +1567,13 @@ export function V4Timeline({
<div className={styles.tlLane}>{renderPills(trimPills, t("hints.pressTrim"))}</div>
<div className={styles.tlLane}>{renderPills(zoomPills, t("hints.pressZoom"))}</div>
<div className={styles.tlLane}>
{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"),
)}
</div>
</>
) : null}
Expand Down
125 changes: 121 additions & 4 deletions src/lib/ai-edition/store/useTimeline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
Expand All @@ -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.
Expand All @@ -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);
Expand All @@ -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<string, unknown>;
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", () => {
Expand Down
Loading
Loading