From 08226c53b9a60d4108a85316d116d6f88ea511f5 Mon Sep 17 00:00:00 2001 From: newHashub <37201917+newHashub@users.noreply.github.com> Date: Wed, 9 Sep 2026 10:12:33 +0800 Subject: [PATCH 1/2] fix: ripple timeline to drop removed ranges instead of leaving dead zones When a clip is split and one half deleted, the editor timeline kept a dead zone for the removed range instead of compacting it. Playback then started from the wrong source position and the clip blocks rendered with a leading gap. Rework the timeline mapping so removed ranges no longer occupy space: - add getKeptTimelineSpans() to compute each kept clip's compacted timeline bounds from its source coordinates - rewrite mapTimelineTimeToSourceTime / mapSourceTimeToTimelineTime to convert through those kept spans (ripple) instead of raw source time - make getTimelineDurationMs sum clip display durations - update clip split / span-change commands to convert timeline offsets back into source coordinates - render clip blocks at their ripple-corrected timeline positions --- .../hooks/useClipRegionCommands.ts | 74 ++++--- .../timeline/model/timelineModel.ts | 9 +- src/components/video-editor/types.test.ts | 55 +++-- src/components/video-editor/types.ts | 193 +++++++++++++----- 4 files changed, 244 insertions(+), 87 deletions(-) diff --git a/src/components/video-editor/hooks/useClipRegionCommands.ts b/src/components/video-editor/hooks/useClipRegionCommands.ts index e94793e35..e28131eb6 100644 --- a/src/components/video-editor/hooks/useClipRegionCommands.ts +++ b/src/components/video-editor/hooks/useClipRegionCommands.ts @@ -2,13 +2,14 @@ import type { Span } from "dnd-timeline"; import { type Dispatch, type MutableRefObject, type SetStateAction, useCallback } from "react"; import { toast } from "sonner"; import { planClipSpeedChange } from "../clipSpeedChange"; -import type { - AnnotationRegion, - AudioRegion, - ClipRegion, - EditorEffectSection, - SpeedRegion, - ZoomRegion, +import { + getClipTimelineStartMs, + type AnnotationRegion, + type AudioRegion, + type ClipRegion, + type EditorEffectSection, + type SpeedRegion, + type ZoomRegion, } from "../types"; type Translator = ( @@ -79,15 +80,25 @@ export function useClipRegionCommands({ const handleClipSplit = useCallback( (splitMs: number) => { - const target = clipRegions.find( - (clip) => splitMs > clip.startMs && splitMs < clip.endMs, - ); + const target = clipRegions.find((clip) => { + const timelineStart = getClipTimelineStartMs(clip, clipRegions); + const timelineEnd = timelineStart + Math.max(0, clip.endMs - clip.startMs); + return splitMs > timelineStart && splitMs < timelineEnd; + }); if (!target) return; const leftId = `clip-${nextClipIdRef.current++}`; const rightId = `clip-${nextClipIdRef.current++}`; + const timelineStart = getClipTimelineStartMs(target, clipRegions); const splitAt = Math.round(splitMs); - const left: ClipRegion = { ...target, id: leftId, endMs: splitAt }; - const right: ClipRegion = { ...target, id: rightId, startMs: splitAt }; + const splitOffset = splitAt - timelineStart; + const newSourceStart = Math.round(target.startMs + splitOffset * target.speed); + const left: ClipRegion = { ...target, id: leftId, endMs: newSourceStart }; + const right: ClipRegion = { + ...target, + id: rightId, + startMs: newSourceStart, + endMs: newSourceStart + Math.max(0, target.endMs - target.startMs - splitOffset), + }; setClipRegions((current) => current.flatMap((clip) => (clip.id === target.id ? [left, right] : [clip])), ); @@ -99,30 +110,41 @@ export function useClipRegionCommands({ const handleClipSpanChange = useCallback( (id: string, span: Span) => { const oldClip = clipRegions.find((clip) => clip.id === id); - const newStart = Math.round(span.start); - const newEnd = Math.round(span.end); + const newTimelineStart = Math.round(span.start); + const newTimelineEnd = Math.round(span.end); + const oldTimelineStart = oldClip + ? getClipTimelineStartMs(oldClip, clipRegions) + : 0; + const oldTimelineEnd = + oldTimelineStart + Math.max(0, (oldClip?.endMs ?? 0) - (oldClip?.startMs ?? 0)); + const newTimelineDuration = Math.max(0, newTimelineEnd - newTimelineStart); + const speed = oldClip && oldClip.speed > 0 ? oldClip.speed : 1; + const newSourceStart = oldClip + ? Math.round(oldClip.startMs + (newTimelineStart - oldTimelineStart) * speed) + : 0; + const newEndMs = newSourceStart + newTimelineDuration; const removedSegments = oldClip ? [ - ...(newStart > oldClip.startMs - ? [{ startMs: oldClip.startMs, endMs: newStart }] + ...(newTimelineStart > oldTimelineStart + ? [{ startMs: oldTimelineStart, endMs: newTimelineStart }] : []), - ...(newEnd < oldClip.endMs - ? [{ startMs: newEnd, endMs: oldClip.endMs }] + ...(newTimelineEnd < oldTimelineEnd + ? [{ startMs: newTimelineEnd, endMs: oldTimelineEnd }] : []), ] : []; if (oldClip) { - const startDelta = newStart - oldClip.startMs; - const endDelta = newEnd - oldClip.endMs; - if (Math.abs(startDelta - endDelta) < 1 && Math.abs(startDelta) > 0) { + const timelineStartDelta = newTimelineStart - oldTimelineStart; + const timelineEndDelta = newTimelineEnd - oldTimelineEnd; + if (Math.abs(timelineStartDelta - timelineEndDelta) < 1 && Math.abs(timelineStartDelta) > 0) { setZoomRegions((current) => current.map((zoom) => - zoom.startMs < oldClip.endMs && zoom.endMs > oldClip.startMs + zoom.startMs < oldTimelineEnd && zoom.endMs > oldTimelineStart ? { ...zoom, - startMs: zoom.startMs + startDelta, - endMs: zoom.endMs + startDelta, + startMs: zoom.startMs + timelineStartDelta, + endMs: zoom.endMs + timelineStartDelta, } : zoom, ), @@ -150,7 +172,9 @@ export function useClipRegionCommands({ setClipRegions((current) => current.map((clip) => - clip.id === id ? { ...clip, startMs: newStart, endMs: newEnd } : clip, + clip.id === id + ? { ...clip, startMs: newSourceStart, endMs: newEndMs } + : clip, ), ); }, diff --git a/src/components/video-editor/timeline/model/timelineModel.ts b/src/components/video-editor/timeline/model/timelineModel.ts index e1589fd60..3477e3b36 100644 --- a/src/components/video-editor/timeline/model/timelineModel.ts +++ b/src/components/video-editor/timeline/model/timelineModel.ts @@ -16,6 +16,7 @@ import { isAudioTrackRowId, } from "../core/rows"; import type { TimelineRegionSpan, TimelineRenderItem } from "../core/timelineTypes"; +import { getKeptTimelineSpans } from "../../types"; export function getAnnotationLabel(region: AnnotationRegion): string { if (region.type === "text") { @@ -60,16 +61,22 @@ export function buildTimelineItems(params: { variant: "zoom", })); + const keptSpansById = new Map( + getKeptTimelineSpans(clipRegions).map((span) => [span.clip.id, span]), + ); const clips: TimelineRenderItem[] = clipRegions.map((region, index) => { const displayDurationMs = Math.max(0, region.endMs - region.startMs); const speed = Number.isFinite(region.speed) && region.speed > 0 ? region.speed : 1; const sourceEndMs = region.startMs + displayDurationMs * speed; const speedLabel = formatClipSpeedLabel(speed); + const keptSpan = keptSpansById.get(region.id); + const timelineStartMs = keptSpan?.timelineStartMs ?? region.startMs; + const timelineEndMs = keptSpan?.timelineEndMs ?? region.endMs; return { id: region.id, rowId: CLIP_ROW_ID, - span: { start: region.startMs, end: region.endMs }, + span: { start: timelineStartMs, end: timelineEndMs }, sourceSpan: { start: region.startMs, end: sourceEndMs }, label: speedLabel ? `Clip ${index + 1} ${speedLabel}` : `Clip ${index + 1}`, speedValue: speedLabel ? speed : undefined, diff --git a/src/components/video-editor/types.test.ts b/src/components/video-editor/types.test.ts index 34b3502bf..9f51e4665 100644 --- a/src/components/video-editor/types.test.ts +++ b/src/components/video-editor/types.test.ts @@ -114,35 +114,49 @@ describe("extendAutoFullTrackClip", () => { }); }); -describe("clip timeline mapping", () => { +describe("clip timeline mapping (ripple)", () => { const clips = [ { id: "clip-1", startMs: 0, endMs: 4_000, speed: 1 }, { id: "clip-2", startMs: 6_000, endMs: 8_000, speed: 2 }, ]; - it("maps kept timeline time into source time", () => { + it("compacts adjacent clips by removing the inter-clip source gap", () => { + // clip-1 display: 4_000ms, clip-2 display: (8_000 - 6_000) = 2_000ms + // (speed is embedded in endMs via getClipSourceEndMs, so display = end - start) + // total timeline length: 6_000ms — the source gap [4000, 6000] contributes nothing + expect(getTimelineDurationMs(clips, 10_000)).toBe(6_000); + }); + + it("maps kept timeline time into source time inside each kept span", () => { + // timeline [0, 4000] → source [0, 4000] at speed 1 expect(mapTimelineTimeToSourceTime(1_500, clips)).toBe(1_500); - expect(mapTimelineTimeToSourceTime(7_000, clips)).toBe(8_000); + // timeline [4000, 6000] → source [6000, 10000] at speed 2 + expect(mapTimelineTimeToSourceTime(5_000, clips)).toBe(8_000); }); - it("snaps timeline gaps to the nearest clip edge", () => { - expect(mapTimelineTimeToSourceTime(4_300, clips)).toBe(4_000); - expect(mapTimelineTimeToSourceTime(5_700, clips)).toBe(6_000); + it("clamps timeline positions that fall outside the compacted range", () => { + // far before first kept span → source origin of clip-1 + expect(mapTimelineTimeToSourceTime(-100, clips)).toBe(0); + // far after last kept span → source end of clip-2 (= 6000 + 4000 source ms) + expect(mapTimelineTimeToSourceTime(7_000, clips)).toBe(10_000); }); - it("maps kept source time back into timeline time", () => { + it("maps source time back into compacted timeline time", () => { expect(mapSourceTimeToTimelineTime(1_500, clips)).toBe(1_500); - expect(mapSourceTimeToTimelineTime(8_000, clips)).toBe(7_000); + expect(mapSourceTimeToTimelineTime(8_000, clips)).toBe(5_000); }); - it("snaps removed source gaps to the nearest kept boundary", () => { - expect(mapSourceTimeToTimelineTime(4_200, clips)).toBe(4_000); - expect(mapSourceTimeToTimelineTime(5_900, clips)).toBe(6_000); + it("clamps source positions inside removed gaps to the nearest kept boundary", () => { + // source [4000, 6000] is removed; both sides collapse onto the timeline splice at 4_000ms. + expect(mapSourceTimeToTimelineTime(4_100, clips)).toBe(4_000); + expect(mapSourceTimeToTimelineTime(5_900, clips)).toBe(4_000); }); it("finds clips only inside visible kept spans", () => { expect(findClipAtTimelineTime(500, clips)?.id).toBe("clip-1"); - expect(findClipAtTimelineTime(5_000, clips)).toBeNull(); + expect(findClipAtTimelineTime(5_000, clips)?.id).toBe("clip-2"); + // exactly on the right boundary of clip-2 → no clip owns the open end + expect(findClipAtTimelineTime(6_000, clips)).toBeNull(); }); it("derives the next clip id after converting trim gaps into clip ids", () => { @@ -174,9 +188,22 @@ describe("getTimelineDurationMs", () => { ).toBe(20_000); }); - it("keeps the source duration when speed edits make clips shorter", () => { + it("uses the display duration of clips, ignoring the source duration", () => { + // speed=2 halves the apparent length: display = 5_000ms regardless of source 10_000ms expect( getTimelineDurationMs([{ id: "clip-1", startMs: 0, endMs: 5_000, speed: 2 }], 10_000), - ).toBe(10_000); + ).toBe(5_000); + }); + + it("sums all clip display durations and ignores inter-clip source gaps", () => { + expect( + getTimelineDurationMs( + [ + { id: "clip-1", startMs: 0, endMs: 4_000, speed: 1 }, + { id: "clip-2", startMs: 6_000, endMs: 8_000, speed: 2 }, + ], + 10_000, + ), + ).toBe(6_000); }); }); diff --git a/src/components/video-editor/types.ts b/src/components/video-editor/types.ts index a66e1db4f..7f6c64d84 100644 --- a/src/components/video-editor/types.ts +++ b/src/components/video-editor/types.ts @@ -239,6 +239,46 @@ export function getClipSourceEndMs(clip: ClipRegion): number { return Math.round(clip.startMs + displayDurationMs * speed); } +/** + * Kept timeline segment for one clip. + * + * `clipRegions` are stored in SOURCE time (startMs = first kept source frame), + * while the editor timeline is a compacted view where removed ranges do not + * occupy space. `timelineStartMs`/`timelineEndMs` are the clip's bounds in that + * compacted timeline, and `sourceStartMs`/`sourceEndMs` are the matching source + * bounds. + */ +export interface KeptTimelineSpan { + clip: ClipRegion; + timelineStartMs: number; + timelineEndMs: number; + sourceStartMs: number; + sourceEndMs: number; + speed: number; +} + +export function getKeptTimelineSpans(clips: ClipRegion[]): KeptTimelineSpan[] { + const sortedClips = sortClipRegions(clips); + const spans: KeptTimelineSpan[] = []; + let cursorMs = 0; + + for (const clip of sortedClips) { + const displayDurationMs = Math.max(0, clip.endMs - clip.startMs); + const speed = getSafeClipSpeed(clip); + spans.push({ + clip, + timelineStartMs: cursorMs, + timelineEndMs: cursorMs + displayDurationMs, + sourceStartMs: clip.startMs, + sourceEndMs: getClipSourceEndMs(clip), + speed, + }); + cursorMs += displayDurationMs; + } + + return spans; +} + export function getTimelineDurationMs(clips: ClipRegion[], sourceDurationMs: number): number { const baseDurationMs = Math.max(0, Math.round(sourceDurationMs)); if (clips.length === 0) { @@ -246,11 +286,36 @@ export function getTimelineDurationMs(clips: ClipRegion[], sourceDurationMs: num } return clips.reduce( - (durationMs, clip) => Math.max(durationMs, Math.max(0, Math.round(clip.endMs))), - baseDurationMs, + (durationMs, clip) => durationMs + Math.max(0, Math.round(clip.endMs - clip.startMs)), + 0, ); } +export function getClipTimelineStartMs(clip: ClipRegion, clips: ClipRegion[]): number { + const sortedClips = sortClipRegions(clips); + let cursorMs = 0; + + for (const candidate of sortedClips) { + if (candidate.id === clip.id) { + return cursorMs; + } + cursorMs += Math.max(0, candidate.endMs - candidate.startMs); + } + + return cursorMs; +} + +/** + * Convert a compacted-timeline offset (ms) back into SOURCE time (ms). + * Used when writing user edits (split / resize) back into `clipRegions`. + */ +export function mapTimelineOffsetToSourceTime( + timelineMs: number, + clips: ClipRegion[], +): number { + return mapTimelineTimeToSourceTime(timelineMs, clips); +} + export function sortClipRegions(clips: ClipRegion[]): ClipRegion[] { return [...clips].sort((left, right) => left.startMs - right.startMs); } @@ -259,78 +324,112 @@ function getSafeClipSpeed(clip: ClipRegion) { return Number.isFinite(clip.speed) && clip.speed > 0 ? clip.speed : 1; } -function clampToNearestClipBoundary( - timeMs: number, - clips: ClipRegion[], - kind: "timeline" | "source", -) { - let nearestTimeMs = Math.round(timeMs); - let nearestDistance = Number.POSITIVE_INFINITY; - - for (const clip of clips) { - const boundaries = - kind === "timeline" - ? [clip.startMs, clip.endMs] - : [clip.startMs, getClipSourceEndMs(clip)]; - - for (const boundary of boundaries) { - const distance = Math.abs(timeMs - boundary); - if (distance < nearestDistance) { - nearestDistance = distance; - nearestTimeMs = Math.round(boundary); - } - } - } - - return nearestTimeMs; -} - export function mapTimelineTimeToSourceTime(timeMs: number, clips: ClipRegion[]): number { const roundedTimeMs = Math.round(timeMs); - const sortedClips = sortClipRegions(clips); + const spans = getKeptTimelineSpans(clips); - for (const clip of sortedClips) { - if (roundedTimeMs < clip.startMs || roundedTimeMs > clip.endMs) { + if (spans.length === 0) { + return roundedTimeMs; + } + + for (const span of spans) { + if (roundedTimeMs < span.timelineStartMs || roundedTimeMs > span.timelineEndMs) { continue; } - return Math.round(clip.startMs + (roundedTimeMs - clip.startMs) * getSafeClipSpeed(clip)); + return Math.round(span.sourceStartMs + (roundedTimeMs - span.timelineStartMs) * span.speed); } - if (sortedClips.length === 0) { - return roundedTimeMs; + const firstSpan = spans[0]; + const lastSpan = spans[spans.length - 1]; + + if (roundedTimeMs < firstSpan.timelineStartMs) { + return Math.round(firstSpan.sourceStartMs); + } + + if (roundedTimeMs > lastSpan.timelineEndMs) { + return Math.round(lastSpan.sourceEndMs); + } + + let previousSpan = firstSpan; + let nextSpan: KeptTimelineSpan | null = null; + for (const span of spans) { + if (span.timelineStartMs > roundedTimeMs) { + nextSpan = span; + break; + } + previousSpan = span; + } + + if (!nextSpan) { + return Math.round(previousSpan.sourceEndMs); } - return clampToNearestClipBoundary(roundedTimeMs, sortedClips, "timeline"); + const distanceToPrevious = roundedTimeMs - previousSpan.timelineEndMs; + const distanceToNext = nextSpan.timelineStartMs - roundedTimeMs; + + return distanceToPrevious <= distanceToNext + ? Math.round(previousSpan.sourceEndMs) + : Math.round(nextSpan.sourceStartMs); } export function mapSourceTimeToTimelineTime(timeMs: number, clips: ClipRegion[]): number { const roundedTimeMs = Math.round(timeMs); - const sortedClips = sortClipRegions(clips); + const spans = getKeptTimelineSpans(clips); - for (const clip of sortedClips) { - const sourceEndMs = getClipSourceEndMs(clip); - if (roundedTimeMs < clip.startMs || roundedTimeMs > sourceEndMs) { + if (spans.length === 0) { + return roundedTimeMs; + } + + for (const span of spans) { + if (roundedTimeMs < span.sourceStartMs || roundedTimeMs > span.sourceEndMs) { continue; } - return Math.round(clip.startMs + (roundedTimeMs - clip.startMs) / getSafeClipSpeed(clip)); + return Math.round(span.timelineStartMs + (roundedTimeMs - span.sourceStartMs) / span.speed); } - if (sortedClips.length === 0) { - return roundedTimeMs; + const firstSpan = spans[0]; + const lastSpan = spans[spans.length - 1]; + + if (roundedTimeMs < firstSpan.sourceStartMs) { + return Math.round(firstSpan.timelineStartMs); + } + + if (roundedTimeMs > lastSpan.sourceEndMs) { + return Math.round(lastSpan.timelineEndMs); } - return clampToNearestClipBoundary(roundedTimeMs, sortedClips, "source"); + let previousSpan = firstSpan; + let nextSpan: KeptTimelineSpan | null = null; + for (const span of spans) { + if (span.sourceStartMs > roundedTimeMs) { + nextSpan = span; + break; + } + previousSpan = span; + } + + if (!nextSpan) { + return Math.round(previousSpan.timelineEndMs); + } + + const distanceToPrevious = roundedTimeMs - previousSpan.sourceEndMs; + const distanceToNext = nextSpan.sourceStartMs - roundedTimeMs; + + return distanceToPrevious <= distanceToNext + ? Math.round(previousSpan.timelineEndMs) + : Math.round(nextSpan.timelineStartMs); } export function findClipAtTimelineTime(timeMs: number, clips: ClipRegion[]): ClipRegion | null { const roundedTimeMs = Math.round(timeMs); - return ( - sortClipRegions(clips).find( - (clip) => roundedTimeMs >= clip.startMs && roundedTimeMs < clip.endMs, - ) ?? null + const span = getKeptTimelineSpans(clips).find( + (candidate) => + roundedTimeMs >= candidate.timelineStartMs && roundedTimeMs < candidate.timelineEndMs, ); + + return span?.clip ?? null; } export function extendAutoFullTrackClip( From ae1ea8d998c3af44004ac5de1f6c03dc74e9d73a Mon Sep 17 00:00:00 2001 From: newHashub <37201917+newHashub@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:17:30 +0800 Subject: [PATCH 2/2] fix: address review feedback on timeline mapping and clip commands - mapTimelineTimeToSourceTime: use half-open span interval so a clip boundary maps to the next kept span, consistent with findClipAtTimelineTime - handleClipSplit: store left.endMs as display end (startMs + splitOffset) and use a safe speed for the source split point - handleClipDelete: filter zoom/annotation/speed/audio regions using the deleted clip's timeline bounds instead of its source coordinates --- .../video-editor/hooks/useClipRegionCommands.ts | 8 ++++++-- src/components/video-editor/types.test.ts | 7 +++++++ src/components/video-editor/types.ts | 4 +++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/components/video-editor/hooks/useClipRegionCommands.ts b/src/components/video-editor/hooks/useClipRegionCommands.ts index e28131eb6..0c41661ff 100644 --- a/src/components/video-editor/hooks/useClipRegionCommands.ts +++ b/src/components/video-editor/hooks/useClipRegionCommands.ts @@ -91,8 +91,12 @@ export function useClipRegionCommands({ const timelineStart = getClipTimelineStartMs(target, clipRegions); const splitAt = Math.round(splitMs); const splitOffset = splitAt - timelineStart; - const newSourceStart = Math.round(target.startMs + splitOffset * target.speed); - const left: ClipRegion = { ...target, id: leftId, endMs: newSourceStart }; + const safeSpeed = Number.isFinite(target.speed) && target.speed > 0 ? target.speed : 1; + const newSourceStart = Math.round(target.startMs + splitOffset * safeSpeed); + // ClipRegion.endMs is the display end (= startMs + display duration), not + // the source end, so the left clip keeps exactly `splitOffset` of display + // time rather than inheriting the source-derived split point. + const left: ClipRegion = { ...target, id: leftId, endMs: target.startMs + splitOffset }; const right: ClipRegion = { ...target, id: rightId, diff --git a/src/components/video-editor/types.test.ts b/src/components/video-editor/types.test.ts index 9f51e4665..d31bdcd61 100644 --- a/src/components/video-editor/types.test.ts +++ b/src/components/video-editor/types.test.ts @@ -134,6 +134,13 @@ describe("clip timeline mapping (ripple)", () => { expect(mapTimelineTimeToSourceTime(5_000, clips)).toBe(8_000); }); + it("maps a clip boundary into the next kept span", () => { + // timeline 4_000 is the splice between clip-1 and clip-2; it must map to + // the next clip's source origin (6_000), not clip-1's source end (4_000). + expect(mapTimelineTimeToSourceTime(4_000, clips)).toBe(6_000); + expect(findClipAtTimelineTime(4_000, clips)?.id).toBe("clip-2"); + }); + it("clamps timeline positions that fall outside the compacted range", () => { // far before first kept span → source origin of clip-1 expect(mapTimelineTimeToSourceTime(-100, clips)).toBe(0); diff --git a/src/components/video-editor/types.ts b/src/components/video-editor/types.ts index 7f6c64d84..b4424df11 100644 --- a/src/components/video-editor/types.ts +++ b/src/components/video-editor/types.ts @@ -333,7 +333,9 @@ export function mapTimelineTimeToSourceTime(timeMs: number, clips: ClipRegion[]) } for (const span of spans) { - if (roundedTimeMs < span.timelineStartMs || roundedTimeMs > span.timelineEndMs) { + // Half-open interval [timelineStartMs, timelineEndMs): a clip boundary + // belongs to the NEXT kept span, matching findClipAtTimelineTime. + if (roundedTimeMs < span.timelineStartMs || roundedTimeMs >= span.timelineEndMs) { continue; }