diff --git a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx index 7fdd1aeef..48c587402 100644 --- a/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx +++ b/src/components/ai-edition/v4/V4Timeline.geometry.test.tsx @@ -122,7 +122,6 @@ function renderTimeline( zoomRegions: [], trimRanges: [], hasEditRegions: true, - ...overrides, selection: null, multiSelection: [], clipSelection: null, @@ -141,6 +140,7 @@ function renderTimeline( clearTimeline: vi.fn(async () => { /* the toolbar only awaits it */ }), + ...overrides, }; const setCurrentTime = vi.fn(); const timeline = ( @@ -316,6 +316,47 @@ describe("V4Timeline lane pills", () => { dragHandle(right, 885.5); expect(tl.updateAnnotationSpan).toHaveBeenLastCalledWith("ann1", 10_000, 1_782_000); }); + + // Two regions with nothing to tell them apart, 100–130 s and 130–160 s: one pill over + // 50–80 px. An edit must reach the one the user pointed at, or the pill never splits. + const MERGED = { + annotationRegions: [ + { id: "ann1", startMs: 100_000, endMs: 130_000 }, + { id: "ann2", startMs: 130_000, endMs: 160_000 }, + ], + }; + + it("selects the region under the pointer on a pill merged from two (#1017)", () => { + const { pill, tl } = renderTimeline(undefined, undefined, undefined, undefined, MERGED); + for (const [clientX, id] of [ + [75, "ann2"], + [55, "ann1"], + ] as const) { + fireEvent.pointerDown(pill, { clientX }); + window.dispatchEvent(new MouseEvent("pointerup", { clientX })); + expect(tl.selectRegion).toHaveBeenLastCalledWith("annotation", id, { additive: false }); + } + }); + + it("moves a merged pill by the region it was grabbed by, which keeps the selection alive", () => { + const { pill, tl } = renderTimeline(undefined, undefined, undefined, undefined, MERGED); + fireEvent.pointerDown(pill, { clientX: 75 }); + window.dispatchEvent(new MouseEvent("pointermove", { clientX: 165 })); + window.dispatchEvent(new MouseEvent("pointerup", { clientX: 165 })); + expect(tl.updateAnnotationSpan).toHaveBeenCalledWith( + "ann2", + expect.any(Number), + expect.any(Number), + ); + }); + + it("draws a merged pill selected whichever of its regions is", () => { + const { pill } = renderTimeline(undefined, undefined, undefined, undefined, { + ...MERGED, + selection: { kind: "annotation", id: "ann2" }, + }); + expect(pill.className).toContain("lanePillSel"); + }); }); describe("V4Timeline lane pill keyboard", () => { diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 0d9d7f537..3ebe75192 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -920,26 +920,46 @@ export function V4Timeline({ end: number; } | null>(null); - // Drag a lane pill to move it (mode "move", keeps duration) or resize one - // edge (mode "l"/"r"). Zoom/speed/annotation are timeline-ms; trims map - // back to source-seconds through their carrying clip. + // A merged pill can be several regions that merely match (#1017), and an edit must reach + // only the one the user pointed at, so the pill splits: a click at `atSec` selects the last + // region starting at or before it. Without a pointer (keyboard), the pill's first region. + // The store still edits every fragment of that region across a clip junction together. + // Returns the id selected. const selectPill = useCallback( - (pill: LanePill, additive: boolean) => { - tl.selectRegion(pill.kind, pill.id, { additive }); + (pill: LanePill, additive: boolean, atSec?: number): string => { + const regions: Record> = { + annotation: tl.annotationRegions, + speed: tl.speedRegions, + zoom: tl.zoomRegions, + cameraFullscreen: tl.cameraFullscreenRegions, + // Nothing to edit on a trim but its span, and a span edit moves the whole pill. + trim: [], + }; + const atMs = (atSec ?? Number.NaN) * 1000; + const [pointed] = regions[pill.kind] + .filter((r) => pill.sourceIds.includes(r.id) && r.startMs <= atMs) + .sort((a, b) => b.startMs - a.startMs); + const id = pointed?.id ?? pill.id; + tl.selectRegion(pill.kind, id, { additive }); + return id; }, [tl], ); + // Drag a lane pill to move it (mode "move", keeps duration) or resize one + // edge (mode "l"/"r"). Zoom/speed/annotation are timeline-ms; trims map + // back to source-seconds through their carrying clip. const startPillDrag = useCallback( (e: ReactPointerEvent, pill: LanePill, dragMode: "move" | "l" | "r") => { e.preventDefault(); e.stopPropagation(); - selectPill(pill, e.shiftKey); // Scale drag deltas against the canvas (full zoomed timeline) width, so a // drag tracks the cursor exactly regardless of padding, scrollbar or zoom. const el = canvasRef.current; if (!el) return; const r = el.getBoundingClientRect(); + // A move rebuilds the pill as one region that keeps this id, so the selection survives. + const grabbed = selectPill(pill, e.shiftKey, ((e.clientX - r.left) / r.width) * total); const startX = e.clientX; const dur = pill.end - pill.start; // A trim can span several clips; it's stored as one source-time entry per @@ -981,12 +1001,12 @@ export function V4Timeline({ const apply = async (start: number, end: number): Promise => { const s = Math.max(0, Math.min(end - MIN_REGION_SEC, start)); const en = Math.min(total, Math.max(s + MIN_REGION_SEC, end)); - if (pill.kind === "zoom") await tl.updateZoomSpan(pill.id, s * 1000, en * 1000); - else if (pill.kind === "speed") await tl.updateSpeedSpan(pill.id, s * 1000, en * 1000); + if (pill.kind === "zoom") await tl.updateZoomSpan(grabbed, s * 1000, en * 1000); + else if (pill.kind === "speed") await tl.updateSpeedSpan(grabbed, s * 1000, en * 1000); else if (pill.kind === "annotation") - await tl.updateAnnotationSpan(pill.id, s * 1000, en * 1000); + await tl.updateAnnotationSpan(grabbed, s * 1000, en * 1000); else if (pill.kind === "cameraFullscreen") - await tl.updateCameraFullscreenSpan(pill.id, s * 1000, en * 1000); + await tl.updateCameraFullscreenSpan(grabbed, s * 1000, en * 1000); else { // Trims are stored in source-time per asset but manipulated on the // timeline like every other pill. Ventilate the new span across the @@ -1611,8 +1631,9 @@ export function V4Timeline({ useChatPromptBus.getState().submit(AI_ENHANCE_PROMPT); }, []); - const isPillSelected = (id: string) => - tl.selection?.id === id || tl.multiSelection.some((m) => m.id === id); + // Any region under the pill: a click selects the one it lands on, not always the first. + const isPillSelected = (p: LanePill) => + p.sourceIds.some((id) => tl.selection?.id === id || tl.multiSelection.some((m) => m.id === id)); // Optimistic preview: during a clip-reorder drag, slide each region pill by // the same amount as the clip it sits on — mirroring the clip transforms so // zoom/speed/annotation/trim pills travel with their content in real time, @@ -1661,7 +1682,7 @@ export function V4Timeline({ tabIndex={seg.interactive ? 0 : undefined} className={`${styles.lanePill} ${laneOf(p.kind)}${ compact ? ` ${styles.lanePillCompact}` : "" - }${seg.interactive && isPillSelected(p.id) ? ` ${styles.lanePillSel}` : ""}`} + }${seg.interactive && isPillSelected(p) ? ` ${styles.lanePillSel}` : ""}`} style={{ left: `${pctAt(seg.segStart)}%`, // Measured on the expanded ruler at BOTH ends: a region straddling a pause diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index bb99bcb1c..917188529 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -6,6 +6,7 @@ import { DEFAULT_TEXT_PLATE } from "../annotations/background"; import { type RegionKind, readSpeedRegions } from "../document/timeline"; import type { AxcutDocument } from "../schema"; import { axcutSchemaVersion, parseDocumentFile } from "../schema"; +import { coalesceRegionsForRuler } from "../timeline/timelineMap"; import { useProjectStore } from "./projectStore"; import { clearHistory, redo, undo } from "./undo"; import { future, past } from "./undoStack"; @@ -1166,6 +1167,79 @@ describe("useTimeline zoom modifiers (rotation + focus mode)", () => { }); }); +describe("useTimeline edits one region of a merged pill (#1017)", () => { + // One 4–13 s pill of two regions with the same properties: `zoom_x` on clip A, and + // `zoom_p` drawn across the A|B junction, stored as one fragment per clip. + const zoom = (id: string, clipId: string, startSec: number, endSec: number) => ({ + id, + startMs: startSec * 1000, + endMs: endSec * 1000, + depth: 3 as const, + focus: { cx: 0.5, cy: 0.5 }, + clipId, + // Both clips play the source at its own timeline position. + sourceStartSec: startSec, + sourceEndSec: endSec, + }); + const clipA = sampleDoc.timeline.clips[0]; + const docWithMergedPill: AxcutDocument = { + ...sampleDoc, + timeline: { + ...sampleDoc.timeline, + clips: [ + clipA, + { + ...clipA, + id: "clip_b", + sourceStartSec: 10, + sourceEndSec: 20, + timelineStartSec: 10, + timelineEndSec: 20, + }, + ], + }, + zoomRanges: [ + zoom("zoom_x", "clip_a", 4, 7), + zoom("zoom_p", "clip_a", 7, 10), + zoom("zoom_p_b", "clip_b", 10, 13), + ], + }; + + 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: docWithMergedPill, + revision: 1, + status: "ready", + error: null, + }); + }); + + it("changes the region picked and its fragments, not its look-alike, and the pill splits", async () => { + expect(coalesceRegionsForRuler(docWithMergedPill.zoomRanges)).toHaveLength(1); + const { result } = renderTimeline(); + await act(async () => { + await result.current.updateZoomDepth("zoom_p_b", 2); + }); + const zooms = useProjectStore.getState().document?.zoomRanges ?? []; + expect(zooms.map((z) => [z.id, z.depth])).toEqual([ + ["zoom_x", 3], + ["zoom_p", 2], + ["zoom_p_b", 2], + ]); + expect(coalesceRegionsForRuler(zooms).map((p) => p.ids)).toEqual([ + ["zoom_x"], + ["zoom_p", "zoom_p_b"], + ]); + }); +}); + // Regression guard for the playhead-stutter fix. `currentTimeSec` is rewritten on // every animation frame during playback, and `useTimeline()` is called by the editor // shell — so subscribing to the playhead here re-rendered the entire editor (timeline, diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index 2b5eb6be4..b8a867f77 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -45,7 +45,7 @@ import { anchorRegionsWithDerivedMs, dropPillsByIds, replacePillSpan, - resolvePillIds, + resolveRegionIds, } from "../timeline/timelineMap"; import { dropTrimPillsByIds, resolveTimelineSpanToTrim } from "../timeline/trim-mapping"; import { MAX_ZOOM_SCALE, MIN_ZOOM_SCALE } from "../timeline/zoom-scale"; @@ -84,15 +84,17 @@ interface RegionHandle { type Clip = AxcutDocument["timeline"]["clips"][number]; /** - * Patch every region under the pill `id` belongs to. A payload edit must hit them all, - * or the pieces of one pill would disagree — and then, by the merge rule, visibly split. + * Patch the region `id` belongs to: every fragment of it across clip junctions, or the + * pieces of one region would disagree and, by the merge rule, visibly split. Not the whole + * pill: a region that merely matches and touches it is another region, left alone so the + * pill separates (#1017, see `resolveRegionIds`). */ -function patchPillById( +function patchRegionById( regions: T[], id: string, patch: Partial, ): T[] { - const under = new Set(resolvePillIds(regions, id)); + const under = new Set(resolveRegionIds(regions, id)); return regions.map((r) => (under.has(r.id) ? { ...r, ...patch } : r)); } @@ -728,7 +730,7 @@ export function useTimeline() { saveDocument( { ...doc, - zoomRanges: patchPillById(doc.zoomRanges, id, patch) as AxcutDocument["zoomRanges"], + zoomRanges: patchRegionById(doc.zoomRanges, id, patch) as AxcutDocument["zoomRanges"], }, { history: true, historyBase }, ), @@ -788,7 +790,7 @@ export function useTimeline() { }; const next: AxcutDocument = { ...doc, - zoomRanges: patchPillById(doc.zoomRanges, id, { + zoomRanges: patchRegionById(doc.zoomRanges, id, { focus: edit.focus, }) as AxcutDocument["zoomRanges"], }; @@ -844,7 +846,7 @@ export function useTimeline() { // again. When the writes since carried the focus along, there is nothing to add. next = { ...doc, - zoomRanges: patchPillById(doc.zoomRanges, edit.id, { + zoomRanges: patchRegionById(doc.zoomRanges, edit.id, { focus: edit.focus, }) as AxcutDocument["zoomRanges"], }; @@ -951,7 +953,7 @@ export function useTimeline() { if (annotationLiveRef.current !== doc) annotationRollbackRef.current = doc; const next: AxcutDocument = { ...doc, - annotations: patchPillById(doc.annotations, id, patch), + annotations: patchRegionById(doc.annotations, id, patch), }; setDocument(next, { history: false }); annotationLiveRef.current = next; @@ -1066,7 +1068,7 @@ export function useTimeline() { ...document, legacyEditor: { ...legacy, - speedRegions: patchPillById(prev, id, { speed }), + speedRegions: patchRegionById(prev, id, { speed }), }, }; await saveDocument(next, { history: true }); diff --git a/src/lib/ai-edition/timeline/timelineMap.test.ts b/src/lib/ai-edition/timeline/timelineMap.test.ts index 2e4bdb014..675601374 100644 --- a/src/lib/ai-edition/timeline/timelineMap.test.ts +++ b/src/lib/ai-edition/timeline/timelineMap.test.ts @@ -17,6 +17,7 @@ import { replacePillSpan, resolveNativePosition, resolvePillIds, + resolveRegionIds, } from "./timelineMap"; function clip(overrides: Partial & Pick): AxcutClip { @@ -811,6 +812,53 @@ describe("pills wired to the universal rules", () => { expect(resolvePillIds(regions, "c")).toEqual(["c"]); }); + it("resolveRegionIds narrows a pill to one region, with every fragment of it (#1017)", () => { + // `mine` is drawn across the A|B junction, so it is stored as two fragments; `other` + // merely matches it and touches it on clip A. One pill, two regions. + const regions = [ + ...anchorRegionsWithDerivedMs( + [{ id: "other", startMs: 10000, endMs: 20000, speed: 3 }], + clips, + ids(), + ), + ...anchorRegionsWithDerivedMs( + [{ id: "mine", startMs: 20000, endMs: 30000, speed: 3 }], + clips, + ids(), + ), + ]; + const mineOnB = regions.find((r) => (r as { clipId?: string }).clipId === "clip_b")?.id; + expect(resolvePillIds(regions, "other")).toHaveLength(3); + expect(resolveRegionIds(regions, "other")).toEqual(["other"]); + expect(resolveRegionIds(regions, mineOnB as string)).toEqual(["mine", mineOnB]); + }); + + it("resolveRegionIds keeps overlapping regions together: one changed alone would overlap", () => { + const regions = anchorRegionsWithDerivedMs( + [ + { id: "a", startMs: 2000, endMs: 6000, speed: 3 }, + { id: "b", startMs: 5000, endMs: 9000, speed: 3 }, + { id: "c", startMs: 9000, endMs: 12000, speed: 3 }, + ], + clips, + ids(), + ); + expect(resolveRegionIds(regions, "a")).toEqual(["a", "b"]); + expect(resolveRegionIds(regions, "c")).toEqual(["c"]); + }); + + it("resolveRegionIds keeps a 1 ms overlap together too: only an exact touch splits", () => { + const regions = anchorRegionsWithDerivedMs( + [ + { id: "a", startMs: 2000, endMs: 6001, speed: 3 }, + { id: "b", startMs: 6000, endMs: 9000, speed: 3 }, + ], + clips, + ids(), + ); + expect(resolveRegionIds(regions, "a")).toEqual(["a", "b"]); + }); + it("resizing a pill across a clip boundary re-anchors it into one fragment per clip", () => { const regions = anchorRegionsWithDerivedMs( [{ id: "s", startMs: 2000, endMs: 5000, speed: 3 }], @@ -824,6 +872,19 @@ describe("pills wired to the universal rules", () => { expect(coalesceRegionsForRuler(out)).toHaveLength(1); }); + it("a moved pill keeps the id it was grabbed by, so a selection on any member survives", () => { + const regions = anchorRegionsWithDerivedMs( + [ + { id: "a", startMs: 2000, endMs: 5000, speed: 3 }, + { id: "b", startMs: 5000, endMs: 9000, speed: 3 }, + ], + clips, + ids(), + ); + const out = replacePillSpan(regions, "b", 3000, 10000, clips, ids()); + expect(out.map((r) => [r.id, r.startMs, r.endMs])).toEqual([["b", 3000, 10000]]); + }); + it("clamps a resize at a neighbouring pill of different properties (magnet)", () => { const regions = [ ...anchorRegionsWithDerivedMs( diff --git a/src/lib/ai-edition/timeline/timelineMap.ts b/src/lib/ai-edition/timeline/timelineMap.ts index 8e3dcbcf7..577f74241 100644 --- a/src/lib/ai-edition/timeline/timelineMap.ts +++ b/src/lib/ai-edition/timeline/timelineMap.ts @@ -319,6 +319,45 @@ export function resolvePillIds p.ids.includes(id))?.ids ?? [id]; } +/** + * The regions an edit of `id`'s PROPERTIES reaches: the one region it belongs to, which can + * be less than its pill (#1017). A pill joins two kinds of neighbours, and only one of them + * is the same region: + * - the fragments of one region across a clip junction, one per clip. They change together, + * or a single edit would split a region the user drew as one; + * - a region that merely matches and touches it on the SAME clip. A region keeps at most one + * fragment per clip, so that is another region: the edit leaves it alone, the identities + * part, and the pill splits in two. + * Same-clip regions that OVERLAP stay together: changing one alone would leave two regions of + * different identities overlapping, which rule 2 forbids. + * + * Nothing records provenance, so two regions meeting exactly at a clip junction still edit as + * one: that layout cannot be told apart from one region's two fragments. + */ +export function resolveRegionIds< + T extends { id: string; startMs: number; endMs: number; clipId?: string }, +>(regions: T[], id: string, epsilonSec = 0.001): string[] { + const byId = new Map(regions.map((r) => [r.id, r])); + let run: string[] = []; + let runEndSec = Number.NEGATIVE_INFINITY; + let prev: T | undefined; + // Left to right; a same-clip member that only touches ends the run (whole ms: no epsilon). + for (const memberId of resolvePillIds(regions, id, epsilonSec)) { + const member = byId.get(memberId); + if (!member) continue; + const startSec = member.startMs / 1000; + if (prev && member.clipId === prev.clipId && startSec >= runEndSec) { + if (run.includes(id)) return run; + run = []; + runEndSec = Number.NEGATIVE_INFINITY; + } + run.push(memberId); + runEndSec = Math.max(runEndSec, member.endMs / 1000); + prev = member; + } + return run.includes(id) ? run : [id]; +} + /** * Deleting a pill deletes every region under it. Which regions those are is RESOLVED from * the universal merge rule (same properties + touching = one pill), never from stored @@ -352,6 +391,9 @@ export function dropPillsByIds( regions: T[], @@ -381,7 +423,7 @@ export function replacePillSpan