Merge pull request #57 from punkhop/fix/ripple-delete-split-edit

edit: ripple delete closes one gap for a clip with a split edit
This commit is contained in:
Brandon Thomas
2026-10-06 11:37:47 -04:00
committed by GitHub
5 changed files with 273 additions and 16 deletions
+57 -12
View File
@@ -351,21 +351,66 @@ pub fn delete_items(seq: &mut Sequence, items: &[ClipId]) -> usize {
n
}
/// Ripple delete track items: remove them and close the resulting gaps.
pub fn ripple_delete_items(seq: &mut Sequence, items: &[ClipId]) -> Result<()> {
// Collect per-track ranges to close (merged), processed right-to-left.
let mut ranges: Vec<(TrackId, TimeRange)> = Vec::new();
/// The stretches a ripple delete of `items` closes, in time order: the time the deleted items
/// covered, less whatever stays on the tracks that lose an item. Every such track then moves up
/// by the same amount, into time that is free on all of them, so later clips stay in sync and
/// nothing is overwritten. A clip and its linked sound with a split edit have different edges;
/// closing each one's own range would move the tracks twice. Locked tracks lose nothing and do
/// not count.
fn ripple_delete_spans(seq: &Sequence, items: &[ClipId]) -> Vec<TimeRange> {
let (mut deleted, mut kept) = (Vec::new(), Vec::new());
for tr in seq.all_tracks().filter(|t| !t.locked && t.items.iter().any(|i| items.contains(&i.id))) {
for i in &tr.items {
if items.contains(&i.id) { deleted.push(i.range()) } else { kept.push(i.range()) }
}
}
let kept = merged(kept);
let mut spans = Vec::new();
for r in merged(deleted) {
let mut at = r.start;
for k in kept.iter().filter(|k| k.overlaps(&r)) {
if k.start > at {
spans.push(TimeRange::from_bounds(at, k.start));
}
at = at.max(k.end());
}
if at < r.end() {
spans.push(TimeRange::from_bounds(at, r.end()));
}
}
spans
}
/// `ranges` in time order, with the ones that touch or overlap joined.
fn merged(mut ranges: Vec<TimeRange>) -> Vec<TimeRange> {
ranges.sort_by_key(|r| r.start);
let mut out: Vec<TimeRange> = Vec::new();
for r in ranges {
match out.last_mut() {
Some(last) if r.start <= last.end() => *last = TimeRange::from_bounds(last.start, last.end().max(r.end())),
_ => out.push(r),
}
}
out
}
/// Ripple delete track items: remove them and close the resulting gaps. Returns the stretches
/// that were closed, in time order. Refused when no gap can close (what stays on the items'
/// tracks covers all of their time): that would be a plain delete.
pub fn ripple_delete_items(seq: &mut Sequence, items: &[ClipId]) -> Result<Vec<TimeRange>> {
// Collect the tracks that lose an item; the gaps close right-to-left.
let mut affected: Vec<TrackId> = Vec::new();
for c in items {
let (tid, it) = seq.find_item(*c).ok_or(EditError::NoItem(*c))?;
ranges.push((tid, it.range()));
let (tid, _) = seq.find_item(*c).ok_or(EditError::NoItem(*c))?;
affected.push(tid);
}
let spans = ripple_delete_spans(seq, items);
if spans.is_empty() && !items.is_empty() {
let locked = affected.iter().all(|t| seq.track(*t).is_some_and(|tr| tr.locked));
return Err(if locked { EditError::Locked } else { EditError::Other("no gap can close: other clips on these tracks cover the deleted time".into()) });
}
// Group identical ranges across tracks (linked A/V) into one ripple.
let mut spans: Vec<TimeRange> = ranges.iter().map(|r| r.1).collect();
spans.sort_by_key(|r| (r.start, r.duration));
spans.dedup();
let mut work = seq.clone();
delete_items(&mut work, items);
let affected: Vec<TrackId> = ranges.iter().map(|r| r.0).collect();
for span in spans.iter().rev() {
// the gap actually closable: from span.start to the next material on affected tracks
for tr in work.all_tracks_mut() {
@@ -383,7 +428,7 @@ pub fn ripple_delete_items(seq: &mut Sequence, items: &[ClipId]) -> Result<()> {
}
}
*seq = work;
Ok(())
Ok(spans)
}
/// Close the gap containing `t` on a track (Ripple Delete on a gap).
+75
View File
@@ -148,6 +148,81 @@ fn ripple_delete_and_conflict() {
assert_eq!(fx.spans(v1), vec![(0, 10), (20, 10)], "unchanged on failure");
}
#[test]
fn ripple_delete_closes_the_time_free_on_every_track() {
// four clips on V1 with their sounds on A1; the second sound starts 2 early and ends 2 late
let build = || {
let mut fx = Fx::new();
let (v1, a1) = (fx.v(0), fx.a(0));
let v: Vec<ClipId> = [(0, 10), (10, 10), (20, 10), (30, 10)].iter().map(|(s, d)| fx.put(v1, *s, *d, 0)).collect();
let a: Vec<ClipId> = [(0, 8), (8, 14), (22, 8), (30, 10)].iter().map(|(s, d)| fx.put(a1, *s, *d, 0)).collect();
(fx, v, a)
};
let span = |start, len| TimeRange::new(f(start), f(len));
// one clip with a split edit on both sides: the picture's stretch closes, everything later stays in sync
let (mut fx, v, a) = build();
assert_eq!(ripple_delete_items(&mut fx.seq, &[v[1], a[1]]), Ok(vec![span(10, 10)]));
assert_eq!(fx.spans(fx.v(0)), vec![(0, 10), (10, 10), (20, 10)]);
assert_eq!(fx.spans(fx.a(0)), vec![(0, 8), (12, 8), (20, 10)]);
fx.seq.check().unwrap();
// two neighbours with a split edit between them close as one stretch
let (mut fx, v, a) = build();
assert_eq!(ripple_delete_items(&mut fx.seq, &[v[1], a[1], v[2], a[2]]), Ok(vec![span(10, 20)]));
assert_eq!(fx.spans(fx.v(0)), vec![(0, 10), (10, 10)]);
assert_eq!(fx.spans(fx.a(0)), vec![(0, 8), (10, 10)]);
// clips apart in time each close their own gap
let (mut fx, v, a) = build();
assert_eq!(ripple_delete_items(&mut fx.seq, &[v[0], a[0], v[3], a[3]]), Ok(vec![span(0, 8), span(30, 10)]));
fx.seq.check().unwrap();
// straight cuts with the sounds on alternating tracks: V1 [0,10) [10,20) [20,30), A1 [0,10) [20,30), A2 [10,20)
let mut fx = Fx::new();
let (v1, a1, a2) = (fx.v(0), fx.a(0), fx.a(1));
let v: Vec<ClipId> = [0, 10, 20].iter().map(|s| fx.put(v1, *s, 10, 0)).collect();
let (s0, s1) = (fx.put(a1, 0, 10, 0), fx.put(a2, 10, 10, 0));
fx.put(a1, 20, 10, 0);
assert_eq!(ripple_delete_items(&mut fx.seq, &[v[0], s0, v[1], s1]), Ok(vec![span(0, 20)]));
assert_eq!((fx.spans(v1), fx.spans(a1), fx.spans(a2)), (vec![(0, 10)], vec![(0, 10)], vec![]));
// a sound shorter than its picture with nothing round it: the whole picture closes
let mut fx = Fx::new();
let (v1, a1) = (fx.v(0), fx.a(0));
let (p, s) = (fx.put(v1, 10, 10, 0), fx.put(a1, 12, 6, 0));
fx.put(v1, 20, 10, 0);
fx.put(a1, 20, 10, 0);
assert_eq!(ripple_delete_items(&mut fx.seq, &[p, s]), Ok(vec![span(10, 10)]));
// with the neighbours' sound right up against it, only the sound's stretch can close
let mut fx = Fx::new();
let (v1, a1) = (fx.v(0), fx.a(0));
fx.put(a1, 0, 12, 0);
let (p, s) = (fx.put(v1, 10, 10, 0), fx.put(a1, 12, 6, 0));
fx.put(v1, 20, 10, 0);
fx.put(a1, 18, 12, 0);
assert_eq!(ripple_delete_items(&mut fx.seq, &[p, s]), Ok(vec![span(12, 6)]));
assert_eq!((fx.spans(v1), fx.spans(a1)), (vec![(14, 10)], vec![(0, 12), (12, 12)]));
// a linked sound on a locked track stays, and does not shrink the gap that closes
let mut fx = Fx::new();
let (v1, a1) = (fx.v(0), fx.a(0));
let (p, s) = (fx.put(v1, 10, 10, 0), fx.put(a1, 12, 6, 0));
fx.put(v1, 20, 10, 0);
fx.seq.track_mut(a1).unwrap().locked = true;
fx.seq.track_mut(a1).unwrap().sync_lock = false;
assert_eq!(ripple_delete_items(&mut fx.seq, &[p, s]), Ok(vec![span(10, 10)]));
assert_eq!((fx.spans(v1), fx.spans(a1)), (vec![(10, 10)], vec![(12, 6)]));
// nothing can close: what stays on each track covers the other track's deleted time. Refused, unchanged.
let mut fx = Fx::new();
let (v1, a1) = (fx.v(0), fx.a(0));
let x = fx.put(v1, 10, 10, 0);
fx.put(v1, 30, 10, 0);
fx.put(a1, 10, 10, 0);
let y = fx.put(a1, 30, 10, 0);
let before = fx.seq.clone();
assert!(matches!(ripple_delete_items(&mut fx.seq, &[x, y]), Err(EditError::Other(_))));
assert_eq!(fx.seq, before);
}
#[test]
fn close_gap_works() {
let mut fx = Fx::new();
+1 -4
View File
@@ -1038,10 +1038,7 @@ fn build() -> Vec<CommandSpec> {
}
let sel = with_links(s, &clips_p(s, p));
s.edit_sequence("Ripple Delete", |q, _, st| {
let mut spans: Vec<TimeRange> = sel.iter().filter_map(|c| q.find_item(*c).map(|(_, i)| i.range())).collect();
spans.sort_by_key(|r| (r.start, r.duration));
spans.dedup();
edit::ripple_delete_items(q, &sel)?;
let spans = edit::ripple_delete_items(q, &sel)?;
if st.ripple_sequence_markers {
for r in spans.iter().rev() {
crate::sequence_tools::ripple_markers(&mut q.markers, r.end(), -r.duration);
+2
View File
@@ -945,6 +945,8 @@ mod relink_tests;
#[cfg(test)]
mod remix_tests;
#[cfg(test)]
mod ripple_delete_tests;
#[cfg(test)]
mod scopes_tests;
#[cfg(test)]
mod sequence_inspect_tests;
+138
View File
@@ -0,0 +1,138 @@
//! `edit.rippleDelete` on a clip whose sound edge was split (a J or an L cut): the clip and its
//! sound go, and the later clips move up by one amount on every track.
use filmcraft_project::ClipId;
use filmcraft_time::Tick;
use serde_json::json;
use crate::Session;
fn range(s: &Session, c: ClipId) -> (Tick, Tick) {
s.active_sequence().unwrap().find_item(c).map(|(_, i)| (i.start, i.end())).unwrap()
}
/// Four 48-frame clips in a row, pictures on V1 and their linked sounds on A1: [(picture, sound); 4].
fn four_clips(s: &mut Session) -> [(ClipId, ClipId); 4] {
s.execute("file.openDemoProject", json!({})).unwrap();
s.execute("file.newSequence", json!({"name": "Split edits", "video": 1, "audio": 1})).unwrap();
let rate = s.sequence_rate();
let item = s.project.items.values().find(|i| i.has_video() && i.has_audio() && i.as_media().is_some()).map(|i| i.id).unwrap();
for n in 0..4 {
s.execute("timeline.place", json!({"item": item.0, "frame": 48 * n, "sourceIn": rate.tick_of(48).0, "duration": rate.tick_of(48).0})).unwrap();
}
let q = s.active_sequence().unwrap();
[0, 1, 2, 3].map(|n| (q.video_tracks[0].items[n].id, q.audio_tracks[0].items[n].id))
}
/// Move one sound edge alone, as a split edit does (Linked Selection off for the trim).
fn trim_sound(s: &mut Session, sound: ClipId, edge: &str, frames: i64) {
s.execute("sequence.linkedSelection", json!({"on": false})).unwrap();
s.execute("timeline.trim", json!({"clip": sound.0, "edge": edge, "mode": "regular", "deltaFrames": frames})).unwrap();
s.execute("sequence.linkedSelection", json!({"on": true})).unwrap();
}
/// The second clip's sound starts 12 frames early (a J cut). Ripple delete was refused: the picture
/// and the sound have different ranges, each range was closed on its own, and the clips after the
/// next one were moved up twice, onto it. With a single clip after it the delete went through and
/// a later marker moved twice.
#[test]
fn ripple_delete_of_a_j_cut_clip_closes_one_gap() {
let mut s = Session::default();
let [(_, sound1), (picture2, sound2), (picture3, sound3), (picture4, sound4)] = four_clips(&mut s);
let f = |n: i64| s.sequence_rate().tick_of(n);
let (f36, f48, f72, f96, f120, f144) = (f(36), f(48), f(72), f(96), f(120), f(144));
trim_sound(&mut s, sound1, "out", -12);
trim_sound(&mut s, sound2, "in", -12);
assert_eq!((range(&s, picture2).0, range(&s, sound2).0), (f48, f36), "the sound leads by 12 frames");
s.state.ripple_sequence_markers = true;
let marker = s.execute("markers.add", json!({"time": f120.0, "name": "Later"})).unwrap()["marker"].clone();
let before = s.project.clone();
s.execute("edit.rippleDelete", json!({"clips": [picture2.0]})).unwrap();
let q = s.active_sequence().unwrap();
q.check().unwrap();
assert!(q.find_item(picture2).is_none() && q.find_item(sound2).is_none(), "the clip and its sound are gone");
assert_eq!(range(&s, picture3), (f48, f96), "the next picture closes the gap");
assert_eq!(range(&s, sound3), (f48, f96), "and its sound moves up by the same amount");
assert_eq!((range(&s, picture4), range(&s, sound4)), ((f96, f144), (f96, f144)), "the clip after it moves up once");
assert_eq!(range(&s, sound1).1, f36, "the sound before is untouched");
assert_eq!(q.duration(), f144);
let at = q.markers.iter().find(|m| json!(m.id.0) == marker).map(|m| m.start);
assert_eq!(at, Some(f72), "a later marker moves up once, by the same amount");
let after = s.project.clone();
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(*s.project, *before, "one undo step restores it");
s.execute("edit.redo", json!({})).unwrap();
assert_eq!(*s.project, *after);
}
/// The second clip's sound runs 12 frames under the next picture (an L cut).
#[test]
fn ripple_delete_of_an_l_cut_clip_keeps_the_next_clip_in_sync() {
let mut s = Session::default();
let [_, (picture2, sound2), (picture3, sound3), (picture4, sound4)] = four_clips(&mut s);
let f = |n: i64| s.sequence_rate().tick_of(n);
let (f48, f60, f96, f108, f144) = (f(48), f(60), f(96), f(108), f(144));
trim_sound(&mut s, sound3, "in", 12);
trim_sound(&mut s, sound2, "out", 12);
assert_eq!((range(&s, picture2).1, range(&s, sound2).1), (f96, f108), "the sound runs on by 12 frames");
s.execute("edit.rippleDelete", json!({"clips": [picture2.0]})).unwrap();
let q = s.active_sequence().unwrap();
q.check().unwrap();
assert!(q.find_item(sound2).is_none());
assert_eq!(range(&s, picture3), (f48, f96));
assert_eq!(range(&s, sound3), (f60, f96), "the next sound still starts 12 frames after its picture");
assert_eq!((range(&s, picture4), range(&s, sound4)), ((f96, f144), (f96, f144)));
}
/// A straight cut is unchanged: the picture and its sound share one range and close one gap.
#[test]
fn ripple_delete_of_a_straight_cut_clip_is_unchanged() {
let mut s = Session::default();
let [_, (picture2, _), (picture3, sound3), _] = four_clips(&mut s);
let f = |n: i64| s.sequence_rate().tick_of(n);
let (f48, f96) = (f(48), f(96));
s.execute("edit.rippleDelete", json!({"clips": [picture2.0]})).unwrap();
assert_eq!((range(&s, picture3), range(&s, sound3)), ((f48, f96), (f48, f96)));
}
/// Two neighbours deleted together, with a split edit between them (the first one's sound runs 12
/// frames under the second picture): one gap closes, the size of both clips.
#[test]
fn ripple_delete_of_two_neighbours_with_a_split_edit_between_them() {
let mut s = Session::default();
let [_, (picture2, sound2), (picture3, sound3), (picture4, sound4)] = four_clips(&mut s);
let f = |n: i64| s.sequence_rate().tick_of(n);
let (f48, f96) = (f(48), f(96));
trim_sound(&mut s, sound3, "in", 12);
trim_sound(&mut s, sound2, "out", 12);
s.execute("edit.rippleDelete", json!({"clips": [picture2.0, picture3.0]})).unwrap();
let q = s.active_sequence().unwrap();
q.check().unwrap();
assert!([picture2, sound2, picture3, sound3].iter().all(|c| q.find_item(*c).is_none()));
assert_eq!((range(&s, picture4), range(&s, sound4)), ((f48, f96), (f48, f96)), "the clip after them closes the whole gap");
}
/// Straight cuts with the sounds on alternating audio tracks: deleting two neighbours closes the
/// whole gap, as it did before.
#[test]
fn ripple_delete_of_two_neighbours_with_sounds_on_alternating_tracks() {
let mut s = Session::default();
s.execute("file.openDemoProject", json!({})).unwrap();
s.execute("file.newSequence", json!({"name": "Alternating", "video": 1, "audio": 2})).unwrap();
let rate = s.sequence_rate();
let item = s.project.items.values().find(|i| i.has_video() && i.has_audio() && i.as_media().is_some()).map(|i| i.id).unwrap();
let mut pictures = Vec::new();
for n in 0..4 {
let track = if n % 2 == 0 { "A1" } else { "A2" };
let r = s.execute("timeline.place", json!({"item": item.0, "audioTrack": track, "frame": 48 * n, "duration": rate.tick_of(48).0})).unwrap();
pictures.push(ClipId(r["clips"][0].as_u64().unwrap()));
}
s.execute("edit.rippleDelete", json!({"clips": [pictures[1].0, pictures[2].0]})).unwrap();
let q = s.active_sequence().unwrap();
q.check().unwrap();
assert_eq!(range(&s, pictures[3]), (rate.tick_of(48), rate.tick_of(96)));
assert_eq!(q.audio_tracks[1].items.iter().map(|i| i.range()).collect::<Vec<_>>(), vec![q.video_tracks[0].items[1].range()], "its sound moved with it");
assert_eq!(q.duration(), rate.tick_of(96));
}