From daca62677836c591e35714cf72f464c2e57cd51b Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 6 Oct 2026 00:10:53 -0400 Subject: [PATCH] Fix fades outliving trimmed clips; property test of random edit sequences with full undo Co-Authored-By: Claude Opus 5.5 --- crates/engine/tests/random_edits.rs | 124 ++++++++++++++++++++++++++++ crates/model/src/clip.rs | 15 +++- 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 crates/engine/tests/random_edits.rs diff --git a/crates/engine/tests/random_edits.rs b/crates/engine/tests/random_edits.rs new file mode 100644 index 0000000..189f1a1 --- /dev/null +++ b/crates/engine/tests/random_edits.rs @@ -0,0 +1,124 @@ +//! Random command sequences against the demo session: nothing panics, invariants hold, and +//! undoing everything restores the original document. + +use proptest::prelude::*; +use serde_json::{Value, json}; +use soundcraft_engine::Engine; + +const CMDS: &[&str] = &[ + "edit.cut", + "edit.copy", + "edit.paste", + "edit.clear", + "edit.duplicate", + "edit.repeat", + "edit.insert_silence", + "edit.separate", + "edit.heal", + "edit.trim_to_selection", + "edit.fades_create", + "edit.fades_delete", + "edit.nudge", + "edit.consolidate", + "edit.strip_silence", + "edit.mute_clips", + "edit.move_clips", + "edit.mode", + "clip.gain", + "clip.loop", + "mix.volume", + "mix.pan", + "mix.insert", + "automation.set_point", + "automation.write_range", + "event.tempo", + "markers.add", + "track.new", + "track.duplicate", + "track.delete", +]; + +fn params(i: usize, a: i64, b: i64, track: &str) -> Value { + let (s, e) = (a.min(b), a.max(b)); + match CMDS.get(i).copied().unwrap_or("") { + "edit.mode" => { + let m = ["slip", "shuffle", "grid", "spot"][(a.unsigned_abs() % 4) as usize]; + json!({ "mode": m }) + } + "edit.repeat" => json!({"tracks": [track], "start": s, "end": e, "count": 2}), + "edit.nudge" => json!({"direction": if a % 2 == 0 { 1 } else { -1 }}), + "edit.move_clips" => json!({"by": b - a}), + "clip.gain" => json!({"delta_db": (a % 7) as f64}), + "clip.loop" => json!({"count": 2}), + "mix.volume" => json!({"track": track, "db": (a % 20) as f64 - 10.0}), + "mix.pan" => json!({"track": track, "pan": ((a % 200) as f64) / 100.0}), + "mix.insert" => json!({"track": track, "plugin": "eq_7band"}), + "automation.set_point" => json!({"track": track, "param": "volume", "at": s, "value": -6.0}), + "automation.write_range" => json!({"track": track, "param": "pan", "start": s, "end": e, "value": 0.5}), + "event.tempo" => json!({"bpm": 60.0 + (a.unsigned_abs() % 100) as f64, "at": s}), + "markers.add" => json!({"at": s}), + "track.new" => json!({"format": "Stereo"}), + "track.duplicate" | "track.delete" => json!({"tracks": [track]}), + _ => json!({"tracks": [track], "start": s, "end": e}), + } +} + +fn check_invariants(e: &Engine) -> Result<(), String> { + let s = e.session(); + for t in &s.tracks { + for pl in &t.playlists { + let mut last = i64::MIN; + for c in &pl.clips { + if c.length <= 0 { + return Err(format!("{}: clip {} has length {}", t.name, c.id, c.length)); + } + if c.start < last { + return Err(format!("{}: playlist not sorted", t.name)); + } + last = c.start; + if c.fade_in.len > c.length || c.fade_out.len > c.length { + return Err(format!("{}: fade longer than clip", t.name)); + } + } + } + if !t.mixer.volume_db.is_finite() { + return Err("non-finite volume".into()); + } + } + serde_json::to_string(s).map(|_| ()).map_err(|err| err.to_string()) +} + +proptest! { + #![proptest_config(ProptestConfig { cases: 48, .. ProptestConfig::default() })] + #[test] + fn random_edits_keep_the_session_sane(ops in proptest::collection::vec((0usize..30, -100_000i64..3_000_000, -100_000i64..3_000_000, 0usize..5), 1..25)) { + let mut e = soundcraft_engine::demo::demo_engine(); + let original = serde_json::to_value(e.session()).unwrap(); + let names = ["Kick", "Snare", "Bass", "Pad", "Keys"]; + let mut applied = 0; + for (i, a, b, t) in ops { + let track = names[t % names.len()]; + let id = CMDS[i % CMDS.len()]; + // Make a selection first so range commands have something to act on. + let _ = e.execute("edit.select", &json!({"tracks": [track], "start": a.min(b).max(0), "end": a.max(b).max(0)})); + if let Err(soundcraft_engine::EngineError::Internal(id, msg)) = e.execute(id, ¶ms(i % CMDS.len(), a, b, track)) { + prop_assert!(false, "{id} panicked: {msg}"); + } + applied += 1; + prop_assert!(check_invariants(&e).is_ok(), "after {id}: {:?}", check_invariants(&e)); + } + prop_assert!(applied > 0); + while e.can_undo() { + e.undo(); + } + let mut back = serde_json::to_value(e.session()).unwrap(); + let mut orig = original; + // Selection and view state are not part of the undo history. + for v in [&mut back, &mut orig] { + if let Some(o) = v.as_object_mut() { + o.remove("edit"); + } + } + prop_assert_eq!(back, orig); + } +} diff --git a/crates/model/src/clip.rs b/crates/model/src/clip.rs index d915530..046b3df 100644 --- a/crates/model/src/clip.rs +++ b/crates/model/src/clip.rs @@ -190,7 +190,7 @@ impl Clip { // MIDI note ticks are re-anchored by the engine, which knows the tempo map. self.length -= delta; self.start = new_start; - self.fade_in.len = self.fade_in.len.min(self.length); + self.clamp_fades(); self.gain_env = self.gain_env.iter().filter_map(|&(t, v)| (t - delta >= 0).then_some((t - delta, v))).collect(); } @@ -198,7 +198,13 @@ impl Clip { pub fn trim_end_to(&mut self, new_end: Samples) { let new_end = new_end.max(self.start + 1); self.length = new_end - self.start; - self.fade_out.len = self.fade_out.len.min(self.length); + self.clamp_fades(); + } + + /// Keep both fades within the clip. + pub fn clamp_fades(&mut self) { + self.fade_in.len = self.fade_in.len.clamp(0, self.length); + self.fade_out.len = self.fade_out.len.clamp(0, self.length); } } @@ -261,5 +267,10 @@ mod tests { assert_eq!(c.length, 200); c.trim_end_to(-50); assert_eq!(c.length, 1); + // Regression: trimming the start must not leave a fade-out longer than the clip. + let mut c = Clip::audio(ClipId(2), "b", SourceId(1), 0, 0, 1000); + c.fade_out = Fade { len: 800, shape: FadeShape::Linear }; + c.trim_start_to(900); + assert!(c.fade_out.len <= c.length); } }