From 7bf80663dc81b4d50cd90b584d3dd334578a68b2 Mon Sep 17 00:00:00 2001 From: Diego Lopes Date: Wed, 14 Oct 2026 13:45:13 -0400 Subject: [PATCH 01/38] Fix resampler edge read at device/session rate mismatch The output resampler interpolated toward a stale sample at the end of every mix block when the device rate differs from the session rate. It now holds the last valid sample. Co-Authored-By: Claude Sonnet 5.5 --- crates/playback/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index b945d4b6..fc3b6a4e 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -362,7 +362,9 @@ impl AudioState { let sample = |c: usize| { let ch = self.pending.get(c); let a = ch.and_then(|v| v.get(i0)).copied().unwrap_or(0.0); - let b = ch.and_then(|v| v.get(i0 + 1)).copied().unwrap_or(a); + // Holds the last sample at the block edge (`pending` is BLOCK long, + // only `pending_len` is valid); carry the next block's first sample if audible. + let b = if i0 + 1 < self.pending_len { ch.and_then(|v| v.get(i0 + 1)).copied().unwrap_or(a) } else { a }; a + (b - a) * fr }; if nch > 2 && channels < nch { From 5f03a5991e1d970263d52dd3a31e11468895edb8 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 03:08:35 +0200 Subject: [PATCH 02/38] Plugin-only editor changes mark the session modified (fixes #455) --- crates/clap-host/src/plugin.rs | 5 +- crates/dsp/src/lib.rs | 5 ++ crates/engine/src/cmd/plugin_state.rs | 23 ++++++++- crates/engine/src/lib.rs | 5 ++ crates/playback/src/lib.rs | 69 ++++++++++++++++++++++++++- crates/ui-egui/src/lib.rs | 44 +++++++++++++++-- 6 files changed, 145 insertions(+), 6 deletions(-) diff --git a/crates/clap-host/src/plugin.rs b/crates/clap-host/src/plugin.rs index 89f8dbf8..4b9aac1e 100644 --- a/crates/clap-host/src/plugin.rs +++ b/crates/clap-host/src/plugin.rs @@ -327,7 +327,6 @@ impl PluginEditor for ClapEditor { fn idle(&mut self) -> Vec<(String, f32)> { let raw = self.link.idle(); - let _ = self.link.host().dirty.swap(false, Ordering::Relaxed); let mut out: Vec<(String, f32)> = Vec::new(); for (cid, v) in raw { let Some(i) = self.ids.iter().position(|&x| x == cid) else { continue }; @@ -341,4 +340,8 @@ impl PluginEditor for ClapEditor { } out } + + fn state_changed(&mut self) -> bool { + self.link.host().dirty.swap(false, Ordering::Relaxed) + } } diff --git a/crates/dsp/src/lib.rs b/crates/dsp/src/lib.rs index d31ddac2..20ea8b37 100644 --- a/crates/dsp/src/lib.rs +++ b/crates/dsp/src/lib.rs @@ -272,6 +272,11 @@ pub trait PluginEditor: Send { /// user closed, and returns parameter edits made in the editor as `(param id, value)` in /// SoundCraft units (ids and ranges of [`PluginInfo::params`]). fn idle(&mut self) -> Vec<(String, f32)>; + /// True once after the plugin said its state changed in a way its parameters don't show + /// (CLAP `state.mark_dirty`), so the session stores the new state. + fn state_changed(&mut self) -> bool { + false + } /// Tells the editor that a parameter changed elsewhere (generic editor, automation), so the /// GUI shows it. Values are in SoundCraft units. fn set_param(&mut self, _id: &str, _value: f32) {} diff --git a/crates/engine/src/cmd/plugin_state.rs b/crates/engine/src/cmd/plugin_state.rs index e999c863..e12000bf 100644 --- a/crates/engine/src/cmd/plugin_state.rs +++ b/crates/engine/src/cmd/plugin_state.rs @@ -47,6 +47,15 @@ fn insert_state(e: &mut Engine, p: &Value) -> Result { Some(Value::String(s)) => Some(b64::decode(s).ok_or_else(|| bad(CMD, "`state` is not valid base64 (or is too large)"))?), Some(_) => return Err(bad(CMD, "`state` must be a base64 string or null")), }; + let n = state.as_ref().map_or(0, Vec::len); + // Plugins report changes that leave their state as it was: don't modify the session for those. + let stored = e.session().track(t).and_then(|tr| match slot { + Some(i) => tr.mixer.inserts.get(i).and_then(Option::as_ref), + None => tr.instrument.as_ref(), + }); + if stored.is_some_and(|ins| ins.state_bytes().as_deref() == state.as_deref().filter(|b| !b.is_empty())) { + return Ok(json!({"bytes": n})); + } let s = e.session_mut(); let tr = s.track_mut(t).ok_or_else(|| bad(CMD, "no track"))?; let ins = match slot { @@ -54,7 +63,6 @@ fn insert_state(e: &mut Engine, p: &Value) -> Result { None => tr.instrument.as_mut(), } .ok_or_else(|| bad(CMD, "empty slot"))?; - let n = state.as_ref().map_or(0, Vec::len); match state { Some(bytes) => ins.set_state_bytes(&bytes), None => ins.state = None, @@ -96,6 +104,19 @@ mod tests { assert_eq!(e.session().tracks[0].mixer.inserts[2].as_ref().unwrap().state, None); } + #[test] + fn storing_the_same_state_again_changes_nothing() { + let mut e = Engine::default(); + e.execute("track.new", &json!({})).unwrap(); + let t = e.session().tracks[0].id.0; + e.execute("mix.insert", &json!({"track": t, "plugin": "gain", "slot": 0})).unwrap(); + let blob = b64::encode(b"same"); + e.execute("mix.insert_state", &json!({"track": t, "slot": 0, "state": blob})).unwrap(); + let rev = e.revision; + e.execute("mix.insert_state", &json!({"track": t, "slot": 0, "state": blob})).unwrap(); + assert_eq!(e.revision, rev); + } + #[test] fn hostile_params_are_errors() { let mut e = Engine::default(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6cc1c90a..c77b3485 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -386,6 +386,11 @@ impl Engine { } } + /// The gesture `execute_merged` is continuing, if any. + pub fn merge_key(&self) -> Option<&str> { + self.merge_key.as_deref() + } + /// End a gesture started with `execute_merged`. pub fn end_merge(&mut self) { self.merge_key = None; diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index b945d4b6..5cd8e84a 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -673,6 +673,12 @@ impl Player { self.side().capture_states() } + /// Reads the state of the plugins in `slots` and of those that said their state changed since + /// the last call, so the session keeps changes made only in plugin editors. + pub fn capture_changed_states(&self, slots: &[(TrackId, usize)]) -> PluginStates { + self.side().capture_changed_states(slots) + } + /// Per-UI-frame housekeeping: drops retired instances and services plugin editors. Returns /// the parameter edits made in open editors since the last call. pub fn idle(&self) -> Vec { @@ -795,6 +801,21 @@ impl Side { self.states.iter_mut().filter(|(k, _)| !self.refused.contains(*k)).filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))).collect() } + fn capture_changed_states(&mut self, slots: &[(TrackId, usize)]) -> PluginStates { + let changed: Vec<(TrackId, usize)> = self.editors.iter_mut().filter_map(|(k, e)| e.state_changed().then_some(*k)).collect(); + self.states + .iter_mut() + .filter(|(k, _)| !self.refused.contains(*k) && (slots.contains(k) || changed.contains(k))) + .filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))) + .collect() + } + + /// A plugin that marks its state changed while it loads or restores it hasn't changed. + fn add_editor(&mut self, key: (TrackId, usize), mut e: Box) { + let _ = e.state_changed(); + self.editors.insert(key, e); + } + fn open_editor(&mut self, key: (TrackId, usize)) -> Result<(), String> { let e = self.editors.get_mut(&key).ok_or_else(|| "this plugin has no editor (or it is not loaded)".to_string())?; e.open()?; @@ -831,7 +852,7 @@ impl Side { self.shadow.insert(key, (spec.id.clone(), spec.ch, made.is_some())); if let Some(soundcraft_mix::NewInstance { mut plugin, refused_state }) = made { if let Some(e) = plugin.editor() { - self.editors.insert(key, e); + self.add_editor(key, e); } if let Some(r) = plugin.state_reader() { self.states.insert(key, r); @@ -1174,6 +1195,52 @@ mod tests { assert_eq!(side.capture_states(), vec![(TrackId(1), 0, b"defaults".to_vec())]); } + /// An editor whose plugin said its state changed (CLAP `state.mark_dirty`). + struct Dirty(bool); + + impl PluginEditor for Dirty { + fn open(&mut self) -> Result<(), String> { + Ok(()) + } + fn close(&mut self) {} + fn is_open(&self) -> bool { + true + } + fn idle(&mut self) -> Vec<(String, f32)> { + Vec::new() + } + fn state_changed(&mut self) -> bool { + std::mem::take(&mut self.0) + } + } + + #[test] + fn changed_states_are_read_once_for_the_session() { + let mut side = Side::new(48_000.0, channel().1); + let (a, b) = (TrackId(1), TrackId(2)); + let synth = (a, soundcraft_mix::INSTRUMENT_SLOT); + side.states.insert((a, 0), Box::new(|| Some(b"a0".to_vec()))); + side.states.insert((a, 1), Box::new(|| Some(b"a1".to_vec()))); + side.states.insert(synth, Box::new(|| Some(b"synth".to_vec()))); + side.states.insert((b, 0), Box::new(|| Some(b"b0".to_vec()))); + side.editors.insert((a, 0), Box::new(Dirty(true))); + side.editors.insert((a, 1), Box::new(Dirty(false))); + side.editors.insert((b, 0), Box::new(Dirty(true))); + side.refused.insert((b, 0)); + let want = vec![(a, 0, b"a0".to_vec()), (a, soundcraft_mix::INSTRUMENT_SLOT, b"synth".to_vec())]; + assert_eq!(side.capture_changed_states(&[synth]), want); + assert!(side.capture_changed_states(&[]).is_empty(), "each change is read once"); + } + + #[test] + fn a_state_marked_changed_while_loading_is_not_a_change() { + let mut side = Side::new(48_000.0, channel().1); + let k = (TrackId(1), 0); + side.states.insert(k, Box::new(|| Some(b"loaded".to_vec()))); + side.add_editor(k, Box::new(Dirty(true))); + assert!(side.capture_changed_states(&[]).is_empty()); + } + #[test] fn side_creates_each_missing_instance_once() { use soundcraft_model::{ChannelFormat, Insert, TrackKind}; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ddd54275..c147840a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -608,25 +608,45 @@ impl SoundApp { /// (`mix.insert_state`), so saves and bounces include it. pub fn capture_plugin_states(&mut self) { let Some(p) = &self.player else { return }; - for (t, slot, data) in p.capture_states() { + let states = p.capture_states(); + self.store_plugin_states(states); + } + + fn store_plugin_states(&mut self, states: soundcraft_playback::PluginStates) { + for (t, slot, data) in states { let slot = if slot == soundcraft_mix::INSTRUMENT_SLOT { json!("instrument") } else { json!(slot) }; - let state = soundcraft_model::b64::encode(&data); - if let Err(e) = self.engine.execute("mix.insert_state", &json!({"track": t.0, "slot": slot, "state": state})) { + let params = json!({"track": t.0, "slot": slot, "state": soundcraft_model::b64::encode(&data)}); + // A state read while a knob is dragged (or a pass recorded) joins that gesture, so it + // stays one undo step. + let stored = match self.engine.merge_key().map(str::to_string) { + Some(key) => self.engine.execute_merged("mix.insert_state", ¶ms, &key), + None => self.engine.execute("mix.insert_state", ¶ms), + }; + if let Err(e) = stored { log::warn!("storing plugin state: {e}"); } } } /// Parameter edits made in plugin editors go into the session (one undo step per gesture). + /// Hosted instruments keep their parameters in their own state, which the session takes + /// instead, as it does for plugins that said their state changed: either way the session is + /// modified, so it is autosaved and asks to be saved. fn apply_editor_edits(&mut self) { let Some(p) = &self.player else { return }; + let mut instruments = Vec::new(); for (t, slot, param, value) in p.idle() { if slot == soundcraft_mix::INSTRUMENT_SLOT { + if !instruments.contains(&(t, slot)) { + instruments.push((t, slot)); + } continue; } let key = format!("param:{}:{slot}:{param}", t.0); let _ = self.engine.execute_merged("mix.insert_param", &json!({"track": t.0, "slot": slot, "param": param, "value": value}), &key); } + let states = p.capture_changed_states(&instruments); + self.store_plugin_states(states); } /// Every two minutes, copy a modified session to the autosave folder (not while recording). @@ -1500,6 +1520,24 @@ mod tests { assert_eq!(app.engine.undo_history().len(), steps + 1); } + #[test] + fn plugin_states_read_during_a_drag_keep_it_one_undo_step() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let kick = app.engine.session().track_by_name("Kick").unwrap().id; + app.run("mix.insert", json!({"track": kick.0, "plugin": "gain", "slot": 0})).unwrap(); + let steps = app.engine.undo_history().len(); + for frame in 0..60u8 { + // What a plugin editor reports each frame of a drag, and the state the plugin marked dirty. + let gain = -10.0 + f32::from(frame) / 10.0; + app.engine + .execute_merged("mix.insert_param", &json!({"track": kick.0, "slot": 0, "param": "gain", "value": gain}), "param:drag") + .unwrap(); + app.store_plugin_states(vec![(kick, 0, vec![frame, 1])]); + } + assert_eq!(app.engine.undo_history().len(), steps + 1); + assert_eq!(app.engine.session().track(kick).unwrap().mixer.inserts[0].as_ref().unwrap().state_bytes(), Some(vec![59, 1])); + } + #[test] fn preferences_saved_before_the_window_was_remembered_keep_full_scale() { let ui: super::UiState = serde_json::from_value(json!({"show_transport": true})).unwrap(); From 278b41b8ff891ccb88b3eeae3cebfe3dc218402a Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 19 Oct 2026 08:18:28 +0000 Subject: [PATCH 03/38] Heal separations left by a cut, not only splits Fixes #772 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/edit.rs | 43 +++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/edit.rs b/crates/engine/src/edit.rs index 5531953d..4659652c 100644 --- a/crates/engine/src/edit.rs +++ b/crates/engine/src/edit.rs @@ -405,7 +405,8 @@ pub fn trim_end_to(s: &mut Session, track: TrackId, at: Samples) -> usize { n } -/// Heal separations inside `range`: merge neighbouring audio clips that continue the same file. +/// Heal separations inside `range`: merge neighbouring audio clips that continue the same file, +/// either touching (a split) or with an empty gap that matches the gap in the source (a cut). pub fn heal(s: &mut Session, track: TrackId, range: Range) -> usize { let Some(pl) = s.track_mut(track).and_then(|t| t.playlist_mut()) else { return 0 }; pl.sort(); @@ -413,13 +414,16 @@ pub fn heal(s: &mut Session, track: TrackId, range: Range) -> usize { let mut healed = 0; for c in pl.clips.drain(..) { if let Some(prev) = out.last_mut() { - let contiguous = prev.end() == c.start && c.start >= range.start && c.start <= range.end; + let contiguous = c.start >= prev.end() && prev.end() >= range.start && c.start <= range.end; + let gap = if contiguous { c.start.saturating_sub(prev.end()) } else { 0 }; let same = match (&prev.content, &c.content) { - (ClipContent::Audio { source: a, offset: oa }, ClipContent::Audio { source: b, offset: ob }) => a == b && oa + prev.length == *ob, + (ClipContent::Audio { source: a, offset: oa }, ClipContent::Audio { source: b, offset: ob }) => { + a == b && oa.saturating_add(prev.length).saturating_add(gap) == *ob + } _ => false, }; if contiguous && same && (prev.gain_db - c.gain_db).abs() < 1e-6 { - prev.length += c.length; + prev.length = c.end().saturating_sub(prev.start); prev.fade_out = c.fade_out; healed += 1; continue; @@ -878,6 +882,37 @@ mod tests { assert_eq!(clips(&s, t), vec![(1000, 4000, 1000)]); } + #[test] + fn heal_rejoins_cut_separation() { + let (mut s, t) = session_with_clip(); + clear_range(&mut s, t, Range::new(2000, 3000), false); + assert_eq!(heal(&mut s, t, Range::new(1500, 3500)), 1); + assert_eq!(clips(&s, t), vec![(1000, 4000, 1000)]); + // A selection that does not span the whole gap leaves it alone. + let (mut s, t) = session_with_clip(); + clear_range(&mut s, t, Range::new(2000, 3000), false); + assert_eq!(heal(&mut s, t, Range::new(2500, 3500)), 0); + // Shuffled pieces no longer line up with their source and stay apart. + let (mut s, t) = session_with_clip(); + clear_range(&mut s, t, Range::new(2000, 3000), true); + assert_eq!(heal(&mut s, t, Range::new(0, 10_000)), 0); + // Moved apart: the timeline gap no longer matches the source gap. + let (mut s, t) = session_with_clip(); + clear_range(&mut s, t, Range::new(2000, 3000), false); + if let Some(pl) = s.track_mut(t).and_then(|t| t.playlist_mut()) { + pl.clips[1].start = 3500; + } + assert_eq!(heal(&mut s, t, Range::new(0, 10_000)), 0); + // Overlapping neighbours whose offsets line up are not a separation. + let (mut s, t) = session_with_clip(); + assert_eq!(separate_at(&mut s, t, 3000), 1); + if let Some(pl) = s.track_mut(t).and_then(|t| t.playlist_mut()) { + pl.clips[1].start = 2500; + } + assert_eq!(heal(&mut s, t, Range::new(0, 10_000)), 0); + assert_eq!(clips(&s, t).len(), 2); + } + #[test] fn clear_slip_and_shuffle() { let (mut s, t) = session_with_clip(); From dfde8a672474331051084510ea81b70a9d68592c Mon Sep 17 00:00:00 2001 From: Diego Lopes Date: Mon, 19 Oct 2026 23:36:00 -0400 Subject: [PATCH 04/38] Add regression test for the resampler block edge Render a long DC block, then short blocks of silence with the device rate below the session rate. The interpolation must not read the stale frame past pending_len. Also correct the comment on the edge hold. Co-Authored-By: Claude Opus 5.5 --- crates/playback/src/lib.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index f5023feb..92823286 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -362,8 +362,7 @@ impl AudioState { let sample = |c: usize| { let ch = self.pending.get(c); let a = ch.and_then(|v| v.get(i0)).copied().unwrap_or(0.0); - // Holds the last sample at the block edge (`pending` is BLOCK long, - // only `pending_len` is valid); carry the next block's first sample if audible. + // `pending` is BLOCK long; only `pending_len` frames are valid. Hold the last one at the edge. let b = if i0 + 1 < self.pending_len { ch.and_then(|v| v.get(i0 + 1)).copied().unwrap_or(a) } else { a }; a + (b - a) * fr }; @@ -1216,6 +1215,26 @@ mod tests { } } + #[test] + fn resampling_at_a_short_block_edge_does_not_mix_in_stale_frames() { + use soundcraft_model::ChannelFormat; + // DC for the first long block, then silence. The device runs slower than the session, so + // reads land between frames; a short block leaves the long block's DC past `pending_len`. + let mut st = audio_output_state(ChannelFormat::Stereo, vec![vec![0.8; BLOCK]; 2]); + st.device_rate = 44_100.0; + let mut long = vec![0.0; 2 * 470]; + st.fill(&mut long, 2); + assert_eq!(st.pending_len, BLOCK); + let mut short = [0.0; 2 * 8]; + for call in 0..20 { + st.fill(&mut short, 2); + assert!(st.pending_len < BLOCK); + // The first frame of the first short call still reads the DC block's last frame. + let from = if call == 0 { 2 } else { 0 }; + assert!(short[from..].iter().all(|x| x.abs() < 1e-6), "call {call}: {short:?}"); + } + } + #[test] fn mono_device_preserves_centered_mono_level_and_includes_surround_right_channels() { use soundcraft_model::ChannelFormat; From 09c2e71141f22172ea5bbd514a20999d65522cbd Mon Sep 17 00:00:00 2001 From: mrrobbiec Date: Tue, 20 Oct 2026 16:01:25 +1000 Subject: [PATCH 05/38] Edit window: draw large edited sessions without per-clip work every frame On a 63,424-clip drum edit the UI thread spent ~187 ms per frame, so with a plugin editor open (a frame every 30 ms) it never went idle and the editor windows, which Windows only paints when the thread's queue is empty, stopped redrawing. Now ~11 ms of CPU per frame: - Overlap shadows: each drawn clip checked every clip of its track; on a sorted track the clips starting inside it are found by binary search (sortedness is checked once per track and frame, and an unsorted track is still scanned). - Fade curves: a fade took at least 65 points, even under a pixel wide. A narrow one takes ~4 * its width in physical pixels; the chord error stays under a quarter pixel (tested). - Clip List: rows are listed and filtered once per document revision and filter text, and only the rows in view are laid out (show_rows). - Universe: each track's clips are merged into same-colour runs covering exactly the same pixels, cached per revision and area (5,000 clips become a handful of rects). - Session::content_end, which walks every clip, is worked out once per revision. - Alternate playlist lanes skip clips outside the view, like the main lane, and clip names are not laid out where no glyph would show. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/edit_window.rs | 210 ++++++++++++++++++++++++++---- crates/ui-egui/src/panels.rs | 106 ++++++++++++--- 2 files changed, 274 insertions(+), 42 deletions(-) diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index 7e24a4c7..95d3ce86 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -115,26 +115,81 @@ pub fn show(app: &mut SoundApp, ui: &mut Ui) { crate::docking::show(app, ui, crate::MainWindow::Edit); } -/// Universe: the whole session in miniature; the box is the visible area (drag or click to move). -fn universe(app: &mut SoundApp, ui: &mut Ui) { - let r = ui.max_rect().shrink2(vec2(8.0, 4.0)); - let s = app.engine.session(); +/// The Universe's clip blocks for one document revision and area: each track's clips merged into +/// runs of one colour. A drum edit holds tens of thousands of clips a pixel or less wide here; +/// painting one rect per clip every frame would cost more than all the rest of the window. +#[derive(Clone)] +struct UniverseRuns { + revision: u64, + area: Rect, + runs: std::sync::Arc>, +} + +/// Each clip covers `[x0, max(x1, x0 + 1)]`; neighbours of one colour that touch or overlap are +/// painted as one rect, which covers exactly the same pixels in the same order. +fn universe_runs(s: &Session, r: Rect) -> Vec<(Rect, Color32)> { let total = (s.content_end() + s.sample_rate.samples(10.0)).max(1) as f32; let tracks: Vec<&Track> = s.tracks.iter().filter(|t| !t.hidden).collect(); let n = tracks.len().max(1) as f32; let rh = (r.height() / n).max(1.0); + let mut runs = Vec::new(); for (i, t) in tracks.iter().enumerate() { let y = r.min.y + i as f32 * rh; + let block = |x0: f32, x1: f32, color| (Rect::from_min_max(pos2(x0, y + 0.5), pos2(x1, y + rh - 0.5)), color); + let mut run: Option<(f32, f32, Color32)> = None; for c in t.clips() { let x0 = r.min.x + c.start as f32 / total * r.width(); - let x1 = r.min.x + c.end() as f32 / total * r.width(); - ui.painter().rect_filled( - Rect::from_min_max(pos2(x0, y + 0.5), pos2(x1.max(x0 + 1.0), y + rh - 0.5)), - 0.0, - rgb(c.color.unwrap_or(t.color)), - ); + let x1 = (r.min.x + c.end() as f32 / total * r.width()).max(x0 + 1.0); + let color = rgb(c.color.unwrap_or(t.color)); + run = match run { + Some((a, b, col)) if col == color && x0 >= a && x0 <= b => Some((a, b.max(x1), col)), + Some((a, b, col)) => { + runs.push(block(a, b, col)); + Some((x0, x1, color)) + } + None => Some((x0, x1, color)), + }; + } + if let Some((a, b, col)) = run { + runs.push(block(a, b, col)); } } + runs +} + +/// [`Session::content_end`], worked out once per document revision: it walks every clip. +fn content_end(app: &SoundApp, ctx: &egui::Context) -> Samples { + let key = egui::Id::new("content_end"); + let revision = app.engine.revision; + if let Some((at, end)) = ctx.memory(|m| m.data.get_temp::<(u64, Samples)>(key)) + && at == revision + { + return end; + } + let end = app.engine.session().content_end(); + ctx.memory_mut(|m| m.data.insert_temp(key, (revision, end))); + end +} + +/// Universe: the whole session in miniature; the box is the visible area (drag or click to move). +fn universe(app: &mut SoundApp, ui: &mut Ui) { + let r = ui.max_rect().shrink2(vec2(8.0, 4.0)); + let key = egui::Id::new("universe_runs"); + let revision = app.engine.revision; + let runs = match ui.ctx().memory(|m| m.data.get_temp::(key)).filter(|u| u.revision == revision && u.area == r) { + Some(u) => u.runs, + None => { + let runs = std::sync::Arc::new(universe_runs(app.engine.session(), r)); + ui.ctx().memory_mut(|m| m.data.insert_temp(key, UniverseRuns { revision, area: r, runs: runs.clone() })); + runs + } + }; + for &(rect, color) in runs.iter() { + ui.painter().rect_filled(rect, 0.0, color); + } + let end = content_end(app, ui.ctx()); + let s = app.engine.session(); + let total = (end + s.sample_rate.samples(10.0)).max(1) as f32; let vis0 = s.edit.zoom.scroll as f32 / total; let tlw = app.edit_layout.timeline[2] - app.edit_layout.timeline[0]; let vis1 = vis0 + (f64::from(tlw.max(100.0)) * s.edit.zoom.samples_per_px) as f32 / total; @@ -373,9 +428,10 @@ fn pan(app: &mut SoundApp, px: f64) { fn hscrollbar(app: &mut SoundApp, ui: &mut Ui, sb: Rect, tl: Rect) { let t = Tokens::current(); ui.painter().rect_filled(sb, 0.0, t.panel_bg); + let end = content_end(app, ui.ctx()); let s = app.engine.session(); let vis = f64::from(tl.width()) * s.edit.zoom.samples_per_px; - let total = (s.content_end().saturating_add(s.sample_rate.samples(30.0)).max(1) as f64).max(s.edit.zoom.scroll as f64 + vis); + let total = (end.saturating_add(s.sample_rate.samples(30.0)).max(1) as f64).max(s.edit.zoom.scroll as f64 + vis); let frac = (vis / total).clamp(0.02, 1.0) as f32; let off = (s.edit.zoom.scroll as f64 / total).clamp(0.0, 1.0) as f32; let thumb = Rect::from_min_size(pos2(sb.min.x + sb.width() * off, sb.min.y + 3.0), vec2((sb.width() * frac).max(24.0), sb.height() - 6.0)); @@ -838,11 +894,12 @@ fn track_row(app: &mut SoundApp, ui: &mut Ui, id: TrackId, row: Rect, tl: Rect, } let view = Range::new(sample_at(&s, tl, tl.min.x), sample_at(&s, tl, tl.max.x)); let auto_view = AutoParam::parse(&track.view); + let sorted = track.clips().is_sorted_by_key(|c| c.start); for clip in track.clips() { if !clip.range().overlaps(&view) { continue; } - draw_clip(app, &painter, &s, track, clip, tl, lane, auto_view.is_some()); + draw_clip(app, &painter, &s, track, sorted, clip, tl, lane, auto_view.is_some()); } draw_live_take(app, &painter, &s, track, tl, lane); crate::video_track::draw_lane(app, &painter, &s, track, tl, lane); @@ -885,8 +942,10 @@ fn playlist_lanes(app: &mut SoundApp, ui: &mut Ui, track: &Track, row: Rect, mai ui.painter().rect_filled(lane, 0.0, t.playlist_lane); ui.painter().text(pos2(head.min.x + 4.0, head.center().y), Align2::LEFT_CENTER, &pl.name, regular(10.5), t.text_dim); let painter = ui.painter().with_clip_rect(lane.intersect(ui.clip_rect())); - for c in &pl.clips { - draw_clip(app, &painter, &s, track, c, tl, lane, false); + let sorted = track.clips().is_sorted_by_key(|c| c.start); + let view = Range::new(sample_at(&s, tl, tl.min.x), sample_at(&s, tl, tl.max.x)); + for c in pl.clips.iter().filter(|c| c.range().overlaps(&view)) { + draw_clip(app, &painter, &s, track, sorted, c, tl, lane, false); } let resp = ui.interact(lane, ui.id().with(("pl_lane", track.id.0, i)), Sense::click()); if resp.clicked() @@ -1511,9 +1570,13 @@ fn fade_curve_points(fade: soundcraft_model::Fade, fade_in: bool, a: f32, b: f32 let scale = if pixels_per_point.is_finite() { pixels_per_point.clamp(0.5, 4.0) } else { 1.0 }; let extent = (b - a).abs().max(body.height().abs()) * scale; let extent = if extent.is_finite() { extent.clamp(64.0, 4096.0) } else { 64.0 }; + // A narrow fade needs fewer: the curve between two samples stays inside a box `width / segments` + // wide, so it is never further than that from its chord. Zoomed out on an edited drum take, + // thousands of fades are a pixel or two wide. + let narrow = ((b - a).abs() * scale * 4.0).ceil() + 1.0; // Physical-pixel sampling avoids visible corners, even for steep bends. // Keep an exact midpoint and cap allocations for extreme zoom/clip sizes. - let segments = (extent.ceil() as usize).div_ceil(2) * 2; + let segments = (extent.min(narrow).max(2.0).ceil() as usize).div_ceil(2) * 2; (0..=segments) .map(|i| { let k = i as f32 / segments as f32; @@ -1582,7 +1645,8 @@ fn grid_veil(line: Color32, bg: Color32) -> Color32 { } } -fn draw_clip(app: &SoundApp, painter: &egui::Painter, s: &Session, track: &Track, clip: &Clip, tl: Rect, lane: Rect, dim: bool) { +/// `sorted`: whether `track.clips()` is sorted by start (worked out once per track and frame). +fn draw_clip(app: &SoundApp, painter: &egui::Painter, s: &Session, track: &Track, sorted: bool, clip: &Clip, tl: Rect, lane: Rect, dim: bool) { let x0 = x_of(s, tl, clip.start); let x1 = x_of(s, tl, clip.end()).max(x0 + 1.0); let r = Rect::from_min_max(pos2(x0, lane.min.y + 1.0), pos2(x1, lane.max.y - 1.0)); @@ -1604,13 +1668,16 @@ fn draw_clip(app: &SoundApp, painter: &egui::Painter, s: &Session, track: &Track let nb = Rect::from_min_max(r.min, pos2(r.max.x, r.min.y + name_h)); painter.rect_filled(nb, CornerRadius { nw: 2, ne: 2, sw: 0, se: 0 }, if clip.muted { Color32::from_rgb(70, 70, 70) } else { bar }); let name_clip = nb.intersect(lane); - painter.with_clip_rect(name_clip).text( - pos2(nb.min.x.max(lane.min.x) + 4.0, nb.center().y), - Align2::LEFT_CENTER, - &clip.name, - bold(10.5), - Color32::from_rgb(12, 12, 12), - ); + // Narrower than this, no glyph would show: the name starts 4 points in. + if name_clip.width() > 6.0 { + painter.with_clip_rect(name_clip).text( + pos2(nb.min.x.max(lane.min.x) + 4.0, nb.center().y), + Align2::LEFT_CENTER, + &clip.name, + bold(10.5), + Color32::from_rgb(12, 12, 12), + ); + } } let body_r = Rect::from_min_max(pos2(r.min.x, r.min.y + name_h), r.max); let vis = body_r.intersect(lane); @@ -1772,8 +1839,16 @@ fn draw_clip(app: &SoundApp, painter: &egui::Painter, s: &Session, track: &Track painter.text(pos2(r.max.x - 6.0, r.max.y - 7.0), Align2::RIGHT_CENTER, "FX", bold(9.0), Color32::from_rgb(250, 230, 120)); } if s.edit.flag("view.clip.overlap_shadows") { - // Shade where a later clip covers this one (crossfades and layered edits). - for o in track.clips() { + // Shade where a later clip covers this one (crossfades and layered edits). Only clips that + // start inside this one can: on a sorted track, find them instead of checking every clip. + let clips = track.clips(); + let later = if sorted { + let from = clips.partition_point(|o| o.start <= clip.start); + clips.get(from..clips.partition_point(|o| o.start < clip.end()).max(from)).unwrap_or_default() + } else { + clips + }; + for o in later { if o.id != clip.id && o.start > clip.start && o.start < clip.end() { let x0 = x_of(s, tl, o.start); let x1 = x_of(s, tl, clip.end().min(o.end())); @@ -2776,6 +2851,60 @@ fn overlay(app: &mut SoundApp, ui: &mut Ui, tl: Rect, area: Rect) { } } +#[cfg(test)] +mod universe_tests { + use super::*; + use soundcraft_model::{ChannelFormat, Clip, ClipId, SourceId, TrackKind}; + + /// The pixel columns each block covers, per colour, as the old one-rect-per-clip painting did. + fn covered(blocks: &[(Rect, Color32)]) -> Vec<(i32, Color32)> { + let mut px: Vec<(i32, Color32)> = Vec::new(); + for (r, c) in blocks { + for x in (r.min.x * 4.0).round() as i32..(r.max.x * 4.0).round() as i32 { + px.retain(|p| p.0 != x); + px.push((x, *c)); + } + } + px.sort_by_key(|p| p.0); + px + } + + fn per_clip(s: &Session, r: Rect) -> Vec<(Rect, Color32)> { + let total = (s.content_end() + s.sample_rate.samples(10.0)).max(1) as f32; + let t = &s.tracks[0]; + t.clips() + .iter() + .map(|c| { + let x0 = r.min.x + c.start as f32 / total * r.width(); + let x1 = (r.min.x + c.end() as f32 / total * r.width()).max(x0 + 1.0); + (Rect::from_min_max(pos2(x0, r.min.y + 0.5), pos2(x1, r.max.y - 0.5)), rgb(c.color.unwrap_or(t.color))) + }) + .collect() + } + + #[test] + fn a_sliced_track_is_a_few_runs_covering_the_same_pixels() { + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None); + let mut clips = Vec::new(); + let mut at = 0; + for i in 0..5000u64 { + let mut c = Clip::audio(ClipId(i + 1), "hit", SourceId(1), 0, at, 200 + (i % 7) as i64 * 50); + // A recoloured stretch, and a gap, in the middle. + if (2000..2100).contains(&i) { + c.color = Some([200, 40, 40]); + } + at = c.end() + if i == 3000 { 2_000_000 } else { 0 }; + clips.push(c); + } + s.track_mut(t).unwrap().playlist_mut().unwrap().clips = clips; + let r = Rect::from_min_size(pos2(8.0, 4.0), vec2(900.0, 56.0)); + let runs = universe_runs(&s, r); + assert!(runs.len() <= 6, "{} rects for 5000 clips", runs.len()); + assert_eq!(covered(&runs), covered(&per_clip(&s, r))); + } +} + #[cfg(test)] mod tests { use super::*; @@ -3444,6 +3573,37 @@ mod tests { } } + #[test] + fn narrow_fades_take_few_points_and_stay_within_a_quarter_pixel() { + for width in [0.2f32, 1.0, 3.0, 12.0] { + for height in [40.0f32, 320.0] { + let body = Rect::from_min_size(Pos2::ZERO, vec2(width, height)); + for scale in [1.0, 2.0] { + for shape in soundcraft_model::FadeShape::ALL { + for curvature in [-1.0, 0.0, 1.0] { + let fade = soundcraft_model::Fade { len: 1000, shape, curvature }; + let points = fade_curve_points(fade, true, 0.0, width, body, scale); + assert!(points.len() <= ((width * scale * 4.0).ceil() as usize + 4), "{width} wide: {} points", points.len()); + assert_eq!((points.len() - 1) % 2, 0, "an exact midpoint"); + let segments = (points.len() - 1) as f32; + for (i, pair) in points.windows(2).enumerate() { + let line = pair[1] - pair[0]; + for fraction in [0.1, 0.25, 0.5, 0.75, 0.9] { + let t = (i as f32 + fraction) / segments; + let actual = pos2(width * t, height - height * fade.gain(t)); + let along = + if line.length_sq() > 0.0 { ((actual - pair[0]).dot(line) / line.length_sq()).clamp(0.0, 1.0) } else { 0.0 }; + let distance = actual.distance(pair[0] + line * along) * scale; + assert!(distance < 0.25, "{width}x{height}, scale={scale}, {shape:?}, curve={curvature}, error={distance}"); + } + } + } + } + } + } + } + } + fn painted_fade_control(h: &Harness, clip_rect: Rect) -> Pos2 { let controls: Vec<_> = h .output() diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index a208511f..54383aea 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -184,32 +184,75 @@ fn gutter_scroll(ui: &Ui, gutter: Rect, out: &egui::scroll_area::ScrollAreaOu } } +/// One row of the clip list: a whole file, or a clip on a track (positions in the session). +#[derive(Clone, Copy)] +enum ClipRow { + Source(usize), + Clip { track: usize, clip: usize }, +} + +/// The clip list's rows for one document revision and filter. A drum edit holds tens of thousands +/// of clips: the rows are listed (and filtered) when either changes, not every frame. +#[derive(Clone)] +struct ClipRows { + revision: u64, + filter: String, + rows: std::sync::Arc>, +} + +fn clip_rows(s: &soundcraft_model::Session, needle: &str) -> Vec { + let shown = |name: &str| needle.is_empty() || name.to_lowercase().contains(needle); + // Whole files first (bold), then clips on tracks. + let sources = + s.sources.iter().enumerate().filter(|(_, src)| shown(&format!("{} ({}ch)", src.name, src.channels))).map(|(i, _)| ClipRow::Source(i)); + let clips = + s.tracks.iter().enumerate().flat_map(|(track, tr)| { + tr.clips().iter().enumerate().filter(|(_, c)| shown(&c.name)).map(move |(clip, _)| ClipRow::Clip { track, clip }) + }); + sources.chain(clips).collect() +} + pub fn clip_list(app: &mut SoundApp, ui: &mut Ui) { let t = Tokens::current(); panel_header(ui, "CLIPS"); - let items: Vec<(u64, String, bool, [u8; 3], bool)> = { - let s = app.engine.session(); - let mut v = Vec::new(); - // Whole files first (bold), then clips on tracks. - for src in &s.sources { - v.push((src.id.0, format!("{} ({}ch)", src.name, src.channels), true, [150, 150, 150], s.edit.selected_sources.contains(&src.id))); - } - for tr in &s.tracks { - for c in tr.clips() { - v.push((c.id.0, c.name.clone(), false, c.color.unwrap_or(tr.color), s.edit.selected_clips.contains(&c.id))); - } - } - v - }; let fkey = egui::Id::new("clip_filter"); let mut filter: String = ui.ctx().memory(|m| m.data.get_temp(fkey)).unwrap_or_default(); ui.add(egui::TextEdit::singleline(&mut filter).hint_text("🔍 Find clips").desired_width(f32::INFINITY)); ui.ctx().memory_mut(|m| m.data.insert_temp(fkey, filter.clone())); let needle = filter.to_lowercase(); - let items: Vec<_> = items.into_iter().filter(|(_, n, _, _, _)| needle.is_empty() || n.to_lowercase().contains(&needle)).collect(); - egui::ScrollArea::vertical().id_salt("clips_scroll").auto_shrink([false, false]).show(ui, |ui| { + let rkey = egui::Id::new("clip_rows"); + let revision = app.engine.revision; + let cached = ui.ctx().memory(|m| m.data.get_temp::(rkey)).filter(|c| c.revision == revision && c.filter == needle); + let rows = match cached { + Some(c) => c.rows, + None => { + let rows = std::sync::Arc::new(clip_rows(app.engine.session(), &needle)); + ui.ctx().memory_mut(|m| m.data.insert_temp(rkey, ClipRows { revision, filter: needle, rows: rows.clone() })); + rows + } + }; + let row_h = 17.0; + egui::ScrollArea::vertical().id_salt("clips_scroll").auto_shrink([false, false]).show_rows(ui, row_h, rows.len(), |ui, shown| { + // Only the rows in view: what they show is read now, before any of them acts on the session. + let items: Vec<(u64, String, bool, [u8; 3], bool)> = { + let s = app.engine.session(); + rows.get(shown) + .unwrap_or_default() + .iter() + .filter_map(|row| match *row { + ClipRow::Source(i) => s.sources.get(i).map(|src| { + (src.id.0, format!("{} ({}ch)", src.name, src.channels), true, [150, 150, 150], s.edit.selected_sources.contains(&src.id)) + }), + ClipRow::Clip { track, clip } => { + let tr = s.tracks.get(track)?; + let c = tr.clips().get(clip)?; + Some((c.id.0, c.name.clone(), false, c.color.unwrap_or(tr.color), s.edit.selected_clips.contains(&c.id))) + } + }) + .collect() + }; for (id, name, whole, color, selected) in items { - let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 17.0), if whole { Sense::click_and_drag() } else { Sense::click() }); + let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), row_h), if whole { Sense::click_and_drag() } else { Sense::click() }); resp.context_menu(|ui| { if whole { if ui.button("Place on New Track").clicked() { @@ -708,3 +751,32 @@ fn preset_menu( }); ui.ctx().memory_mut(|m| m.data.insert_temp(key, name)); } + +#[cfg(test)] +mod tests { + use super::*; + use soundcraft_model::{ChannelFormat, Clip, Session, TrackKind}; + + #[test] + fn clip_rows_list_every_clip_in_track_order_and_filter_by_name() { + let mut s = Session::default(); + for names in [["Kick 1", "Snare 1"], ["Kick 2", "Tom"]] { + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None); + for (i, n) in names.into_iter().enumerate() { + let c = Clip::audio(s.new_clip_id(), n, SourceId(1), 0, i as i64 * 100, 100); + s.track_mut(t).unwrap().playlist_mut().unwrap().clips.push(c); + } + } + let names = |rows: Vec| -> Vec { + rows.into_iter() + .filter_map(|r| match r { + ClipRow::Clip { track, clip } => Some(s.tracks[track].clips()[clip].name.clone()), + ClipRow::Source(_) => None, + }) + .collect() + }; + assert_eq!(names(clip_rows(&s, "")), ["Kick 1", "Snare 1", "Kick 2", "Tom"]); + assert_eq!(names(clip_rows(&s, "kick")), ["Kick 1", "Kick 2"]); + assert!(clip_rows(&s, "cowbell").is_empty()); + } +} From 797aaac058ccee08923acf2785e693151de0ec7c Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:43:18 +0200 Subject: [PATCH 06/38] Resolve hostnames in remote control client targets Found while testing crates/automation/src/lib.rs:100 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/automation/src/lib.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 1172f7be..352200d3 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -13,7 +13,7 @@ use serde_json::{Value, json}; use soundcraft_engine::Engine; use soundcraft_engine::file_access::{self, AutomationRoots}; use std::io::{BufRead, BufReader, Write}; -use std::net::TcpStream; +use std::net::{TcpStream, ToSocketAddrs}; use std::sync::Arc; use std::time::Duration; @@ -97,9 +97,19 @@ impl Remote { if self.conn.is_some() { return Ok(()); } - let sock = self.addr.parse().map_err(|e| format!("{}: {e}", self.addr))?; - let s = - TcpStream::connect_timeout(&sock, Duration::from_millis(800)).map_err(|e| format!("cannot reach SoundCraft at {}: {e}", self.addr))?; + let socks: Vec<_> = self.addr.to_socket_addrs().map_err(|e| format!("{}: {e}", self.addr))?.collect(); + let mut last = "no addresses found".to_string(); + let mut stream = None; + for sock in &socks { + match TcpStream::connect_timeout(sock, Duration::from_millis(800)) { + Ok(s) => { + stream = Some(s); + break; + } + Err(e) => last = e.to_string(), + } + } + let s = stream.ok_or_else(|| format!("cannot reach SoundCraft at {}: {last}", self.addr))?; s.set_read_timeout(Some(Duration::from_secs(120))).map_err(|e| e.to_string())?; let r = s.try_clone().map_err(|e| e.to_string())?; self.conn = Some((s, BufReader::new(r))); @@ -179,6 +189,14 @@ mod tests { assert!(r.call("session.inspect", json!({})).is_err()); } + #[test] + fn remote_resolves_hostnames() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let mut r = Remote::new(&format!("localhost:{port}")); + assert_eq!(r.connect(), Ok(())); + } + /// Fake app: accepts one connection per entry in `replies`, counts the request line it reads, /// then either drops the connection (`false`) or answers ok (`true`). fn fake_app(replies: Vec) -> (String, Arc) { From 187ee51ae0f37b60bba53c9fa4542534e66da481 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:30:10 +0200 Subject: [PATCH 07/38] Resolve same-pitch overlapping notes when writing SMF Found while testing crates/midi/src/smf.rs:486-493 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/midi/src/smf.rs | 64 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/crates/midi/src/smf.rs b/crates/midi/src/smf.rs index 7747a061..55ceaf54 100644 --- a/crates/midi/src/smf.rs +++ b/crates/midi/src/smf.rs @@ -483,11 +483,49 @@ pub fn write_smf(smf: &Smf) -> Result, MidiError> { if !track.name.is_empty() { ev.push(WEvent { tick: 0, order: 0, bytes: meta(0x03, track.name.as_bytes())? }); } - for n in &track.sequence.notes { + let notes = &track.sequence.notes; + let mut ends: Vec = notes.iter().map(|n| clamp_tick(n.end()).max(clamp_tick(n.start).saturating_add(1))).collect(); + let mut dropped = vec![false; notes.len()]; + // The reader pairs offs with ons first-in first-out, so a note still sounding when the next + // same-pitch note starts would swap ends with it: trim it to that start. When two same-pitch + // notes start on the same tick they cannot be told apart, so only the longer one is written. + let mut order: Vec = (0..notes.len()).collect(); + order.sort_by_key(|&i| { + notes.get(i).map_or((0, 0, 0, std::cmp::Reverse(0)), |n| { + (n.channel & 0x0F, n.pitch.min(127), clamp_tick(n.start), std::cmp::Reverse(ends.get(i).copied().unwrap_or(0))) + }) + }); + let mut prev: Option = None; + for &b in &order { + let Some(nb) = notes.get(b) else { continue }; + if let Some(a) = prev + && let Some(na) = notes.get(a) + && na.channel & 0x0F == nb.channel & 0x0F + && na.pitch.min(127) == nb.pitch.min(127) + { + let (sa, sb) = (clamp_tick(na.start), clamp_tick(nb.start)); + if sa == sb { + if let Some(d) = dropped.get_mut(b) { + *d = true; + } + continue; + } + if let Some(ea) = ends.get_mut(a) + && sb < *ea + { + *ea = sb; + } + } + prev = Some(b); + } + for (i, n) in notes.iter().enumerate() { + if dropped.get(i).copied().unwrap_or(false) { + continue; + } let ch = n.channel & 0x0F; let pitch = n.pitch.min(127); let start = clamp_tick(n.start); - let end = clamp_tick(n.end()).max(start.saturating_add(1)); + let end = ends.get(i).copied().unwrap_or(start.saturating_add(1)); ev.push(WEvent { tick: start, order: 3, bytes: vec![0x90 | ch, pitch, n.velocity.clamp(1, 127)] }); ev.push(WEvent { tick: end, order: 1, bytes: vec![0x80 | ch, pitch, n.release_velocity.min(127)] }); } @@ -536,4 +574,26 @@ mod tests { push_vlq(&mut b, 0x2000).unwrap(); assert_eq!(b, [0xC0, 0x00]); } + + #[test] + fn same_pitch_overlaps_do_not_swap_ends() { + let mk = |start, length| Note { pitch: 60, velocity: 100, release_velocity: 64, channel: 0, start, length }; + let smf = Smf { + format: 1, + tracks: vec![SmfTrack { + name: String::new(), + sequence: Sequence { notes: vec![mk(0, 1000), mk(100, 200), mk(1000, 100), mk(2000, 500), mk(2000, 900)], ..Sequence::default() }, + }], + tempos: vec![], + meters: vec![], + markers: vec![], + key_sigs: vec![], + }; + let back = read_smf(&write_smf(&smf).unwrap()).unwrap(); + let mut got: Vec<(i64, i64)> = back.tracks.iter().flat_map(|t| t.sequence.notes.iter().map(|n| (n.start, n.length))).collect(); + got.sort_unstable(); + // The earlier note is trimmed to the start of the next same-pitch note; later notes keep their length; + // of two notes starting together only the longer survives. + assert_eq!(got, vec![(0, 100), (100, 200), (1000, 100), (2000, 900)]); + } } From 717ae97af5bfe50f9687ba3fbe5fe716a674f684 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 15:16:05 +0200 Subject: [PATCH 08/38] Scroll Into View in the track name menu: the track goes to the top of the Edit window and the left of the Mix --- crates/ui-egui/src/edit_window.rs | 27 +++++ crates/ui-egui/src/lib.rs | 6 + crates/ui-egui/src/mix_window.rs | 7 ++ crates/ui-egui/tests/scroll_into_view.rs | 134 +++++++++++++++++++++++ docs/ui-parity.md | 5 +- 5 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/tests/scroll_into_view.rs diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index 7e24a4c7..ae7758d8 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -239,6 +239,9 @@ fn main_area_content(app: &mut SoundApp, ui: &mut Ui) { draw_rulers(app, ui, rulers_rect, tl, &rulers); views_button(app, ui, rulers_rect); column_titles(ui, rulers_rect, &cols); + if let Some(id) = app.edit_scroll_into_view.take() { + scroll_into_view(app, id, tl.height()); + } // Tracks. let mut y = tl.min.y - app.edit_layout.scroll_y; app.edit_layout.rows.clear(); @@ -289,6 +292,25 @@ fn main_area_content(app: &mut SoundApp, ui: &mut Ui) { } } +/// Scroll Into View: track `id` to the top, as far as the scroll bar reaches. A track the Edit +/// window doesn't show (hidden, or in a closed folder) leaves the view where it is. +fn scroll_into_view(app: &mut SoundApp, id: TrackId, view_h: f32) { + let s = app.engine.session(); + let (mut top, mut total, mut found) = (0.0, 0.0, false); + for t in s.tracks.iter().filter(|t| in_edit_window(s, t)) { + found |= t.id == id; + if !found { + top += track_height(t) + 1.0; + } + total += track_height(t) + 1.0; + } + if found { + let l = &mut app.edit_layout; + l.content_h = total; + l.scroll_by(top - l.scroll_y, view_h); + } +} + fn in_edit_window(s: &Session, t: &Track) -> bool { ops::shown(s, t) } @@ -1456,6 +1478,11 @@ pub(crate) fn track_context_menu(app: &mut SoundApp, ui: &mut Ui, id: TrackId) { if app.engine.session().track(id).is_some_and(|track| track.folder.is_some()) && ui.button("Remove from Folder").clicked() { let _ = app.run("track.remove_from_folder", json!({"tracks": selection})); } + // Only the track clicked: the command has one target, and the selection stays as it is. + if ui.button("Scroll Into View").clicked() { + app.edit_scroll_into_view = Some(id); + app.mix_scroll_into_view = Some(id); + } ui.separator(); ui.menu_button("Track Height", |ui| { for h in TrackHeight::ALL { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3fdd9892..da75825f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -295,6 +295,10 @@ pub struct SoundApp { pub palette: palette::PaletteState, /// Bring the separate Mix window to the front on the next frame. pub(crate) focus_mix: bool, + /// Scroll Into View: the track each window brings to its top (Edit) or left (Mix) when it + /// next draws, so a window not on screen catches up when it is shown. + pub(crate) edit_scroll_into_view: Option, + pub(crate) mix_scroll_into_view: Option, pub synthetic: Vec, /// Where a dropped audio file loads into a track's sampler (last frame's rects). pub sample_drop_targets: Vec<(egui::Rect, TrackId)>, @@ -381,6 +385,8 @@ impl SoundApp { extra: extra_windows::ExtraState::default(), palette: palette::PaletteState::default(), focus_mix: false, + edit_scroll_into_view: None, + mix_scroll_into_view: None, synthetic: Vec::new(), sample_drop_targets: Vec::new(), sample_reads: Vec::new(), diff --git a/crates/ui-egui/src/mix_window.rs b/crates/ui-egui/src/mix_window.rs index 35b5f5cb..0b63e3b4 100644 --- a/crates/ui-egui/src/mix_window.rs +++ b/crates/ui-egui/src/mix_window.rs @@ -157,6 +157,13 @@ fn strip(app: &mut SoundApp, ui: &mut Ui, id: TrackId, snap: Option<&MeterSnapsh let w = strip_width(narrow); let h = ui.available_height().max(560.0); let (r, _) = ui.allocate_exact_size(vec2(w, h), Sense::hover()); + if app.mix_scroll_into_view == Some(id) { + app.mix_scroll_into_view = None; + // egui stops `item_spacing` short of the edge (the scroll area's, not the strips' 1 pt), + // which would leave a sliver of the previous strip showing. + let margin = ui.ctx().global_style().spacing.item_spacing.x; + ui.scroll_to_rect(r.translate(vec2(margin, 0.0)), Some(egui::Align::Min)); + } let selected = app.engine.session().edit.selected_tracks.contains(&id); let bg = if selected { t.strip_selected } else { t.strip_bg }; vgradient(ui.painter(), r, shade(bg, 0.06), bg); diff --git a/crates/ui-egui/tests/scroll_into_view.rs b/crates/ui-egui/tests/scroll_into_view.rs new file mode 100644 index 00000000..c0d64efc --- /dev/null +++ b/crates/ui-egui/tests/scroll_into_view.rs @@ -0,0 +1,134 @@ +//! Scroll Into View, in a track name's right-click menu: the track goes to the top of the Edit +//! window and to the left of the Mix window, both windows at once. + +use egui::{Event, Modifiers, PointerButton, Pos2, Shape, pos2, vec2}; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use serde_json::json; +use soundcraft_ui_egui::{MainWindow, Services, SoundApp}; + +const SIZE: (f32, f32) = (1400.0, 900.0); + +fn harness(tracks: usize) -> Harness<'static, SoundApp> { + let mut app = SoundApp::new(soundcraft_engine::Engine::default(), None, Services::default()); + for i in 1..=tracks { + app.run("track.new", json!({"name": format!("T{i:02}")})).unwrap(); + } + // Uneven heights, so an offset counted as "n rows" can't pass for the real one. + app.run("track.height", json!({"tracks": ["T02"], "height": "large"})).unwrap(); + app.run("track.height", json!({"tracks": ["T04"], "height": "small"})).unwrap(); + let mut h = Harness::builder().with_size(vec2(SIZE.0, SIZE.1)).with_step_dt(1.0 / 60.0).build_ui_state( + |ui, app: &mut SoundApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + app, + ); + h.run_steps(4); + h +} + +fn right_click(h: &mut Harness<'_, SoundApp>, at: Pos2) { + h.hover_at(at); + for pressed in [true, false] { + h.event(Event::PointerButton { pos: at, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); + } + h.run_steps(3); +} + +fn scroll_into_view(h: &mut Harness<'_, SoundApp>) { + h.get_by_label("Scroll Into View").click(); + h.run_steps(4); +} + +fn track_id(h: &Harness<'_, SoundApp>, name: &str) -> u64 { + h.state().engine.session().tracks.iter().find(|t| t.name == name).unwrap().id.0 +} + +fn edit_row(h: &Harness<'_, SoundApp>, name: &str) -> [f32; 4] { + let id = track_id(h, name); + h.state().edit_layout.rows.iter().find(|(t, _)| *t == id).unwrap().1 +} + +/// Height of the Edit window's track area, as last drawn. +fn view_h(h: &Harness<'_, SoundApp>) -> f32 { + let [_, y0, _, y1] = h.state().edit_layout.timeline; + y1 - y0 +} + +fn max_scroll(h: &Harness<'_, SoundApp>) -> f32 { + h.state().edit_layout.content_h - view_h(h) + 40.0 +} + +/// The centre of a Mix strip's name plate: its name is painted centred on it, at the bottom of +/// the strip (the galley's box, not the glyphs, so "T01" and "T08" line up alike). +fn plate(h: &Harness<'_, SoundApp>, name: &str) -> Option { + h.output().shapes.iter().find_map(|shape| match &shape.shape { + Shape::Text(text) if text.galley.job.text == name && text.pos.y > SIZE.1 - 40.0 => Some(text.pos + text.galley.size() / 2.0), + _ => None, + }) +} + +#[test] +fn the_track_goes_to_the_top_of_the_edit_window() { + let mut h = harness(20); + assert_eq!(h.state().edit_layout.scroll_y, 0.0); + let row = edit_row(&h, "T06"); + right_click(&mut h, pos2(row[0] + 60.0, row[1] + 10.0)); + scroll_into_view(&mut h); + + let above: f32 = ["T01", "T02", "T03", "T04", "T05"].iter().map(|n| edit_row(&h, n)).map(|r| r[3] - r[1] + 1.0).sum(); + assert_eq!(h.state().edit_layout.scroll_y, above); + assert_eq!(edit_row(&h, "T06")[1], h.state().edit_layout.timeline[1], "T06 is the first row in view"); +} + +#[test] +fn near_the_end_it_scrolls_only_as_far_as_the_scroll_bar_goes() { + let mut h = harness(20); + let max = max_scroll(&h); + h.state_mut().edit_layout.scroll_y = max - 60.0; + h.run_steps(3); + let row = edit_row(&h, "T19"); + assert!(row[1] - h.state().edit_layout.timeline[1] < max, "the fixture puts T19 past the furthest scroll"); + right_click(&mut h, pos2(row[0] + 60.0, row[1] + 10.0)); + scroll_into_view(&mut h); + + assert_eq!(h.state().edit_layout.scroll_y, max); +} + +#[test] +fn from_the_mix_the_clicked_strip_goes_left_and_the_edit_window_follows() { + let mut h = harness(20); + let t03 = track_id(&h, "T03"); + h.state_mut().engine.session_mut().edit.selected_tracks = vec![soundcraft_model::TrackId(t03)]; + h.state_mut().ui.window = MainWindow::Mix; + h.run_steps(4); + let first = plate(&h, "T01").expect("the first strip's name").x; + let target = plate(&h, "T08").expect("T08 is on screen"); + right_click(&mut h, target); + scroll_into_view(&mut h); + + let now = plate(&h, "T08").expect("T08 still on screen").x; + assert!((now - first).abs() < 1.0, "T08 is the leftmost strip: at {now}, the first was at {first}"); + assert_eq!(h.state().engine.session().edit.selected_tracks, vec![soundcraft_model::TrackId(t03)], "the selection is left alone"); + + h.state_mut().ui.window = MainWindow::Edit; + h.run_steps(4); + assert_eq!(edit_row(&h, "T08")[1], h.state().edit_layout.timeline[1], "the Edit window scrolled too"); +} + +#[test] +fn a_track_in_a_closed_folder_scrolls_the_mix_but_not_the_edit_window() { + let mut h = harness(20); + h.state_mut().run("track.move_to_new_folder", json!({"tracks": ["T08"], "name": "F"})).unwrap(); + h.state_mut().run("track.folder_toggle", json!({"track": "F", "open": false})).unwrap(); + h.state_mut().ui.window = MainWindow::Mix; + h.run_steps(4); + let target = plate(&h, "T08").expect("the Mix shows tracks of a closed folder"); + right_click(&mut h, target); + scroll_into_view(&mut h); + + h.state_mut().ui.window = MainWindow::Edit; + h.run_steps(4); + assert_eq!(h.state().edit_layout.scroll_y, 0.0); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index cd40ace0..f7d06d57 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (drag across checkboxes, #399; waveforms preview fades during drag; a plugin opens when inserted, #486) · **Target:** Avid Pro Tools Ultimate 2026.4.1 +> **Last reviewed:** 2026-10-16 · **Last updated:** 2026-10-16 · **Change:** minor (Scroll Into View in the track name menu) · **Target:** Avid Pro Tools Ultimate 2026.4.1 How SoundCraft feels in the hands of a Pro Tools user: tools, modes, modifiers, shortcuts, windows, meters. Measured from `crates/model/src/session.rs` (tools, modes), the command @@ -35,7 +35,7 @@ Legend: ✅ matches · 🟡 partial · ❌ missing. | Modifier gestures (Alt for all tracks, Shift-Alt for selected, Cmd-click fine faders, Ctrl-click defaults) | 🟡 | Main ones on faders and pan; not across every control | | Scroll/zoom: wheel, trackpad pinch | 🟡 | Anchored trackpad zoom in open PRs #112, #137 | | Drag and drop: Clip List → tracks, files → tracks | ✅ macOS/Windows; 🟡 Wayland (PR #150) | | -| Context menus (clip, track name, ruler, Clip List) | 🟡 | Clip and track menus exist; depth lower than Pro Tools'; Clip List delete (#4) | +| Context menus (clip, track name, ruler, Clip List) | 🟡 | Clip and track menus exist; depth lower than Pro Tools'; Clip List delete (#4); track name ▸ Scroll Into View (Edit top, Mix left) | | Drag across checkboxes to check or uncheck several | ✅ | #399; every checkbox, as in the other Craft apps | ## Windows and panels @@ -69,6 +69,7 @@ Legend: ✅ matches · 🟡 partial · ❌ missing. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Track name menu: Scroll Into View brings the track to the top of the Edit window and the left of the Mix. Readiness estimate unchanged | | 2026-10-13 | minor | A plugin opens when it's inserted (#486) | | 2026-10-13 | minor | Drag across checkboxes (#399) | | 2026-10-13 | minor | Waveforms and fade curves share temporary envelopes during drag, without rebuilding media or editing the document before release. Readiness estimate unchanged | From 16371a8502ee46db6a98d1dd09ea91d81a0749f6 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 10:23:03 +0200 Subject: [PATCH 09/38] Scroll Into View is a UI command, view.scroll_into_view, and the track name menu dispatches it --- crates/ui-egui/src/edit_window.rs | 3 +-- crates/ui-egui/src/menus.rs | 15 +++++++++++++++ crates/ui-egui/tests/scroll_into_view.rs | 18 ++++++++++++++++++ docs/control-protocol.md | 4 ++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index ae7758d8..ec26a9f9 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -1480,8 +1480,7 @@ pub(crate) fn track_context_menu(app: &mut SoundApp, ui: &mut Ui, id: TrackId) { } // Only the track clicked: the command has one target, and the selection stays as it is. if ui.button("Scroll Into View").clicked() { - app.edit_scroll_into_view = Some(id); - app.mix_scroll_into_view = Some(id); + let _ = app.run("view.scroll_into_view", json!({"track": id.0})); } ui.separator(); ui.menu_button("Track Height", |ui| { diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 99e4b503..5cd048d0 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -69,6 +69,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, Option<&str>)] = &[ ("window.io_setup", "I/O Setup", "", None), ("window.shortcuts", "Keyboard Shortcuts", "", None), ("view.mix_section", "Mix Window View", "", None), + ("view.scroll_into_view", "Scroll Into View", "", None), ("ui.new_tracks_dialog", "New Tracks…", "", None), ("ui.bounce_dialog", "Bounce Mix…", "", None), ("ui.set", "Set UI State", "", None), @@ -647,6 +648,20 @@ pub fn run_ui_command(app: &mut SoundApp, id: &str, p: &Value) -> Option { + // `track` (id or name), else the first selected track: the palette passes no params. + let track = match soundcraft_engine::cmd::track_param(&app.engine, id, p, "track") { + Ok(Some(t)) => t, + Ok(None) => match app.engine.session().edit.selected_tracks.first() { + Some(t) => *t, + None => return Some(Err("view.scroll_into_view: pass `track` or select a track".into())), + }, + Err(e) => return Some(Err(e.to_string())), + }; + app.edit_scroll_into_view = Some(track); + app.mix_scroll_into_view = Some(track); + json!({"track": track.0}) + } "ui.new_tracks_dialog" => { return Some(Err("ui.new_tracks_dialog opens a dialog from the menu only; call track.new with parameters".into())); } diff --git a/crates/ui-egui/tests/scroll_into_view.rs b/crates/ui-egui/tests/scroll_into_view.rs index c0d64efc..fbb06d1a 100644 --- a/crates/ui-egui/tests/scroll_into_view.rs +++ b/crates/ui-egui/tests/scroll_into_view.rs @@ -132,3 +132,21 @@ fn a_track_in_a_closed_folder_scrolls_the_mix_but_not_the_edit_window() { h.run_steps(4); assert_eq!(h.state().edit_layout.scroll_y, 0.0); } + +#[test] +fn the_command_takes_a_track_by_name_from_the_control_channel() { + let mut h = harness(20); + h.state_mut().ui.window = MainWindow::Mix; + h.run_steps(4); + let first = plate(&h, "T01").expect("the first strip's name").x; + h.state_mut().run("view.scroll_into_view", json!({"track": "T08"})).unwrap(); + h.run_steps(4); + + let now = plate(&h, "T08").expect("T08 on screen").x; + assert!((now - first).abs() < 1.0, "T08 is the leftmost strip: at {now}, the first was at {first}"); + h.state_mut().ui.window = MainWindow::Edit; + h.run_steps(4); + assert_eq!(edit_row(&h, "T08")[1], h.state().edit_layout.timeline[1], "the Edit window scrolled too"); + assert!(h.state_mut().run("view.scroll_into_view", json!({"track": "T99"})).is_err()); +} + diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9efe3789..8417148f 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -87,6 +87,10 @@ When the integration cannot report an appearance, System uses Dark. Manual Light fixed when the OS appearance changes. An offscreen render without `--system-theme` supplies no OS appearance and therefore resolves System to Dark. +`view.scroll_into_view {"track":"Bass"}` (a track id or name; without it, the first selected +track) brings the track to the top of the Edit window and the left of the Mix window, the same as +Scroll Into View in the track name menu. A window not on screen catches up when it is next shown. + ## Panel docking Tracks and Groups, Clips, MIDI Editor, Memory Locations, and Undo History use movable panels. Commands target the active Edit or Mix view; floating panels stay inside the current viewport. Edit and Mix retain separate arrangements; Clips and MIDI Editor belong to Edit. Transport, plug-in, and specialist utility windows retain their existing hosts. The document/session view is protected. Layouts, active tabs, panel visibility, and floating rectangles are saved with the app's existing UI preferences. These operations do not edit document contents. From 76bac6495e4ad19bafd5e1eb42b8f551caba1fd9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 10:23:03 +0200 Subject: [PATCH 10/38] Scroll Into View: a Mix request lasts one frame, so a hidden track doesn't scroll the Mix when it comes back --- crates/ui-egui/src/mix_window.rs | 10 ++++++---- crates/ui-egui/tests/scroll_into_view.rs | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/mix_window.rs b/crates/ui-egui/src/mix_window.rs index 0b63e3b4..4b9a2869 100644 --- a/crates/ui-egui/src/mix_window.rs +++ b/crates/ui-egui/src/mix_window.rs @@ -47,11 +47,14 @@ pub(crate) fn content(app: &mut SoundApp, ui: &mut Ui) { let ids: Vec = app.engine.session().tracks.iter().filter(|x| !x.hidden && x.kind != TrackKind::Video).map(|x| x.id).collect(); // Per-channel peaks for multichannel meters (read once per frame). let snap = app.player.as_ref().filter(|_| app.meters_live()).map(|p| p.meters()); + // Taken for this frame only: a strip that isn't drawn (hidden track) must not scroll the Mix + // whenever it comes back. A request made during this frame (the strip's menu) waits for the next. + let scroll_to = app.mix_scroll_into_view.take(); egui::ScrollArea::horizontal().auto_shrink([false, false]).show(ui, |ui| { ui.horizontal_top(|ui| { ui.spacing_mut().item_spacing.x = 1.0; for id in ids { - strip(app, ui, id, snap.as_ref()); + strip(app, ui, id, snap.as_ref(), scroll_to == Some(id)); } let rest = Rect::from_min_max(pos2(ui.cursor().min.x, ui.clip_rect().min.y), ui.clip_rect().max); if rest.width() > 4.0 && ui.interact(rest, ui.id().with("past_strips"), Sense::click()).double_clicked() { @@ -149,7 +152,7 @@ pub(crate) fn route_popup(app: &mut SoundApp, resp: &egui::Response, id: TrackId egui::Popup::menu(resp).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| route_menu(app, ui, id, input)); } -fn strip(app: &mut SoundApp, ui: &mut Ui, id: TrackId, snap: Option<&MeterSnapshot>) { +fn strip(app: &mut SoundApp, ui: &mut Ui, id: TrackId, snap: Option<&MeterSnapshot>, scroll_here: bool) { let t = Tokens::current(); let session = app.engine.session_arc(); let Some(track) = session.track(id) else { return }; @@ -157,8 +160,7 @@ fn strip(app: &mut SoundApp, ui: &mut Ui, id: TrackId, snap: Option<&MeterSnapsh let w = strip_width(narrow); let h = ui.available_height().max(560.0); let (r, _) = ui.allocate_exact_size(vec2(w, h), Sense::hover()); - if app.mix_scroll_into_view == Some(id) { - app.mix_scroll_into_view = None; + if scroll_here { // egui stops `item_spacing` short of the edge (the scroll area's, not the strips' 1 pt), // which would leave a sliver of the previous strip showing. let margin = ui.ctx().global_style().spacing.item_spacing.x; diff --git a/crates/ui-egui/tests/scroll_into_view.rs b/crates/ui-egui/tests/scroll_into_view.rs index fbb06d1a..2a817357 100644 --- a/crates/ui-egui/tests/scroll_into_view.rs +++ b/crates/ui-egui/tests/scroll_into_view.rs @@ -150,3 +150,19 @@ fn the_command_takes_a_track_by_name_from_the_control_channel() { assert!(h.state_mut().run("view.scroll_into_view", json!({"track": "T99"})).is_err()); } +#[test] +fn a_hidden_track_leaves_no_scroll_waiting_in_the_mix() { + let mut h = harness(20); + h.state_mut().ui.window = MainWindow::Mix; + h.state_mut().run("track.hide", json!({"tracks": ["T08"], "hidden": true})).unwrap(); + h.run_steps(4); + let first = plate(&h, "T01").expect("the first strip's name").x; + h.state_mut().run("view.scroll_into_view", json!({"track": "T08"})).unwrap(); + h.run_steps(4); + + // Shown again later, T08 must not pull the Mix to itself: the request was for frames long gone. + h.state_mut().run("track.hide", json!({"tracks": ["T08"], "hidden": false})).unwrap(); + h.run_steps(4); + let now = plate(&h, "T01").expect("T01 still on screen").x; + assert!((now - first).abs() < 1.0, "the Mix stayed put: T01 at {now}, was at {first}"); +} From 63eacf115db89928d4924c73f0202a4619b76b5b Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 10:41:15 +0200 Subject: [PATCH 11/38] Read a plugin editor's state once per gesture, not every frame The state of an instrument edited in its editor (or of a plugin that marked its state dirty) was read and stored on every frame with a change, so a drag serialized the plugin and resent the session each frame. Each plugin's state now waits until its gesture ends: the first frame its editor reports no change, or its editor closing. A plugin that keeps changing doesn't hold back the others. --- crates/playback/src/lib.rs | 33 +++++--- crates/ui-egui/src/lib.rs | 151 +++++++++++++++++++++++++++++++++++-- 2 files changed, 165 insertions(+), 19 deletions(-) diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index 5cd8e84a..dc7c5df4 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -673,10 +673,15 @@ impl Player { self.side().capture_states() } - /// Reads the state of the plugins in `slots` and of those that said their state changed since - /// the last call, so the session keeps changes made only in plugin editors. - pub fn capture_changed_states(&self, slots: &[(TrackId, usize)]) -> PluginStates { - self.side().capture_changed_states(slots) + /// The plugins that said their state changed since the last call, so the session can keep + /// changes made only in plugin editors. + pub fn changed_states(&self) -> Vec<(TrackId, usize)> { + self.side().changed_states() + } + + /// Reads the state of the plugins in `slots`. + pub fn capture_states_of(&self, slots: &[(TrackId, usize)]) -> PluginStates { + self.side().capture_states_of(slots) } /// Per-UI-frame housekeeping: drops retired instances and services plugin editors. Returns @@ -801,11 +806,14 @@ impl Side { self.states.iter_mut().filter(|(k, _)| !self.refused.contains(*k)).filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))).collect() } - fn capture_changed_states(&mut self, slots: &[(TrackId, usize)]) -> PluginStates { - let changed: Vec<(TrackId, usize)> = self.editors.iter_mut().filter_map(|(k, e)| e.state_changed().then_some(*k)).collect(); + fn changed_states(&mut self) -> Vec<(TrackId, usize)> { + self.editors.iter_mut().filter_map(|(k, e)| e.state_changed().then_some(*k)).collect() + } + + fn capture_states_of(&mut self, slots: &[(TrackId, usize)]) -> PluginStates { self.states .iter_mut() - .filter(|(k, _)| !self.refused.contains(*k) && (slots.contains(k) || changed.contains(k))) + .filter(|(k, _)| !self.refused.contains(*k) && slots.contains(k)) .filter_map(|(&(t, slot), r)| Some((t, slot, r.save_state()?))) .collect() } @@ -1215,7 +1223,7 @@ mod tests { } #[test] - fn changed_states_are_read_once_for_the_session() { + fn changed_states_are_reported_once_and_read_for_the_session() { let mut side = Side::new(48_000.0, channel().1); let (a, b) = (TrackId(1), TrackId(2)); let synth = (a, soundcraft_mix::INSTRUMENT_SLOT); @@ -1227,9 +1235,12 @@ mod tests { side.editors.insert((a, 1), Box::new(Dirty(false))); side.editors.insert((b, 0), Box::new(Dirty(true))); side.refused.insert((b, 0)); + let mut changed = side.changed_states(); + changed.sort(); + assert_eq!(changed, vec![(a, 0), (b, 0)]); + assert!(side.changed_states().is_empty(), "each change is reported once"); let want = vec![(a, 0, b"a0".to_vec()), (a, soundcraft_mix::INSTRUMENT_SLOT, b"synth".to_vec())]; - assert_eq!(side.capture_changed_states(&[synth]), want); - assert!(side.capture_changed_states(&[]).is_empty(), "each change is read once"); + assert_eq!(side.capture_states_of(&[(a, 0), synth, (b, 0)]), want); } #[test] @@ -1238,7 +1249,7 @@ mod tests { let k = (TrackId(1), 0); side.states.insert(k, Box::new(|| Some(b"loaded".to_vec()))); side.add_editor(k, Box::new(Dirty(true))); - assert!(side.capture_changed_states(&[]).is_empty()); + assert!(side.changed_states().is_empty()); } #[test] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c147840a..f3c3c86e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -319,6 +319,8 @@ pub struct SoundApp { touched: std::collections::HashSet, /// The pass's last point; `i64::MIN` after a stop or a jump, so the next point erases nothing it skipped. last_write_at: Samples, + /// Plugins changed in their editors whose state the session hasn't taken yet (see `editor_states_due`). + editor_states: Vec<(TrackId, usize)>, pub frame_ms: f32, last_frame: Option, last_meter_poll: f64, @@ -332,6 +334,29 @@ pub struct SoundApp { _palette_lock: theme::TestPaletteGuard, } +/// The plugins whose state the session takes this frame, out of those changed in their editors +/// (`pending`, kept across frames). A plugin editor doesn't say when a gesture ends: a plugin's +/// gesture lasts while its editor reports a change every frame, and its state is read on the first +/// frame its editor reports none, or as soon as the editor is closed. A drag reads and stores the +/// state once, not every frame, and a plugin that keeps changing doesn't hold back the others. +fn editor_states_due( + pending: &mut Vec<(TrackId, usize)>, + edits: &[soundcraft_playback::EditorEdit], + changed: Vec<(TrackId, usize)>, + closed: impl Fn((TrackId, usize)) -> bool, +) -> Vec<(TrackId, usize)> { + let instruments = edits.iter().filter(|e| e.1 == soundcraft_mix::INSTRUMENT_SLOT).map(|e| (e.0, e.1)); + let reported: Vec<(TrackId, usize)> = instruments.chain(changed).collect(); + for k in &reported { + if !pending.contains(k) { + pending.push(*k); + } + } + let (due, rest) = std::mem::take(pending).into_iter().partition(|&k| !reported.contains(&k) || closed(k)); + *pending = rest; + due +} + /// Whether another automation-write pass is due: the transport moved at least /// `step` since `last` in either direction. `last == i64::MIN` marks "never /// written", so the first pass after (re)start is always due. Saturating @@ -395,6 +420,7 @@ impl SoundApp { arrange_request: false, touched: std::collections::HashSet::new(), last_write_at: i64::MIN, + editor_states: Vec::new(), frame_ms: 0.0, last_frame: None, last_meter_poll: f64::NEG_INFINITY, @@ -631,21 +657,27 @@ impl SoundApp { /// Parameter edits made in plugin editors go into the session (one undo step per gesture). /// Hosted instruments keep their parameters in their own state, which the session takes /// instead, as it does for plugins that said their state changed: either way the session is - /// modified, so it is autosaved and asks to be saved. + /// modified, so it is autosaved and asks to be saved. The state is read once per gesture (see + /// `editor_states_due`). fn apply_editor_edits(&mut self) { - let Some(p) = &self.player else { return }; - let mut instruments = Vec::new(); - for (t, slot, param, value) in p.idle() { + let Some(p) = &self.player else { + self.editor_states.clear(); + return; + }; + let edits = p.idle(); + let due = editor_states_due(&mut self.editor_states, &edits, p.changed_states(), |(t, slot)| !p.editor_open(t, slot)); + let states = if due.is_empty() { Vec::new() } else { p.capture_states_of(&due) }; + self.store_editor_changes(edits, states); + } + + fn store_editor_changes(&mut self, edits: Vec, states: soundcraft_playback::PluginStates) { + for (t, slot, param, value) in edits { if slot == soundcraft_mix::INSTRUMENT_SLOT { - if !instruments.contains(&(t, slot)) { - instruments.push((t, slot)); - } continue; } let key = format!("param:{}:{slot}:{param}", t.0); let _ = self.engine.execute_merged("mix.insert_param", &json!({"track": t.0, "slot": slot, "param": param, "value": value}), &key); } - let states = p.capture_changed_states(&instruments); self.store_plugin_states(states); } @@ -1324,6 +1356,7 @@ fn failed_plugin_status(s: &soundcraft_model::Session, track: TrackId, slot: usi mod tests { use super::{Services, SoundApp, automation_step_due}; use serde_json::json; + use soundcraft_model::TrackId; #[test] fn new_session_stops_playback_and_the_old_clock() { @@ -1538,6 +1571,108 @@ mod tests { assert_eq!(app.engine.session().track(kick).unwrap().mixer.inserts[0].as_ref().unwrap().state_bytes(), Some(vec![59, 1])); } + /// One UI frame of plugin editors: the instruments edited in theirs, the plugins that said + /// their state changed, the editors closed, and the state every plugin has now. Returns the + /// plugins whose state was read. + fn editor_frame(app: &mut SoundApp, edited: &[TrackId], changed: &[(TrackId, usize)], closed: &[TrackId], now: u8) -> Vec<(TrackId, usize)> { + let slot = soundcraft_mix::INSTRUMENT_SLOT; + let edits: Vec<_> = edited.iter().map(|&t| (t, slot, "cutoff".to_string(), f32::from(now))).collect(); + let due = super::editor_states_due(&mut app.editor_states, &edits, changed.to_vec(), |(t, _)| closed.contains(&t)); + let states = due.iter().map(|&(t, slot)| (t, slot, vec![now, 1])).collect(); + app.store_editor_changes(edits, states); + due + } + + fn instrument_state(app: &SoundApp, t: TrackId) -> Option> { + app.engine.session().track(t).unwrap().instrument.as_ref().unwrap().state_bytes() + } + + #[test] + fn an_instrument_editor_drag_reads_and_stores_its_state_once() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let keys = app.engine.session().track_by_name("Keys").unwrap().id; + let mut reads = Vec::new(); + let mut revisions = Vec::new(); + for now in 0..60 { + reads.extend(editor_frame(&mut app, &[keys], &[], &[], now)); + revisions.push(app.engine.revision); + } + reads.extend(editor_frame(&mut app, &[], &[], &[], 59)); + revisions.push(app.engine.revision); + assert_eq!(reads, [(keys, soundcraft_mix::INSTRUMENT_SLOT)]); + // The session goes to the audio thread on each new revision: once, not every frame. + revisions.dedup(); + assert_eq!(revisions.len(), 2, "{revisions:?}"); + assert_eq!(instrument_state(&app, keys), Some(vec![59, 1])); + } + + #[test] + fn two_instrument_editor_drags_store_its_state_twice() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let keys = app.engine.session().track_by_name("Keys").unwrap().id; + let mut reads = Vec::new(); + for now in 0..10 { + reads.extend(editor_frame(&mut app, &[keys], &[], &[], now)); + } + reads.extend(editor_frame(&mut app, &[], &[], &[], 9)); + assert_eq!(instrument_state(&app, keys), Some(vec![9, 1])); + for now in 20..30 { + reads.extend(editor_frame(&mut app, &[keys], &[], &[], now)); + } + reads.extend(editor_frame(&mut app, &[], &[], &[], 29)); + assert_eq!(reads.len(), 2); + assert_eq!(instrument_state(&app, keys), Some(vec![29, 1])); + } + + #[test] + fn closing_an_instrument_editor_mid_gesture_still_stores_its_state() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let keys = app.engine.session().track_by_name("Keys").unwrap().id; + let lead = app.engine.session().track_by_name("Lead").unwrap().id; + let lead_before = instrument_state(&app, lead); + for now in 0..10 { + editor_frame(&mut app, &[keys, lead], &[], &[], now); + } + // Keys' editor closes on a frame it still reports a change in. + let reads = editor_frame(&mut app, &[keys, lead], &[], &[keys], 10); + assert_eq!(reads, [(keys, soundcraft_mix::INSTRUMENT_SLOT)]); + assert_eq!(instrument_state(&app, keys), Some(vec![10, 1])); + assert_eq!(instrument_state(&app, lead), lead_before); + } + + #[test] + fn a_plugin_that_keeps_changing_does_not_hold_back_another_ones_state() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let keys = app.engine.session().track_by_name("Keys").unwrap().id; + let kick = app.engine.session().track_by_name("Kick").unwrap().id; + app.run("mix.insert", json!({"track": kick.0, "plugin": "gain", "slot": 0})).unwrap(); + // A plugin on Kick marks its state changed on every frame while Keys is dragged, then + // keeps doing it after the drag. + for now in 0..10 { + editor_frame(&mut app, &[keys], &[(kick, 0)], &[], now); + } + let reads = editor_frame(&mut app, &[], &[(kick, 0)], &[], 10); + assert_eq!(reads, [(keys, soundcraft_mix::INSTRUMENT_SLOT)]); + assert_eq!(instrument_state(&app, keys), Some(vec![10, 1])); + assert_eq!(app.editor_states, [(kick, 0)]); + } + + #[test] + fn a_preset_changed_in_an_effect_editor_is_stored_once() { + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + let kick = app.engine.session().track_by_name("Kick").unwrap().id; + app.run("mix.insert", json!({"track": kick.0, "plugin": "gain", "slot": 0})).unwrap(); + // No parameter edits: the plugin only says its state changed, frame after frame. + let mut reads = Vec::new(); + for now in 0..5 { + reads.extend(editor_frame(&mut app, &[], &[(kick, 0)], &[], now)); + } + reads.extend(editor_frame(&mut app, &[], &[], &[], 4)); + assert_eq!(reads, [(kick, 0)]); + let state = app.engine.session().track(kick).unwrap().mixer.inserts[0].as_ref().unwrap().state_bytes(); + assert_eq!(state, Some(vec![4, 1])); + } + #[test] fn preferences_saved_before_the_window_was_remembered_keep_full_scale() { let ui: super::UiState = serde_json::from_value(json!({"show_transport": true})).unwrap(); From 52253e89c5eea56bdb5bab39e9e4a5e976695369 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 21 Oct 2026 11:30:15 +0200 Subject: [PATCH 12/38] Autosave a plugin editor gesture that is still open The state of a plugin whose editor gesture hasn't ended is not in the session yet, so the session looked clean: the two-minute autosave and closing the session skipped it. Both now count a pending editor state as a change to keep, and read the plugins' states as they do for a modified session. --- crates/ui-egui/src/lib.rs | 45 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f3c3c86e..75ab3999 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -681,12 +681,18 @@ impl SoundApp { self.store_plugin_states(states); } + /// The session is modified, or a plugin editor gesture still holds a state the session hasn't + /// taken (see `editor_states_due`): both read the plugins' states before autosaving. + fn has_unsaved_changes(&self) -> bool { + self.engine.is_dirty() || !self.editor_states.is_empty() + } + /// Every two minutes, copy a modified session to the autosave folder (not while recording). /// Timed by the wall clock: egui's stands still while the window is hidden. fn autosave(&mut self) { let Some(dir) = self.autosave_dir.clone() else { return }; let last = *self.last_autosave.get_or_insert_with(std::time::Instant::now); - if last.elapsed().as_secs() < 120 || !self.engine.is_dirty() || self.engine.transport.recording { + if last.elapsed().as_secs() < 120 || !self.has_unsaved_changes() || self.engine.transport.recording { return; } self.last_autosave = Some(std::time::Instant::now()); @@ -749,7 +755,7 @@ impl SoundApp { if recording { self.stop_play(); } - let keep = self.engine.is_dirty() && (recording || !self.quit_requested); + let keep = self.has_unsaved_changes() && (recording || !self.quit_requested); if keep { self.capture_plugin_states(); self.write_autosave(); @@ -1444,6 +1450,41 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// A clean demo session with its autosave folder, and an editor gesture still open on Keys + /// when `gesture` is set (its state hasn't reached the session yet). + fn app_with_editor_gesture(name: &str, gesture: bool) -> (SoundApp, std::path::PathBuf) { + let dir = std::env::temp_dir().join(format!("soundcraft-{name}-{}-{gesture}", std::process::id())); + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + app.autosave_dir = Some(dir.clone()); + if gesture { + let keys = app.engine.session().track_by_name("Keys").unwrap().id; + app.editor_states.push((keys, soundcraft_mix::INSTRUMENT_SLOT)); + } + assert!(!app.engine.is_dirty()); + (app, dir) + } + + #[test] + fn an_open_plugin_editor_gesture_is_autosaved() { + for gesture in [false, true] { + let (mut app, dir) = app_with_editor_gesture("gesture-autosave", gesture); + app.last_autosave = std::time::Instant::now().checked_sub(std::time::Duration::from_secs(121)); + app.autosave(); + assert_eq!(app.latest_autosave().is_some(), gesture); + let _ = std::fs::remove_dir_all(dir); + } + } + + #[test] + fn closing_during_a_plugin_editor_gesture_keeps_the_autosave() { + for gesture in [false, true] { + let (mut app, dir) = app_with_editor_gesture("gesture-close", gesture); + app.close_autosave(); + assert_eq!(app.latest_autosave().is_some(), gesture); + let _ = std::fs::remove_dir_all(dir); + } + } + #[test] fn session_reset_does_not_apply_old_stop_edits_to_the_new_session() { let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); From 590ac97e93ce5ed014399fc0a5a6771bc3a448ac Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:54:38 +0200 Subject: [PATCH 13/38] Reset fades at cut edges in copy_range Found while testing crates/engine/src/edit.rs:157-174 Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/edit.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/engine/src/edit.rs b/crates/engine/src/edit.rs index 5531953d..cea86073 100644 --- a/crates/engine/src/edit.rs +++ b/crates/engine/src/edit.rs @@ -149,9 +149,11 @@ pub fn copy_range(s: &Session, track: TrackId, range: Range) -> Vec { let old = part.start; part.trim_start_to(isect.start); rebase_midi_start(&mut part, old, t); + part.fade_in = Fade::default(); } if isect.end < part.end() { part.trim_end_to(isect.end); + part.fade_out = Fade::default(); } part.start -= range.start; out.push(part); @@ -868,6 +870,20 @@ mod tests { s.track(t).unwrap().clips().iter().map(|c| (c.start, c.length, c.source_offset())).collect() } + #[test] + fn copy_range_resets_fades_at_cut_edges() { + let (mut s, t) = session_with_clip(); + if let Some(c) = s.track_mut(t).and_then(|t| t.playlist_mut()).and_then(|p| p.clips.first_mut()) { + c.fade_in = Fade { len: 100, shape: FadeShape::Linear, curvature: 0.0 }; + c.fade_out = Fade { len: 100, shape: FadeShape::Linear, curvature: 0.0 }; + } + let fades = |r: Range| -> Vec<(Samples, Samples)> { copy_range(&s, t, r).iter().map(|c| (c.fade_in.len, c.fade_out.len)).collect() }; + assert_eq!(fades(Range::new(2000, 3000)), vec![(0, 0)]); + assert_eq!(fades(Range::new(0, 3000)), vec![(100, 0)]); + assert_eq!(fades(Range::new(2000, 6000)), vec![(0, 100)]); + assert_eq!(fades(Range::new(0, 6000)), vec![(100, 100)]); + } + #[test] fn separate_splits_and_heal_rejoins() { let (mut s, t) = session_with_clip(); From 5029626f9e870b31cfd7b75d363d887c10c842ad Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:03:38 +0300 Subject: [PATCH 14/38] Autosave: the autosave folders are owner-only on Unix Autosaves and the takes of a session never saved are unsaved work. Their folders were made with create_dir_all, so on Unix they got the default mode and other accounts on the machine could read them. The folders the autosaver creates, and the folder a never-saved session records into, are now 0o700. A folder that already exists keeps its mode. The same change as storytold/pdfcraft#3320 and storytold/vectorcraft#3406. --- crates/engine/src/autosave.rs | 54 ++++++++++++++++++++++++++++++++--- crates/ui-egui/src/lib.rs | 5 ++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 01899d2f..03b61dd3 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -180,7 +180,7 @@ impl Autosave { fn create(root: &Path, name: &str) -> Result { let stamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap_or_default().as_millis(); let dir = root.join(format!("{} {stamp}-{}", sanitize_name(name), std::process::id())); - let lock = std::fs::create_dir_all(&dir).and_then(|()| std::fs::File::create(dir.join(LOCK))).map_err(|err| io_error(&dir, err))?; + let lock = create_folder(&dir).and_then(|()| std::fs::File::create(dir.join(LOCK))).map_err(|err| io_error(&dir, err))?; lock.try_lock().map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))?; Ok(Autosave { dir, lock: Some(lock), copies: BTreeMap::new(), next: 1, recording: false }) } @@ -210,9 +210,7 @@ impl Autosave { self.next += 1; let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: BitDepth::Float32, dither: false, bwf: None }; let bytes = soundcraft_audio_io::encode(&audio.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; - std::fs::create_dir_all(&audio_dir) - .and_then(|()| write_synced(&audio_dir.join(&name), &bytes)) - .map_err(|err| io_error(&audio_dir, err))?; + create_folder(&audio_dir).and_then(|()| write_synced(&audio_dir.join(&name), &bytes)).map_err(|err| io_error(&audio_dir, err))?; name } }; @@ -395,6 +393,18 @@ pub fn discard(left: &Leftover) { let _ = std::fs::remove_dir_all(&left.dir); } +/// Create `dir` and its missing parents under the autosave root. On Unix every folder made here is +/// owner-only (`0o700`): autosaves and the takes of a session never saved are unsaved work, and +/// with the default mode other accounts on the machine could read them. A folder that already +/// exists keeps its mode. +pub fn create_folder(dir: &Path) -> std::io::Result<()> { + let mut builder = std::fs::DirBuilder::new(); + builder.recursive(true); + #[cfg(unix)] + std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700); + builder.create(dir) +} + fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> { let mut f = std::fs::File::create(path)?; f.write_all(bytes)?; @@ -511,6 +521,42 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } + /// Autosaves are unsaved work: on Unix the folders the autosaver makes are owner-only, and an + /// autosave root that already exists keeps its mode. + #[cfg(unix)] + #[test] + fn the_autosaver_makes_owner_only_folders() { + use std::os::unix::fs::PermissionsExt as _; + let mode = |p: &Path| std::fs::metadata(p).unwrap().permissions().mode() & 0o777; + let base = folder("autosave-modes"); + let mut e = Engine::default(); + record(&mut e, 0.5); + // A root that doesn't exist yet is made with the first autosave, owner-only like the + // session's folder and the audio copied into it. + let new = base.join("Autosave"); + let w = Autosaver::new(new.clone()); + w.write(e.session_arc(), None, None); + w.flush().unwrap(); + let session = std::fs::read_dir(&new).unwrap().flatten().map(|d| d.path()).next().unwrap(); + assert_eq!((mode(&new), mode(&session), mode(&session.join("Audio Files"))), (0o700, 0o700, 0o700)); + w.close(false); + w.flush().unwrap(); + // A root that already exists keeps its mode; the folders made inside it are owner-only, + // also the one a session never saved records its takes into. + let old = base.join("old"); + std::fs::create_dir_all(&old).unwrap(); + std::fs::set_permissions(&old, std::fs::Permissions::from_mode(0o755)).unwrap(); + let w = Autosaver::new(old.clone()); + w.write(e.session_arc(), None, None); + w.flush().unwrap(); + let session = std::fs::read_dir(&old).unwrap().flatten().map(|d| d.path()).next().unwrap(); + create_folder(&old.join("Untitled")).unwrap(); + assert_eq!((mode(&old), mode(&session), mode(&old.join("Untitled"))), (0o755, 0o700, 0o700)); + w.close(false); + w.flush().unwrap(); + let _ = std::fs::remove_dir_all(&base); + } + #[test] fn a_take_still_recording_at_a_crash_is_placed_on_recovery() { use crate::io::Take; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3fdd9892..1db64e12 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -600,6 +600,11 @@ impl SoundApp { if plan.is_empty() { return Err("no audio track is record-enabled".into()); } + // The takes are unsaved work: the folder under the autosave root is owner-only, like the + // autosaves, before the recorder makes anything in it. + if self.engine.path.is_none() && self.autosave_dir.is_some() { + soundcraft_engine::autosave::create_folder(&unsaved_dir).map_err(|e| format!("{}: {e}", unsaved_dir.display()))?; + } let takes = plan .iter() .map(|(track, inputs, path)| soundcraft_engine::io::Take { track: *track, path: path.clone(), channels: inputs.len() }) From da26cf832ae960b5ca863637988559fa36ec32e2 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:43:56 +0000 Subject: [PATCH 15/38] Place Seconds and Frames grid lines at exact multiples of the step Found while testing crates/time/src/grid.rs:127-128,158-161 Co-Authored-By: Claude Opus 5.5 --- crates/time/src/grid.rs | 77 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/crates/time/src/grid.rs b/crates/time/src/grid.rs index b4e56a5c..13551b52 100644 --- a/crates/time/src/grid.rs +++ b/crates/time/src/grid.rs @@ -155,15 +155,49 @@ impl GridValue { let k = (t - bar_tick).div_euclid(step); map.tick_to_samples(bar_tick + k * step, sr) } - _ => { - let step = self.step_samples(pos, sr, map, rate); - pos.div_euclid(step).saturating_mul(step) - } + _ => self.abs_line(self.abs_index_at_or_before(pos, sr, rate), sr, rate), } } + /// Position of absolute grid line `k` (Seconds, Frames, Samples), at an exact multiple of the step + /// rounded once, so rounding never accumulates from line to line. + fn abs_line(&self, k: i64, sr: SampleRate, rate: FrameRate) -> Samples { + match *self { + GridValue::Seconds(s) => to_samples(k as f64 * Self::seconds_step(s, sr)), + GridValue::Frames(f) => rate.samples_at_frame(k.saturating_mul(f.clamp(1, 1_000_000)), sr), + GridValue::Samples(n) => k.saturating_mul(n.max(1)), + GridValue::Note { .. } => 0, + } + } + + /// Index of the absolute grid line at or before `pos`. + fn abs_index_at_or_before(&self, pos: Samples, sr: SampleRate, rate: FrameRate) -> i64 { + let mut k = match *self { + GridValue::Seconds(s) => (pos as f64 / Self::seconds_step(s, sr)).floor() as i64, + GridValue::Frames(f) => rate.frames_at(pos, sr).div_euclid(f.clamp(1, 1_000_000)), + GridValue::Samples(n) => return pos.div_euclid(n.max(1)), + GridValue::Note { .. } => return 0, + }; + // Lines are rounded to whole samples, so the floored index can be one off either way. + if self.abs_line(k, sr, rate) > pos { + k = k.saturating_sub(1); + } else if self.abs_line(k.saturating_add(1), sr, rate) <= pos { + k = k.saturating_add(1); + } + k + } + + /// The exact (fractional) Seconds step in samples, at least one sample. + fn seconds_step(s: f64, sr: SampleRate) -> f64 { + (s.clamp(0.000_01, 3600.0) * sr.as_f64()).max(1.0) + } + /// The first grid line strictly after `line`. pub fn next_line(&self, line: Samples, sr: SampleRate, map: &TempoMap, rate: FrameRate) -> Samples { + if !matches!(self, GridValue::Note { .. }) { + let next = self.abs_line(self.abs_index_at_or_before(line, sr, rate).saturating_add(1), sr, rate); + return if next > line { next } else { line.saturating_add(1) }; + } let step = self.step_samples(line, sr, map, rate); let candidate = self.line_at_or_before(line.saturating_add(step), sr, map, rate); if candidate > line { candidate } else { line.saturating_add(step) } @@ -222,6 +256,41 @@ mod tests { assert_eq!(GridValue::Samples(0).step_samples(0, sr, &map, rate), 1); } + #[test] + fn seconds_grid_lines_land_on_whole_seconds() { + let (sr, map, rate) = (SampleRate::HZ_44100, TempoMap::default(), FrameRate::Fps30); + for g in [GridValue::Seconds(0.001), GridValue::Seconds(0.01)] { + for n in 0..20 { + let s = n * 44_100; + assert_eq!(g.snap(s, sr, &map, rate), s, "{g:?} at {n} s"); + assert_eq!(g.line_at_or_before(s, sr, &map, rate), s, "{g:?} at {n} s"); + } + } + } + + #[test] + fn frame_grid_lines_land_on_frame_starts() { + let sr = SampleRate::HZ_48000; + let map = TempoMap::default(); + for rate in [FrameRate::Fps29_97, FrameRate::Fps23_976] { + for f in [1, 10] { + let g = GridValue::Frames(f); + for k in (0..5000).step_by(37) { + let s = rate.samples_at_frame(k, sr); + let line = rate.samples_at_frame(k.div_euclid(f) * f, sr); + assert_eq!(g.line_at_or_before(s, sr, &map, rate), line, "{rate:?} {f} frames at frame {k}"); + if k % f == 0 { + assert_eq!(g.snap(s, sr, &map, rate), s, "{rate:?} {f} frames at frame {k}"); + } + } + let start = rate.samples_at_frame(4000, sr); + let end = rate.samples_at_frame(4000 + 20 * f, sr); + let want: Vec<_> = (0..20).map(|i| rate.samples_at_frame(4000 + i * f, sr)).collect(); + assert_eq!(g.lines(start, end, sr, &map, rate, 100), want, "{rate:?} {f} frames"); + } + } + } + #[test] fn lines_are_capped() { let (sr, map, rate) = (SampleRate::HZ_48000, TempoMap::default(), FrameRate::Fps25); From d49e114d8420b91cb2e7be20f3546db0ddaf5d75 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:48:00 +0300 Subject: [PATCH 16/38] Autosave: the folder takes are written into is owner-only arm_recorder made a never-saved session's take folder owner-only, but the recorder then made Audio Files inside it with create_dir_all. In a take folder an earlier version left with a wider mode, new takes were readable. arm_recorder now makes the folder each take goes into owner-only. --- crates/engine/src/autosave.rs | 16 ++++++++++++---- crates/ui-egui/src/lib.rs | 8 +++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 03b61dd3..20a52880 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -541,8 +541,7 @@ mod tests { assert_eq!((mode(&new), mode(&session), mode(&session.join("Audio Files"))), (0o700, 0o700, 0o700)); w.close(false); w.flush().unwrap(); - // A root that already exists keeps its mode; the folders made inside it are owner-only, - // also the one a session never saved records its takes into. + // A root that already exists keeps its mode; the folders made inside it are owner-only. let old = base.join("old"); std::fs::create_dir_all(&old).unwrap(); std::fs::set_permissions(&old, std::fs::Permissions::from_mode(0o755)).unwrap(); @@ -550,8 +549,17 @@ mod tests { w.write(e.session_arc(), None, None); w.flush().unwrap(); let session = std::fs::read_dir(&old).unwrap().flatten().map(|d| d.path()).next().unwrap(); - create_folder(&old.join("Untitled")).unwrap(); - assert_eq!((mode(&old), mode(&session), mode(&old.join("Untitled"))), (0o755, 0o700, 0o700)); + assert_eq!((mode(&old), mode(&session)), (0o755, 0o700)); + // A session never saved records its takes into `/Audio Files` under the root. Both + // are made owner-only, and so is `Audio Files` in a take folder an earlier version left. + let takes = old.join("Untitled").join("Audio Files"); + create_folder(&takes).unwrap(); + assert_eq!((mode(&old.join("Untitled")), mode(&takes)), (0o700, 0o700)); + let earlier = old.join("Earlier"); + std::fs::create_dir_all(&earlier).unwrap(); + std::fs::set_permissions(&earlier, std::fs::Permissions::from_mode(0o755)).unwrap(); + create_folder(&earlier.join("Audio Files")).unwrap(); + assert_eq!((mode(&earlier), mode(&earlier.join("Audio Files"))), (0o755, 0o700)); w.close(false); w.flush().unwrap(); let _ = std::fs::remove_dir_all(&base); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1db64e12..5311d4d9 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -600,10 +600,12 @@ impl SoundApp { if plan.is_empty() { return Err("no audio track is record-enabled".into()); } - // The takes are unsaved work: the folder under the autosave root is owner-only, like the - // autosaves, before the recorder makes anything in it. + // The takes are unsaved work: the folder they go into under the autosave root is + // owner-only, like the autosaves, before the recorder makes anything in it. if self.engine.path.is_none() && self.autosave_dir.is_some() { - soundcraft_engine::autosave::create_folder(&unsaved_dir).map_err(|e| format!("{}: {e}", unsaved_dir.display()))?; + for dir in plan.iter().filter_map(|(_, _, path)| path.parent()) { + soundcraft_engine::autosave::create_folder(dir).map_err(|e| format!("{}: {e}", dir.display()))?; + } } let takes = plan .iter() From a966917c76867980708986ae3e9307e8a0e4b5ad Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:13:14 +0200 Subject: [PATCH 17/38] Fix automation_id overflow and bad ids for high send/plugin slots Found while testing crates/ui-egui/src/edit_window.rs:2474-2477 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/edit_window.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index 7e24a4c7..714e9fa5 100644 --- a/crates/ui-egui/src/edit_window.rs +++ b/crates/ui-egui/src/edit_window.rs @@ -2471,13 +2471,18 @@ fn automation_id(p: &AutoParam) -> String { AutoParam::Pan(0) => "pan".into(), AutoParam::Pan(_) => "pan2".into(), AutoParam::Mute => "mute".into(), - AutoParam::SendLevel(i) => format!("send_{}_level", (b'a' + i) as char), - AutoParam::SendPan(i) => format!("send_{}_pan", (b'a' + i) as char), - AutoParam::SendMute(i) => format!("send_{}_mute", (b'a' + i) as char), - AutoParam::Plugin { slot, param } => format!("plugin:{}:{param}", (b'a' + slot) as char), + AutoParam::SendLevel(i) => format!("send_{}_level", slot_id(*i)), + AutoParam::SendPan(i) => format!("send_{}_pan", slot_id(*i)), + AutoParam::SendMute(i) => format!("send_{}_mute", slot_id(*i)), + AutoParam::Plugin { slot, param } => format!("plugin:{}:{param}", slot_id(*slot)), } } +/// A slot as `AutoParam::parse` reads it: a letter for 0..=25, the number beyond that. +fn slot_id(i: u8) -> String { + if i < 26 { char::from(b'a' + i).to_string() } else { i.to_string() } +} + /// Grid snapping while dragging; holding Cmd/Ctrl swaps Grid and Slip. fn drag_snap(s: &Session, at: Samples, flip: bool) -> Samples { if s.edit.edit_mode.is_grid() != flip { s.edit.grid.snap(at, s.sample_rate, &s.tempo, s.frame_rate) } else { at } @@ -2781,6 +2786,17 @@ mod tests { use super::*; use egui_kittest::Harness; + #[test] + fn automation_id_round_trips_for_high_slots() { + for slot in [0u8, 25, 26, 155, 156, 255] { + for p in + [AutoParam::SendLevel(slot), AutoParam::SendPan(slot), AutoParam::SendMute(slot), AutoParam::Plugin { slot, param: "trim".into() }] + { + assert_eq!(AutoParam::parse(&automation_id(&p)), Some(p)); + } + } + } + #[test] fn follow_jumps_to_offscreen_playhead_while_playing() { // Steady view, follow mode, playhead left the screen: jump to it. From 79adead5bf94f7d96c283f63bb146e216648e7fc Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 14:14:07 -0500 Subject: [PATCH 18/38] FreeBSD package.sh: package the aarch64 cross build The release workflow now cross-compiles FreeBSD aarch64 inside the x86_64 VM (storytold/craft-common-actions#50) and sets CARGO_BUILD_TARGET=aarch64-unknown-freebsd. package.sh now reads the binaries from target//release when CARGO_BUILD_TARGET is set, names the archive after the target instead of uname -m, and skips the --version run when the binary is for another architecture than the machine. x86_64 packages are unchanged. Part of storytold/craft-common-actions#39. --- packaging/freebsd/package.sh | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 71af80c3..e232919c 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -29,11 +29,18 @@ while [ $# -gt 0 ]; do done # FreeBSD calls x86_64 "amd64"; release names use the Rust/Linux spelling. -case "$(uname -m)" in - amd64 | x86_64) ARCH=x86_64 ;; - arm64 | aarch64) ARCH=aarch64 ;; - *) echo "unsupported architecture $(uname -m)" >&2; exit 2 ;; -esac +release_arch() { + case "$1" in + amd64 | x86_64) echo x86_64 ;; + arm64 | aarch64) echo aarch64 ;; + *) echo "unsupported architecture $1" >&2; return 2 ;; + esac +} +HOST_ARCH="$(release_arch "$(uname -m)")" +# The release workflow cross-compiles aarch64 in an x86_64 VM with CARGO_BUILD_TARGET set to +# aarch64-unknown-freebsd (craft-common-actions#39): name the archive after the target. +ARCH="$HOST_ARCH" +if [ -n "${CARGO_BUILD_TARGET:-}" ]; then ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")"; fi BASENAME="soundcraft-$VERSION-freebsd-$ARCH" echo "==> SoundCraft $VERSION for FreeBSD $ARCH" @@ -41,7 +48,8 @@ echo "==> SoundCraft $VERSION for FreeBSD $ARCH" if [ "$SKIP_BUILD" = 0 ]; then (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli) fi -BIN="$CARGO_TARGET_DIR/release" +# Cargo puts a cross build under target//release. +BIN="$CARGO_TARGET_DIR/${CARGO_BUILD_TARGET:+$CARGO_BUILD_TARGET/}release" WORK="$CARGO_TARGET_DIR/freebsd-package" STAGE="$WORK/$BASENAME" rm -rf "$WORK" @@ -63,6 +71,10 @@ copy_docs "$STAGE/share/doc/soundcraft" tar -C "$WORK" -czf "$DIST/$BASENAME.tar.gz" "$BASENAME" echo "wrote $DIST/$BASENAME.tar.gz" -"$STAGE/bin/soundcraft-cli" --version +if [ "$ARCH" = "$HOST_ARCH" ]; then + "$STAGE/bin/soundcraft-cli" --version +else + echo "skipping --version: an $ARCH binary does not run on this $HOST_ARCH machine" +fi echo "==> done" ls -lh "$DIST" From ce42466dd6e336adf2ff9be180cbdb37b14c95cd Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 14:32:12 -0500 Subject: [PATCH 19/38] FreeBSD package.sh: accept only FreeBSD targets, check cross-built binaries Review follow-up. CARGO_BUILD_TARGET must now be a *-unknown-freebsd triple, so a Linux or macOS target can no longer be packaged under a FreeBSD name, and it is read before uname -m, so an unsupported host does not block a valid cross build. When the binaries are for another architecture and cannot run --version, file(1) must report each as a FreeBSD ELF executable for the target architecture. Part of storytold/craft-common-actions#39. --- packaging/freebsd/package.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index e232919c..a1ec8271 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -36,11 +36,15 @@ release_arch() { *) echo "unsupported architecture $1" >&2; return 2 ;; esac } -HOST_ARCH="$(release_arch "$(uname -m)")" # The release workflow cross-compiles aarch64 in an x86_64 VM with CARGO_BUILD_TARGET set to # aarch64-unknown-freebsd (craft-common-actions#39): name the archive after the target. -ARCH="$HOST_ARCH" -if [ -n "${CARGO_BUILD_TARGET:-}" ]; then ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")"; fi +case "${CARGO_BUILD_TARGET:-}" in + "") ARCH="$(release_arch "$(uname -m)")" ;; + *-unknown-freebsd) ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")" ;; + *) echo "CARGO_BUILD_TARGET=$CARGO_BUILD_TARGET is not a FreeBSD target" >&2; exit 2 ;; +esac +# Only binaries for this machine run here (the --version check at the end). +HOST_ARCH="$(release_arch "$(uname -m)" 2>/dev/null)" || HOST_ARCH="$(uname -m)" BASENAME="soundcraft-$VERSION-freebsd-$ARCH" echo "==> SoundCraft $VERSION for FreeBSD $ARCH" @@ -74,6 +78,14 @@ echo "wrote $DIST/$BASENAME.tar.gz" if [ "$ARCH" = "$HOST_ARCH" ]; then "$STAGE/bin/soundcraft-cli" --version else + # They cannot run here, so check what they are instead. + want="$ARCH"; [ "$ARCH" != x86_64 ] || want=x86-64 + for b in "$STAGE"/bin/*; do + case "$(file -b "$b")" in + ELF*"$want"*FreeBSD*) ;; + *) echo "error: $b is not a FreeBSD $ARCH executable: $(file -b "$b")" >&2; exit 1 ;; + esac + done echo "skipping --version: an $ARCH binary does not run on this $HOST_ARCH machine" fi echo "==> done" From 8063aa71731b88a24e78b076dc8b13db59fcca96 Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 14:49:35 -0500 Subject: [PATCH 20/38] FreeBSD package.sh: accept only executables in the cross-build check Review follow-up. The file(1) check for binaries that cannot run here now needs an executable (file says "executable" or "pie executable") or an ELF with an interpreter, which is how an older file(1) still shows a PIE it calls a shared object. FreeBSD libraries, object files and core files for the target architecture are refused. Part of storytold/craft-common-actions#39. --- packaging/freebsd/package.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index a1ec8271..e50853b8 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -78,11 +78,12 @@ echo "wrote $DIST/$BASENAME.tar.gz" if [ "$ARCH" = "$HOST_ARCH" ]; then "$STAGE/bin/soundcraft-cli" --version else - # They cannot run here, so check what they are instead. + # They cannot run here, so check what they are instead: executables, not libraries or objects. + # An older file(1) calls a PIE a shared object; its interpreter still marks it as a program. want="$ARCH"; [ "$ARCH" != x86_64 ] || want=x86-64 for b in "$STAGE"/bin/*; do case "$(file -b "$b")" in - ELF*"$want"*FreeBSD*) ;; + ELF*" executable, "*"$want"*FreeBSD* | ELF*"$want"*FreeBSD*interpreter*) ;; *) echo "error: $b is not a FreeBSD $ARCH executable: $(file -b "$b")" >&2; exit 1 ;; esac done From 38d62661ce28c4b1fc7e4e95c95a9997aca412ef Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 15:31:37 -0500 Subject: [PATCH 21/38] FreeBSD package.sh: never run a FreeBSD target's binary on another OS Review follow-up. With CARGO_BUILD_TARGET set on a host that is not FreeBSD (say an x86_64 Mac building x86_64-unknown-freebsd), the architectures match but the binary still cannot run, so the --version run is skipped there and the file(1) check is used instead. Part of storytold/craft-common-actions#39. --- packaging/freebsd/package.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index e50853b8..52a122ab 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -43,8 +43,12 @@ case "${CARGO_BUILD_TARGET:-}" in *-unknown-freebsd) ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")" ;; *) echo "CARGO_BUILD_TARGET=$CARGO_BUILD_TARGET is not a FreeBSD target" >&2; exit 2 ;; esac -# Only binaries for this machine run here (the --version check at the end). +# Only binaries for this machine run here (the --version check at the end); a FreeBSD target built +# on another OS never does. HOST_ARCH="$(release_arch "$(uname -m)" 2>/dev/null)" || HOST_ARCH="$(uname -m)" +if [ -n "${CARGO_BUILD_TARGET:-}" ] && [ "$(uname -s)" != FreeBSD ]; then + HOST_ARCH="$(uname -s) $HOST_ARCH" +fi BASENAME="soundcraft-$VERSION-freebsd-$ARCH" echo "==> SoundCraft $VERSION for FreeBSD $ARCH" From d9f2b070cbb0b39a0d6fddf7c43c2b14aeb5abdd Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 16:06:19 -0500 Subject: [PATCH 22/38] FreeBSD package.sh: accept only the two supported FreeBSD targets Review follow-up. CARGO_BUILD_TARGET must be aarch64-unknown-freebsd or x86_64-unknown-freebsd, the two the release builds; anything else, including made-up triples such as aarch64-foo-unknown-freebsd, stops with exit 2. Part of storytold/craft-common-actions#39. --- packaging/freebsd/package.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 52a122ab..285c17a5 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -40,8 +40,8 @@ release_arch() { # aarch64-unknown-freebsd (craft-common-actions#39): name the archive after the target. case "${CARGO_BUILD_TARGET:-}" in "") ARCH="$(release_arch "$(uname -m)")" ;; - *-unknown-freebsd) ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")" ;; - *) echo "CARGO_BUILD_TARGET=$CARGO_BUILD_TARGET is not a FreeBSD target" >&2; exit 2 ;; + aarch64-unknown-freebsd | x86_64-unknown-freebsd) ARCH="$(release_arch "${CARGO_BUILD_TARGET%%-*}")" ;; + *) echo "CARGO_BUILD_TARGET=$CARGO_BUILD_TARGET is not a supported FreeBSD target" >&2; exit 2 ;; esac # Only binaries for this machine run here (the --version check at the end); a FreeBSD target built # on another OS never does. From a36228f0ec44c1c1ed0196b822aa9c1ada11c921 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Thu, 22 Oct 2026 18:02:15 -0600 Subject: [PATCH 23/38] #264: test that the Edit toolbar scrolls horizontally in a narrow window The Edit toolbar laid its tool groups out in a bare row, so in a narrow window the groups on the right (the tempo among them) were clipped and unreachable. The test renders a 700 pt wide window, scrolls the toolbar sideways with the wheel and checks the tempo field is then painted inside the window. Without the horizontal ScrollArea the tempo is never painted, so the assertion fails. --- crates/ui-egui/tests/toolbar_scroll.rs | 51 ++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 crates/ui-egui/tests/toolbar_scroll.rs diff --git a/crates/ui-egui/tests/toolbar_scroll.rs b/crates/ui-egui/tests/toolbar_scroll.rs new file mode 100644 index 00000000..8615e363 --- /dev/null +++ b/crates/ui-egui/tests/toolbar_scroll.rs @@ -0,0 +1,51 @@ +// Regression test for #264: the Edit toolbar clipped its tools in a narrow window; it now scrolls horizontally. +use egui::{Event, Pos2, Shape, vec2}; +use serde_json::json; +use soundcraft_ui_egui::{Services, SoundApp}; + +const WIDTH: f32 = 700.0; + +/// Left edge of the toolbar's tempo field (the last group but one), once painted. +fn tempo_x(h: &egui_kittest::Harness<'_, SoundApp>) -> Option<(f32, f32)> { + h.output().shapes.iter().find_map(|c| match &c.shape { + Shape::Text(t) if t.galley.text() == "120.0000" => { + let r = t.galley.rect.translate(t.pos.to_vec2()); + Some((r.min.x, r.center().y)) + } + _ => None, + }) +} + +#[test] +fn a_narrow_window_scrolls_the_toolbar_to_reach_the_tempo() { + let mut engine = soundcraft_engine::Engine::default(); + engine.execute("track.new", &json!({})).unwrap(); + let app = SoundApp::new(engine, None, Services::default()); + let mut h = egui_kittest::Harness::builder().with_size(vec2(WIDTH, 700.0)).build_ui_state( + |ui, app: &mut SoundApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + app, + ); + for _ in 0..4 { + h.step(); + } + // Off the right edge, so egui culls it (or paints it beyond the window). + assert!(tempo_x(&h).is_none_or(|(x, _)| x > WIDTH), "the tempo should start off-screen in a {WIDTH} pt window"); + let y = 40.0; + + // Wheel over the toolbar (a vertical wheel does not scroll a horizontal-only area, a horizontal one does). + for _ in 0..12 { + h.event(Event::PointerMoved(Pos2::new(WIDTH / 2.0, y))); + h.event(Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(-300.0, 0.0), + modifiers: egui::Modifiers::NONE, + phase: egui::TouchPhase::Move, + }); + h.step(); + } + let at = tempo_x(&h).map(|(x, _)| x); + assert!(at.is_some_and(|x| x < WIDTH - 20.0), "scrolling the toolbar must bring the tempo into the window, it is at {at:?}"); +} From 9049eb53c5d0f474ffe86cb126413645a26eae9d Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 23 Oct 2026 02:08:38 +0200 Subject: [PATCH 24/38] Save Session: a bare file name is checked as the working directory against the write root (fixes #869) --- apps/soundcraft-cli/tests/cli.rs | 31 +++++++++++++++++++++++++++++-- crates/engine/src/io.rs | 5 ++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/apps/soundcraft-cli/tests/cli.rs b/apps/soundcraft-cli/tests/cli.rs index ef2fb881..35109095 100644 --- a/apps/soundcraft-cli/tests/cli.rs +++ b/apps/soundcraft-cli/tests/cli.rs @@ -3,7 +3,7 @@ #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] use std::io::Write; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; use serde_json::Value; @@ -28,7 +28,16 @@ fn tmpdir(tag: &str) -> PathBuf { /// Send `calls` (tool name, arguments) to `soundcraft-cli mcp `; returns whether it exited /// cleanly, its stderr, and the stdout lines after `initialize`, one per call. fn mcp_session(args: &[&str], calls: &[(&str, Value)]) -> (bool, String, Vec) { - let mut child = bin().arg("mcp").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + mcp_session_in(None, args, calls) +} + +/// [`mcp_session`] with the server started in the working directory `cwd`. +fn mcp_session_in(cwd: Option<&Path>, args: &[&str], calls: &[(&str, Value)]) -> (bool, String, Vec) { + let mut cmd = bin(); + if let Some(cwd) = cwd { + cmd.current_dir(cwd); + } + let mut child = cmd.arg("mcp").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); { let stdin = child.stdin.as_mut().unwrap(); writeln!(stdin, r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18"}}}}"#).unwrap(); @@ -102,6 +111,24 @@ fn mcp_automation_roots_confine_the_server() { let _ = std::fs::remove_dir_all(base); } +#[test] +fn a_bare_save_session_name_lands_in_the_working_directory_inside_the_roots() { + let work = tmpdir("bare-name"); + let work_s = work.to_str().unwrap(); + let calls = [ + ("new_session", serde_json::json!({"name": "t", "sample_rate": 48000})), + ("execute", serde_json::json!({"command": "track.new", "params": {"kind": "audio", "name": "Tone", "format": "mono"}})), + ("save_session", serde_json::json!({"path": "t.scraft"})), + ]; + let (success, stderr, replies) = mcp_session_in(Some(&work), &["--automation-read-root", work_s, "--automation-write-root", work_s], &calls); + assert!(success, "{stderr}"); + for reply in &replies { + assert_eq!(reply["result"]["isError"], false, "{reply}"); + } + assert!(work.join("t.scraft").is_file()); + let _ = std::fs::remove_dir_all(work); +} + #[test] fn mcp_automation_root_flags_are_checked_up_front() { let base = tmpdir("roots-args"); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index eff105ec..6f53af5a 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -80,7 +80,10 @@ pub fn stored_path(stored: &str, session: &Path) -> std::result::Result) -> Result<()> { let dir = dir.as_ref(); - crate::file_access::check_write(dir).map_err(EngineError::Io)?; + // The empty folder of a bare file name ("t.scraft") is the working directory, which the root + // check would refuse as an empty path. + let checked = if dir.as_os_str().is_empty() { Path::new(".") } else { dir }; + crate::file_access::check_write(checked).map_err(EngineError::Io)?; std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display()))) } From 44663d9601f69583c5d8bc148bf98598b3a683d1 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Thu, 22 Oct 2026 19:13:26 -0600 Subject: [PATCH 25/38] #264: require the whole tempo field visible and unclipped The test only checked the tempo text's left edge, so a field cut off on either side would still pass. It now checks that the whole text lies inside the window and inside the clip rect it is painted with. With the toolbar's scrolling turned off it still fails. --- crates/ui-egui/tests/toolbar_scroll.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/tests/toolbar_scroll.rs b/crates/ui-egui/tests/toolbar_scroll.rs index 8615e363..3ac616cf 100644 --- a/crates/ui-egui/tests/toolbar_scroll.rs +++ b/crates/ui-egui/tests/toolbar_scroll.rs @@ -5,13 +5,10 @@ use soundcraft_ui_egui::{Services, SoundApp}; const WIDTH: f32 = 700.0; -/// Left edge of the toolbar's tempo field (the last group but one), once painted. -fn tempo_x(h: &egui_kittest::Harness<'_, SoundApp>) -> Option<(f32, f32)> { +/// The toolbar's tempo text (the last group but one) and the clip rect it is painted in, once painted. +fn tempo(h: &egui_kittest::Harness<'_, SoundApp>) -> Option<(egui::Rect, egui::Rect)> { h.output().shapes.iter().find_map(|c| match &c.shape { - Shape::Text(t) if t.galley.text() == "120.0000" => { - let r = t.galley.rect.translate(t.pos.to_vec2()); - Some((r.min.x, r.center().y)) - } + Shape::Text(t) if t.galley.text() == "120.0000" => Some((t.galley.rect.translate(t.pos.to_vec2()), c.clip_rect)), _ => None, }) } @@ -32,7 +29,7 @@ fn a_narrow_window_scrolls_the_toolbar_to_reach_the_tempo() { h.step(); } // Off the right edge, so egui culls it (or paints it beyond the window). - assert!(tempo_x(&h).is_none_or(|(x, _)| x > WIDTH), "the tempo should start off-screen in a {WIDTH} pt window"); + assert!(tempo(&h).is_none_or(|(r, _)| r.min.x > WIDTH), "the tempo should start off-screen in a {WIDTH} pt window"); let y = 40.0; // Wheel over the toolbar (a vertical wheel does not scroll a horizontal-only area, a horizontal one does). @@ -46,6 +43,9 @@ fn a_narrow_window_scrolls_the_toolbar_to_reach_the_tempo() { }); h.step(); } - let at = tempo_x(&h).map(|(x, _)| x); - assert!(at.is_some_and(|x| x < WIDTH - 20.0), "scrolling the toolbar must bring the tempo into the window, it is at {at:?}"); + let at = tempo(&h); + assert!( + at.is_some_and(|(r, clip)| r.min.x >= 0.0 && r.max.x <= WIDTH && clip.contains_rect(r)), + "scrolling the toolbar must bring the whole tempo into the window, unclipped: (text, clip) = {at:?}" + ); } From 811d78729d365b428d10895b723d5f72988afc52 Mon Sep 17 00:00:00 2001 From: justinjohn0306 <34035011+justinjohn0306@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:55:15 +0530 Subject: [PATCH 26/38] Prevent H264 skip-run bounds overflow --- crates/video/src/h264/slicedec.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/video/src/h264/slicedec.rs b/crates/video/src/h264/slicedec.rs index a71a6b4e..34ae0f14 100644 --- a/crates/video/src/h264/slicedec.rs +++ b/crates/video/src/h264/slicedec.rs @@ -13,6 +13,10 @@ use crate::h264::slice::{SliceHeader, SliceType}; use crate::h264::tables::*; use crate::h264::transform::{self, LevelScale}; +fn skip_run_fits(addr: usize, run: usize, total: usize) -> bool { + addr < total && run <= total - addr +} + /// Per-slice data kept with the picture (deblocking, motion field export). #[derive(Clone, Debug, Default)] pub struct SliceInfo { @@ -1444,7 +1448,7 @@ impl<'a> SliceDecoder<'a> { ensure!(addr < total, "macroblock address {addr} out of range"); if !intra_slice { let run = r.read_ue()? as usize; - ensure!(addr + run <= total, "mb_skip_run out of range"); + ensure!(skip_run_fits(addr, run, total), "mb_skip_run out of range"); for _ in 0..run { self.start_mb(addr); self.decode_skip()?; @@ -1467,3 +1471,16 @@ impl<'a> SliceDecoder<'a> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::skip_run_fits; + + #[test] + fn skip_run_must_fit_remaining_macroblocks_without_overflow() { + assert!(skip_run_fits(1, 98, 99)); + assert!(!skip_run_fits(1, 99, 99)); + assert!(!skip_run_fits(1, usize::MAX, 99)); + assert!(!skip_run_fits(99, 0, 99)); + } +} From ce235c6f80388523341f51d437804504b6005c67 Mon Sep 17 00:00:00 2001 From: Royce Date: Fri, 23 Oct 2026 20:09:26 +0800 Subject: [PATCH 27/38] fix(model): saturate the arithmetic in trim_start_to / trim_end_to (#907) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Samples` is `i64`, and these two public APIs did plain subtraction: ```rust let new_start = new_start.min(self.end() - 1); // end() - 1 let delta = new_start - self.start; // extreme new_start vs extreme start self.length -= delta; // -= let new_end = new_end.max(self.start + 1); // start + 1 self.length = new_end - self.start; // extreme new_end vs extreme start ``` Command callers clamp to `MAX_POSITION` first, so nothing in the app trips this today — but the methods are `pub`, and a clip read from a hostile file can already hold any `i64` before `sanitize` runs. An extreme `new_start` against an extreme `start` wraps in release and panics in debug. Each site becomes a saturating operation followed by the same clamp `sanitize` already applies, so the result is identical for every input a command can produce. Test: `trims_saturate_on_extreme_arguments` sweeps `i64::MIN`/`MAX`, `±2 * MAX_POSITION` and the edges, in both directions, asserting positions stay within `±MAX_POSITION` and lengths within `1..=MAX_POSITION`. Verified against the unfixed arithmetic: the test fails with `attempt to subtract with overflow`. cargo test -p soundcraft-model: 41 passed, 0 failed. cargo clippy -p soundcraft-model --lib --no-deps --tests clean. --- crates/model/src/clip.rs | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/model/src/clip.rs b/crates/model/src/clip.rs index 850cf044..11e80b0f 100644 --- a/crates/model/src/clip.rs +++ b/crates/model/src/clip.rs @@ -244,13 +244,16 @@ impl Clip { /// Move the start edge to `new_start` keeping material anchored (trim start). pub fn trim_start_to(&mut self, new_start: Samples) { - let new_start = new_start.min(self.end() - 1); - let delta = new_start - self.start; + // #907: these are public APIs, so they have to survive arguments a command would never + // pass (commands clamp to `MAX_POSITION` first). `Samples` is `i64`, so an extreme + // `new_start` against an extreme `start` wrapped in release and panicked in debug. + let new_start = new_start.clamp(-MAX_POSITION, MAX_POSITION).min(self.end().saturating_sub(1)); + let delta = new_start.saturating_sub(self.start); if let ClipContent::Audio { offset, .. } | ClipContent::Video { offset, .. } = &mut self.content { *offset = offset.saturating_add(delta); } // MIDI note ticks are re-anchored by the engine, which knows the tempo map. - self.length -= delta; + self.length = self.length.saturating_sub(delta).clamp(1, MAX_POSITION); self.start = new_start; if self.sync_point != 0 { self.sync_point = self.sync_point.saturating_sub(delta); @@ -267,8 +270,9 @@ impl Clip { /// Move the end edge to `new_end`. 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; + // #907: saturating, like `trim_start_to`. + let new_end = new_end.clamp(-MAX_POSITION, MAX_POSITION).max(self.start.saturating_add(1)); + self.length = new_end.saturating_sub(self.start).clamp(1, MAX_POSITION); self.drop_stray_sync(); self.clamp_fades(); } @@ -479,4 +483,24 @@ mod tests { c.trim_start_to(900); assert!(c.fade_out.len <= c.length); } + + /// #907: `trim_start_to` / `trim_end_to` are public APIs, so they have to survive arguments a + /// command would never pass. `Samples` is `i64`, so an extreme `new_start` against an extreme + /// `start` wrapped in release and panicked in debug. Every position stays within + /// `±MAX_POSITION` and every length within `1..=MAX_POSITION`. + #[test] + fn trims_saturate_on_extreme_arguments() { + for &new_start in &[Samples::MIN, -(MAX_POSITION * 2), -1, 0, MAX_POSITION, MAX_POSITION * 2, Samples::MAX] { + let mut c = Clip::audio(ClipId(1), "a", SourceId(1), 1000, MAX_POSITION / 2, 1000); + c.trim_start_to(new_start); + assert!((-(MAX_POSITION as i128)..=(MAX_POSITION as i128)).contains(&(c.start as i128)), "start {new_start}: {}", c.start); + assert!((1..=MAX_POSITION).contains(&c.length), "start {new_start}: length {}", c.length); + } + for &new_end in &[Samples::MIN, -(MAX_POSITION * 2), -1, 0, MAX_POSITION, MAX_POSITION * 2, Samples::MAX] { + let mut c = Clip::audio(ClipId(2), "b", SourceId(1), 1000, -(MAX_POSITION / 2), 1000); + c.trim_end_to(new_end); + assert!((1..=MAX_POSITION).contains(&c.length), "end {new_end}: length {}", c.length); + assert!(c.end() - c.start == c.length, "end {new_end}: start {} end {}", c.start, c.end()); + } + } } From 7c690b19e1cc5f310a2d8ad96acfeaba3188105b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:32:47 +0200 Subject: [PATCH 28/38] Follow the desktop light/dark setting for the System theme on Linux (#951) --- Cargo.lock | 1 + crates/ui-egui/Cargo.toml | 3 + crates/ui-egui/src/appearance.rs | 135 +++++++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/theme.rs | 5 +- docs/control-protocol.md | 8 +- 6 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/src/appearance.rs diff --git a/Cargo.lock b/Cargo.lock index eb578a28..228c822a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3870,6 +3870,7 @@ dependencies = [ "soundcraft-video", "soundcraft-vst3-host", "toml", + "zbus", ] [[package]] diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 66158bba..874fd2e1 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -27,6 +27,9 @@ serde_json.workspace = true log.workspace = true image.workspace = true +[target.'cfg(target_os = "linux")'.dependencies] +zbus = "5" + [dev-dependencies] egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } diff --git a/crates/ui-egui/src/appearance.rs b/crates/ui-egui/src/appearance.rs new file mode 100644 index 00000000..da917bfe --- /dev/null +++ b/crates/ui-egui/src/appearance.rs @@ -0,0 +1,135 @@ +//! The desktop's dark/light setting for the System theme on Linux, where eframe/winit reports no +//! system theme to egui. +//! +//! The XDG settings portal (`org.freedesktop.appearance` / `color-scheme`) is read once on a worker +//! thread, which then follows its `SettingChanged` signal and wakes the UI only when the value +//! changes. Nothing is polled. Elsewhere egui's own report (`Context::system_theme`) is enough. + +/// egui's own report when it has one, otherwise the desktop's. +pub fn resolve(reported: Option, desktop: impl FnOnce() -> Option) -> Option { + reported.or_else(desktop) +} + +/// The desktop's appearance, or `None` when it is unknown or this platform has no portal. +/// Unit tests never ask the real desktop, so they do not depend on the machine's setting. +pub fn desktop_theme(ctx: &egui::Context) -> Option { + #[cfg(all(target_os = "linux", not(test)))] + { + linux::current(ctx) + } + #[cfg(any(not(target_os = "linux"), test))] + { + let _ = ctx; + None + } +} + +#[cfg(target_os = "linux")] +#[cfg_attr(test, allow(dead_code))] +mod linux { + use std::sync::atomic::{AtomicU8, Ordering}; + use std::sync::{Arc, OnceLock}; + + const PORTAL_NAMESPACE: &str = "org.freedesktop.appearance"; + const PORTAL_KEY: &str = "color-scheme"; + + /// The last `color-scheme` seen: 0 = unknown or no preference, 1 = dark, 2 = light. + static STATE: OnceLock> = OnceLock::new(); + + pub fn current(ctx: &egui::Context) -> Option { + let state = STATE.get_or_init(|| { + let value = Arc::new(AtomicU8::new(0)); + let (worker_value, worker_ctx) = (Arc::clone(&value), ctx.clone()); + // If the thread cannot start, the value stays 0 and System keeps following egui only. + let _ = std::thread::Builder::new().name("appearance-portal".into()).spawn(move || watch(&worker_value, &worker_ctx)); + value + }); + decode(u32::from(state.load(Ordering::Relaxed))) + } + + fn portal_proxy() -> Option> { + let conn = zbus::blocking::connection::Builder::session().ok()?.method_timeout(std::time::Duration::from_secs(2)).build().ok()?; + zbus::blocking::Proxy::new(&conn, "org.freedesktop.portal.Desktop", "/org/freedesktop/portal/desktop", "org.freedesktop.portal.Settings").ok() + } + + fn watch(value: &AtomicU8, ctx: &egui::Context) { + let Some(proxy) = portal_proxy() else { return }; + // Subscribe before Read so a change during startup is queued rather than lost. + let Ok(signals) = proxy.receive_signal("SettingChanged") else { return }; + let initial: Option = proxy.call("Read", &(PORTAL_NAMESPACE, PORTAL_KEY)).ok(); + publish(value, ctx, initial.and_then(portal_code).and_then(decode)); + // Blocks until the portal sends a signal: no polling. + for message in signals { + let Ok((namespace, key, changed)) = message.body().deserialize::<(String, String, zbus::zvariant::OwnedValue)>() else { + continue; + }; + if namespace == PORTAL_NAMESPACE && key == PORTAL_KEY { + publish(value, ctx, portal_code(changed).and_then(decode)); + } + } + } + + fn publish(value: &AtomicU8, ctx: &egui::Context, theme: Option) { + if value.swap(code_of(theme), Ordering::Relaxed) != code_of(theme) { + ctx.request_repaint(); + } + } + + /// The `u32` inside a portal value. `Settings.Read` answers a variant, and some portals nest + /// another variant inside it. + fn portal_code(value: zbus::zvariant::OwnedValue) -> Option { + let mut value: zbus::zvariant::Value<'_> = value.into(); + for _ in 0..4 { + match value { + zbus::zvariant::Value::Value(inner) => value = *inner, + other => return u32::try_from(other).ok(), + } + } + None + } + + /// The portal's `color-scheme`: 0 = no preference, 1 = prefer dark, 2 = prefer light. + pub(super) fn decode(code: u32) -> Option { + match code { + 1 => Some(egui::Theme::Dark), + 2 => Some(egui::Theme::Light), + _ => None, + } + } + + fn code_of(theme: Option) -> u8 { + match theme { + Some(egui::Theme::Dark) => 1, + Some(egui::Theme::Light) => 2, + None => 0, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn desktop_setting_is_used_only_when_egui_reports_nothing() { + assert_eq!(resolve(None, || Some(egui::Theme::Light)), Some(egui::Theme::Light)); + assert_eq!(resolve(None, || None), None); + assert_eq!(resolve(Some(egui::Theme::Dark), || Some(egui::Theme::Light)), Some(egui::Theme::Dark)); + let asked = std::cell::Cell::new(false); + let theme = resolve(Some(egui::Theme::Light), || { + asked.set(true); + None + }); + assert_eq!(theme, Some(egui::Theme::Light)); + assert!(!asked.get(), "must not ask the desktop"); + } + + #[cfg(target_os = "linux")] + #[test] + fn portal_color_scheme_codes() { + assert_eq!(linux::decode(1), Some(egui::Theme::Dark)); + assert_eq!(linux::decode(2), Some(egui::Theme::Light)); + assert_eq!(linux::decode(0), None); + assert_eq!(linux::decode(7), None); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3fdd9892..60b789b9 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -4,6 +4,7 @@ //! executing commands. It can be replaced without touching anything below it. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +pub mod appearance; pub mod bounce; pub mod control; pub mod credits; diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 0cf5b9bd..1b4541aa 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -393,12 +393,13 @@ pub fn is_light() -> bool { LIGHT_THEME.load(Ordering::Relaxed) } -/// Resolve each frame from the integration's current OS appearance. Unknown uses Dark. +/// Resolve each frame from the integration's current OS appearance, or on Linux the desktop's own +/// setting when egui reports none. Unknown uses Dark. pub fn wants_light(ctx: &egui::Context, mode: ThemeMode) -> bool { match mode { ThemeMode::Light => true, ThemeMode::Dark => false, - ThemeMode::System => ctx.system_theme() == Some(egui::Theme::Light), + ThemeMode::System => crate::appearance::resolve(ctx.system_theme(), || crate::appearance::desktop_theme(ctx)) == Some(egui::Theme::Light), } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9efe3789..0ece1140 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -83,9 +83,11 @@ script events such as `{"system_theme": "dark"}` to change it on subsequent fram Appearance defaults to Dark. `ui.theme {"mode":"system"}` follows the appearance supplied by the native or web integration at launch and on later frames, while the saved choice stays System. -When the integration cannot report an appearance, System uses Dark. Manual Light and Dark stay -fixed when the OS appearance changes. An offscreen render without `--system-theme` supplies no -OS appearance and therefore resolves System to Dark. +When the integration cannot report an appearance, as on Linux, System reads the desktop's +settings portal (`org.freedesktop.appearance` `color-scheme`) and follows its changes; without +a portal answer it uses Dark. Manual Light and Dark stay fixed when the OS appearance changes. +An offscreen render without `--system-theme` supplies no OS appearance and therefore resolves +System to Dark, or on Linux to the desktop's portal setting when one is available. ## Panel docking From d01abd0e4eae234cbae7770c5f00b4c53143003d Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 23 Oct 2026 18:18:05 +0200 Subject: [PATCH 29/38] MCP: --automation-read-root and --automation-write-root may each be given more than once (fixes #874) --- apps/soundcraft-cli/src/automation_args.rs | 56 ++++---- apps/soundcraft-cli/src/main.rs | 19 +-- apps/soundcraft-cli/tests/cli.rs | 55 +++++++- crates/automation/tests/automation_roots.rs | 4 +- crates/engine/src/file_access.rs | 149 ++++++++++++++------ docs/mcp.md | 25 ++-- 6 files changed, 216 insertions(+), 92 deletions(-) diff --git a/apps/soundcraft-cli/src/automation_args.rs b/apps/soundcraft-cli/src/automation_args.rs index f874226f..e8135f91 100644 --- a/apps/soundcraft-cli/src/automation_args.rs +++ b/apps/soundcraft-cli/src/automation_args.rs @@ -3,30 +3,22 @@ use std::path::PathBuf; +#[derive(Default)] pub struct AutomationArgs { - pub read_root: Option, - pub write_root: Option, - read_given: bool, - write_given: bool, + pub read_roots: Vec, + pub write_roots: Vec, } impl AutomationArgs { - pub fn new(read_root: Option, write_root: Option) -> Self { - Self { read_root, write_root, read_given: false, write_given: false } - } - - /// Returns whether this argument is a root flag. A second occurrence of a flag is an error, - /// including when the two spellings (`--flag DIR`, `--flag=DIR`) are mixed. + /// Returns whether this argument is a root flag. Each flag may be given more than once (#874), + /// in either spelling (`--flag DIR`, `--flag=DIR`): every use adds a folder. pub fn consume(&mut self, arg: &str, args: &mut impl Iterator) -> Result { let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value))); - let (root, given) = match flag { - "--automation-read-root" => (&mut self.read_root, &mut self.read_given), - "--automation-write-root" => (&mut self.write_root, &mut self.write_given), + let roots = match flag { + "--automation-read-root" => &mut self.read_roots, + "--automation-write-root" => &mut self.write_roots, _ => return Ok(false), }; - if *given { - return Err(format!("{flag} given twice")); - } let value = match inline { Some(value) => value.to_owned(), None => { @@ -40,8 +32,7 @@ impl AutomationArgs { if value.is_empty() { return Err(format!("{flag} needs a non-empty folder")); } - *root = Some(value.into()); - *given = true; + roots.push(value.into()); Ok(true) } } @@ -51,40 +42,43 @@ mod tests { use super::*; #[test] - fn roots_are_taken_once_and_other_arguments_are_left() { - let mut roots = AutomationArgs::new(None, None); + fn roots_add_up_and_other_arguments_are_left() { + let mut roots = AutomationArgs::default(); let mut args = ["a folder".to_owned(), "song.scraft".to_owned()].into_iter(); assert!(roots.consume("--automation-read-root", &mut args).unwrap()); assert!(roots.consume("--automation-write-root=out", &mut args).unwrap()); assert!(!roots.consume("--demo", &mut args).unwrap()); - assert_eq!(roots.read_root, Some("a folder".into())); - assert_eq!(roots.write_root, Some("out".into())); + assert_eq!(roots.read_roots, [PathBuf::from("a folder")]); + assert_eq!(roots.write_roots, [PathBuf::from("out")]); assert_eq!(args.next().as_deref(), Some("song.scraft")); - assert!(roots.consume("--automation-read-root=other", &mut args).is_err()); - assert_eq!(roots.read_root, Some("a folder".into())); + // #874: a flag given again adds a folder, in either spelling. + assert!(roots.consume("--automation-read-root=other", &mut args).unwrap()); + assert!(roots.consume("--automation-write-root", &mut ["out2".to_owned()].into_iter()).unwrap()); + assert_eq!(roots.read_roots, [PathBuf::from("a folder"), PathBuf::from("other")]); + assert_eq!(roots.write_roots, [PathBuf::from("out"), PathBuf::from("out2")]); } #[test] fn invalid_operands_leave_roots_unchanged() { for flag in ["--automation-read-root", "--automation-write-root"] { for operand in [None, Some(""), Some("--version"), Some("--connect"), Some("-folder")] { - let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into())); + let mut roots = AutomationArgs { read_roots: vec!["read".into()], write_roots: vec!["write".into()] }; let mut args = operand.map(str::to_owned).into_iter(); assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}"); - assert_eq!(roots.read_root, Some("read".into())); - assert_eq!(roots.write_root, Some("write".into())); + assert_eq!(roots.read_roots, [PathBuf::from("read")]); + assert_eq!(roots.write_roots, [PathBuf::from("write")]); } - let mut roots = AutomationArgs::new(None, None); + let mut roots = AutomationArgs::default(); assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err()); } } #[test] fn explicit_dash_folder_and_relative_dash_folder_are_valid() { - let mut roots = AutomationArgs::new(None, None); + let mut roots = AutomationArgs::default(); assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap()); assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap()); - assert_eq!(roots.read_root, Some("-folder".into())); - assert_eq!(roots.write_root, Some("./-folder".into())); + assert_eq!(roots.read_roots, [PathBuf::from("-folder")]); + assert_eq!(roots.write_roots, [PathBuf::from("./-folder")]); } } diff --git a/apps/soundcraft-cli/src/main.rs b/apps/soundcraft-cli/src/main.rs index 9a51cc54..b21b7838 100644 --- a/apps/soundcraft-cli/src/main.rs +++ b/apps/soundcraft-cli/src/main.rs @@ -8,12 +8,14 @@ //! soundcraft-cli commands [FILTER] list commands //! soundcraft-cli describe ID one command's details //! soundcraft-cli app [--port P] METHOD [JSON] call a running app's control channel -//! soundcraft-cli mcp [--connect PORT] [--demo] [--automation-read-root DIR] [--automation-write-root DIR] -//! MCP server on stdio. The two roots confine every file the +//! soundcraft-cli mcp [--connect PORT] [--demo] [--automation-read-root DIR]... [--automation-write-root DIR]... +//! MCP server on stdio. The roots confine every file the //! agent's commands read (open, import, presets) and write //! (save, bounce, export) to those folders, links followed; -//! a root left out grants none of its access. They run the -//! in-process server, so they conflict with --connect. +//! each flag may be given more than once, a path inside any +//! of its folders being allowed, and a kind left out grants +//! none of its access. They run the in-process server, so +//! they conflict with --connect. //! soundcraft-cli parity [--write PATH] parity report (markdown) //! soundcraft-cli plugins list built-in plugins //! ``` @@ -398,7 +400,7 @@ struct McpArgs { fn parse_mcp_args(args: &[String]) -> Result { let mut connect = None; let mut demo = false; - let mut roots_args = automation_args::AutomationArgs::new(None, None); + let mut roots_args = automation_args::AutomationArgs::default(); let mut it = args.iter().cloned(); while let Some(a) = it.next() { if roots_args.consume(&a, &mut it)? { @@ -411,7 +413,9 @@ fn parse_mcp_args(args: &[String]) -> Result { _ => {} } } - let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?; + let read: Vec<&std::path::Path> = roots_args.read_roots.iter().map(std::path::PathBuf::as_path).collect(); + let write: Vec<&std::path::Path> = roots_args.write_roots.iter().map(std::path::PathBuf::as_path).collect(); + let roots = AutomationRoots::new(&read, &write)?; if roots.is_some() && connect.is_some() { return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \ so drop --connect (or use --connect without the roots)" @@ -437,8 +441,7 @@ fn mcp(args: &[String]) -> ExitCode { if let Some(roots) = roots { // Every thread of this process, not only the server's: nothing reaches past the roots. file_access::confine_process(roots.clone()); - let show = |root: Option<&std::path::Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string()); - eprintln!("soundcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root())); + eprintln!("soundcraft-cli: files confined: {}", roots.describe()); h = h.with_automation_roots(Some(roots)); } let mut s = Server::new(h); diff --git a/apps/soundcraft-cli/tests/cli.rs b/apps/soundcraft-cli/tests/cli.rs index ef2fb881..1e84a4fb 100644 --- a/apps/soundcraft-cli/tests/cli.rs +++ b/apps/soundcraft-cli/tests/cli.rs @@ -102,6 +102,56 @@ fn mcp_automation_roots_confine_the_server() { let _ = std::fs::remove_dir_all(base); } +/// #874: each root flag may be given more than once. A path inside any root of its kind passes; +/// the two roots' common parent, `..` out of a root and a link out of one stay refused. +#[test] +fn mcp_automation_roots_take_several_folders() { + // Resolved, as the server shows the roots (`/var` is `/private/var` on macOS). + let base = std::fs::canonicalize(tmpdir("several")).unwrap(); + for d in ["in", "in2", "out", "out2", "outside"] { + std::fs::create_dir_all(base.join(d)).unwrap(); + } + for f in ["in/a.wav", "in2/b.wav", "outside/secret.wav", "parent.wav"] { + make_wav(&base.join(f)); + } + let p = |f: &str| base.join(f).to_str().unwrap().to_string(); + let sep = std::path::MAIN_SEPARATOR; + let calls = [ + ("import_audio", serde_json::json!({"path": p("in/a.wav")})), + ("import_audio", serde_json::json!({"path": p("in2/b.wav")})), + // The roots' common parent is not a root. + ("import_audio", serde_json::json!({"path": p("parent.wav")})), + ("import_audio", serde_json::json!({"path": format!("{}{sep}..{sep}outside{sep}secret.wav", p("in2"))})), + ("bounce_mix", serde_json::json!({"path": p("out/a.wav")})), + ("bounce_mix", serde_json::json!({"path": p("out2/b.wav")})), + ("bounce_mix", serde_json::json!({"path": p("x.wav")})), + // Read and write stay separate: a read root is not a write root, nor the other way round. + ("bounce_mix", serde_json::json!({"path": p("in2/c.wav")})), + ("import_audio", serde_json::json!({"path": p("out2/b.wav")})), + ]; + let read2 = format!("--automation-read-root={}", p("in2")); + let args = ["--automation-read-root", &p("in"), &read2, "--automation-write-root", &p("out"), "--automation-write-root", &p("out2")]; + let (success, stderr, replies) = mcp_session(&args, &calls); + assert!(success, "{stderr}"); + assert!(stderr.contains(&format!("read {}, {}", p("in"), p("in2"))), "{stderr}"); + let errors: Vec = replies.iter().map(|r| r["result"]["isError"] == true).collect(); + assert_eq!(errors, [false, false, true, true, false, false, true, true, true], "{replies:?}"); + for (i, kind) in [(2, "read"), (3, "read"), (6, "write"), (7, "write"), (8, "read")] { + assert!(text(&replies[i]).contains(&format!("outside the {kind} roots ")), "{}", text(&replies[i])); + } + assert!(base.join("out/a.wav").is_file() && base.join("out2/b.wav").is_file()); + assert!(!base.join("x.wav").exists() && !base.join("in2/c.wav").exists()); + + // A link in one root that leads out of all of them is outside. + #[cfg(unix)] + { + std::os::unix::fs::symlink(base.join("outside/secret.wav"), base.join("in2/leak.wav")).unwrap(); + let (_, _, replies) = mcp_session(&args, &[("import_audio", serde_json::json!({"path": p("in2/leak.wav")}))]); + assert!(text(&replies[0]).contains("outside the read roots"), "{}", text(&replies[0])); + } + let _ = std::fs::remove_dir_all(base); +} + #[test] fn mcp_automation_root_flags_are_checked_up_front() { let base = tmpdir("roots-args"); @@ -118,8 +168,9 @@ fn mcp_automation_root_flags_are_checked_up_front() { // The flags confine the in-process server, so they cannot be combined with --connect. fails(&["--connect", "7979", "--automation-read-root", dir], "--connect"); fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect"); - fails(&["--automation-read-root", dir, "--automation-read-root", dir], "given twice"); - fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice"); + // #874: a flag may be given again, but every folder it names must exist. + fails(&["--automation-read-root", dir, "--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); + fails(&["--automation-write-root", dir, "--automation-write-root"], "needs a folder"); fails(&["--automation-read-root"], "needs a folder"); fails(&["--automation-write-root", "--demo"], "needs a folder"); fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); diff --git a/crates/automation/tests/automation_roots.rs b/crates/automation/tests/automation_roots.rs index 2c4fa160..5f342560 100644 --- a/crates/automation/tests/automation_roots.rs +++ b/crates/automation/tests/automation_roots.rs @@ -58,7 +58,7 @@ fn fixture(name: &str) -> Fixture { } fn server(read: Option<&Path>, write: Option<&Path>) -> Server { - let roots = AutomationRoots::new(read, write).unwrap(); + let roots = AutomationRoots::new(read.as_slice(), write.as_slice()).unwrap(); Server::new(Headless::new(Engine::default()).with_automation_roots(roots)) } @@ -187,7 +187,7 @@ fn writes(dir: &Path) -> Vec<(&'static str, Value)> { } fn demo_server(read: Option<&Path>, write: Option<&Path>) -> Server { - let roots = AutomationRoots::new(read, write).unwrap(); + let roots = AutomationRoots::new(read.as_slice(), write.as_slice()).unwrap(); Server::new(Headless::new(soundcraft_engine::demo::demo_engine()).with_automation_roots(roots)) } diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs index 1f1a0c5f..7f4cda88 100644 --- a/crates/engine/src/file_access.rs +++ b/crates/engine/src/file_access.rs @@ -1,15 +1,16 @@ //! The folders automation may read and write. //! //! `soundcraft-cli mcp --automation-read-root --automation-write-root ` confines the -//! files an agent's commands reach. [`AutomationRoots`] holds the two folders. While they are in -//! force, every path a command reads goes through [`check_read`] and every path it writes through [`check_write`]: +//! files an agent's commands reach; each flag may be given more than once (#874). [`AutomationRoots`] +//! holds the folders of each kind. While they are in force, every path a command reads goes through +//! [`check_read`] and every path it writes through [`check_write`]: //! //! - The path is made absolute against the working directory, as the file system reads it. Its //! links are followed: for a file that doesn't exist yet, the links of its nearest existing //! folder, the rest of the path being plain names (`..` below a missing folder is refused, and so //! is a link that leads nowhere). -//! - The result must lie inside the root, compared by whole components (`/work2` isn't inside -//! `/work`), without regard to case on Windows. +//! - The result must lie inside one of the roots of its kind, compared by whole components +//! (`/work2` isn't inside `/work`), without regard to case on Windows. //! - Read and write access are separate: a root given alone grants only its own kind of access. //! //! Roots are in force on a thread inside [`confine`], and in the whole process after @@ -32,11 +33,11 @@ pub enum Access { Write, } -/// The folders automation may read and write; a root left out grants no access of its kind. +/// The folders automation may read and write; a kind with no root grants no access of its kind. #[derive(Debug)] pub struct AutomationRoots { - read: Option, - write: Option, + read: Vec, + write: Vec, } #[derive(Debug)] @@ -62,48 +63,61 @@ impl Root { } impl AutomationRoots { - /// The roots `read` and `write`, which must be existing folders; `None` when neither is given - /// (nothing is confined then). - pub fn new(read: Option<&Path>, write: Option<&Path>) -> Result>, String> { - if read.is_none() && write.is_none() { + /// The roots `read` and `write`, which must be existing folders (one given twice counts once); + /// `None` when neither kind has any (nothing is confined then). + pub fn new(read: &[&Path], write: &[&Path]) -> Result>, String> { + if read.is_empty() && write.is_empty() { return Ok(None); } - let read = read.map(|p| Root::open(p, Access::Read)).transpose()?; - let write = write.map(|p| Root::open(p, Access::Write)).transpose()?; - Ok(Some(Arc::new(Self { read, write }))) + let open = |paths: &[&Path], access: Access| -> Result, String> { + let mut roots: Vec = Vec::with_capacity(paths.len()); + for path in paths { + let root = Root::open(path, access)?; + if !roots.iter().any(|r| r.resolved == root.resolved) { + roots.push(root); + } + } + Ok(roots) + }; + Ok(Some(Arc::new(Self { read: open(read, Access::Read)?, write: open(write, Access::Write)? }))) } - /// The read root, absolute with its links followed. - pub fn read_root(&self) -> Option<&Path> { - self.read.as_ref().map(|r| r.shown.as_path()) + /// The roots for people: `read A, B; write C` (`none` for a kind without any). + pub fn describe(&self) -> String { + let list = |roots: &[Root]| { + if roots.is_empty() { + return "none".to_string(); + } + roots.iter().map(|r| r.shown.display().to_string()).collect::>().join(", ") + }; + format!("read {}; write {}", list(&self.read), list(&self.write)) } - /// The write root, absolute with its links followed. - pub fn write_root(&self) -> Option<&Path> { - self.write.as_ref().map(|r| r.shown.as_path()) - } - - /// May the file or folder at `path` be read (or written)? `Err` says why not. + /// May the file or folder at `path` be read (or written)? It must lie inside one of the roots + /// of that kind. `Err` says why not. pub fn check(&self, path: impl AsRef, access: Access) -> Result<(), String> { let path = path.as_ref(); let shown_path = path.display(); - let root = match access { - Access::Read => self.read.as_ref(), - Access::Write => self.write.as_ref(), + let roots = match access { + Access::Read => &self.read, + Access::Write => &self.write, }; - let Some(Root { resolved: folder, shown }) = root else { + if roots.is_empty() { return match access { Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {shown_path}")), Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {shown_path}")), }; - }; - if inside(&resolve(path)?, folder) { + } + let resolved = resolve(path)?; + if roots.iter().any(|r| inside(&resolved, &r.resolved)) { return Ok(()); } - let root = shown.display(); - match access { - Access::Read => Err(format!("automation path rejected: outside the read root {root}: {shown_path}")), - Access::Write => Err(format!("automation path rejected: outside the write root {root}: {shown_path}")), + let shown: Vec = roots.iter().map(|r| r.shown.display().to_string()).collect(); + match (shown.as_slice(), access) { + ([root], Access::Read) => Err(format!("automation path rejected: outside the read root {root}: {shown_path}")), + ([root], Access::Write) => Err(format!("automation path rejected: outside the write root {root}: {shown_path}")), + (_, Access::Read) => Err(format!("automation path rejected: outside the read roots {}: {shown_path}", shown.join(", "))), + (_, Access::Write) => Err(format!("automation path rejected: outside the write roots {}: {shown_path}", shown.join(", "))), } } } @@ -280,7 +294,7 @@ mod tests { } std::fs::write(base.join("in/a.scraft"), "x").unwrap(); std::fs::write(base.join("outside/secret.scraft"), "x").unwrap(); - let roots = AutomationRoots::new(Some(&base.join("in")), Some(&base.join("out"))).unwrap().unwrap(); + let roots = AutomationRoots::new(&[&base.join("in")], &[&base.join("out")]).unwrap().unwrap(); (base, roots) } @@ -340,18 +354,18 @@ mod tests { #[test] fn a_missing_root_is_an_error_and_none_is_no_confinement() { let (base, _) = folders("missing"); - let e = AutomationRoots::new(Some(&base.join("nope")), None).unwrap_err(); + let e = AutomationRoots::new(&[&base.join("nope")], &[]).unwrap_err(); assert!(e.starts_with("cannot open the automation read root"), "{e}"); - let e = AutomationRoots::new(None, Some(&base.join("in/a.scraft"))).unwrap_err(); + let e = AutomationRoots::new(&[], &[&base.join("in/a.scraft")]).unwrap_err(); assert!( e.starts_with("cannot open the automation write root") && e.ends_with(&std::io::Error::from(std::io::ErrorKind::NotADirectory).to_string()), "{e}" ); - assert!(AutomationRoots::new(Some(Path::new("")), None).unwrap_err().starts_with("cannot open the automation read root ``")); - assert!(AutomationRoots::new(None, None).unwrap().is_none()); + assert!(AutomationRoots::new(&[Path::new("")], &[]).unwrap_err().starts_with("cannot open the automation read root ``")); + assert!(AutomationRoots::new(&[], &[]).unwrap().is_none()); // One root alone grants only its own access. - let read_only = AutomationRoots::new(Some(&base.join("in")), None).unwrap().unwrap(); + let read_only = AutomationRoots::new(&[&base.join("in")], &[]).unwrap().unwrap(); let e = read_only.check(s(base.join("in/x.scraft")), Access::Write).unwrap_err(); assert!(e.contains("write authority is absent"), "{e}"); let _ = std::fs::remove_dir_all(base); @@ -388,11 +402,66 @@ mod tests { assert_eq!(roots.check(s(base.join("in/alias/x.scraft")), Access::Read), Ok(())); // A root given through a link is the folder it leads to. assert!(made(&base.join("in"), &base.join("in-link"))); - let linked = AutomationRoots::new(Some(&base.join("in-link")), None).unwrap().unwrap(); + let linked = AutomationRoots::new(&[&base.join("in-link")], &[]).unwrap().unwrap(); assert_eq!(linked.check(s(base.join("in/a.scraft")), Access::Read), Ok(())); let _ = std::fs::remove_dir_all(base); } + /// #874: several roots of a kind: a path inside any of them passes, one inside none is refused + /// naming them all, and read and write stay separate. + #[test] + fn several_roots_of_a_kind_each_grant_their_access() { + let (base, _) = folders("several"); + std::fs::create_dir_all(base.join("out2")).unwrap(); + std::fs::write(base.join("in2/b.scraft"), "x").unwrap(); + std::fs::write(base.join("parent.scraft"), "x").unwrap(); + let roots = AutomationRoots::new(&[&base.join("in"), &base.join("in2")], &[&base.join("out"), &base.join("out2")]).unwrap().unwrap(); + assert_eq!(roots.check(s(base.join("in/a.scraft")), Access::Read), Ok(())); + assert_eq!(roots.check(s(base.join("in2/b.scraft")), Access::Read), Ok(())); + let e = roots.check(s(base.join("outside/secret.scraft")), Access::Read).unwrap_err(); + assert!(e.starts_with("automation path rejected: outside the read roots ") && e.contains(", "), "{e}"); + // Two sibling roots don't make their parent a root. + assert!(roots.check(s(base.join("parent.scraft")), Access::Read).unwrap_err().contains("outside the read roots")); + assert!(roots.check(s(base.clone()), Access::Read).is_err()); + assert_eq!(roots.check(s(base.join("out/x.wav")), Access::Write), Ok(())); + assert_eq!(roots.check(s(base.join("out2/new/x.wav")), Access::Write), Ok(())); + assert!(roots.check(s(base.join("outside/x.wav")), Access::Write).unwrap_err().contains("outside the write roots")); + assert!(roots.check(s(base.join("x.wav")), Access::Write).is_err()); + assert!(roots.check(s(base.join("in2/b.scraft")), Access::Write).is_err()); + assert!(roots.check(s(base.join("out2/x.wav")), Access::Read).is_err()); + // `..` out of one root is outside, also when it lands next to another root. + let sep = std::path::MAIN_SEPARATOR; + let up = format!("{}{sep}..{sep}outside{sep}secret.scraft", s(base.join("in2"))); + assert!(roots.check(up, Access::Read).is_err()); + // `..` from one root into another stays inside. + let across = format!("{}{sep}..{sep}in{sep}a.scraft", s(base.join("in2"))); + assert_eq!(roots.check(across, Access::Read), Ok(())); + // A folder given twice (here once through `..`) counts once. + let twice = AutomationRoots::new(&[&base.join("in"), &base.join("in/sub/..")], &[]).unwrap().unwrap(); + assert!(!twice.describe().contains(", ") && twice.describe().ends_with("; write none"), "{}", twice.describe()); + // A missing folder among several is an error. + let e = AutomationRoots::new(&[&base.join("in"), &base.join("nope")], &[]).unwrap_err(); + assert!(e.starts_with("cannot open the automation read root"), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + + /// A link inside one root that leads into another root of the same kind stays inside; one that + /// leads out of all of them is outside. + #[cfg(unix)] + #[test] + fn links_between_several_roots() { + let (base, _) = folders("several-links"); + std::fs::write(base.join("in2/b.scraft"), "x").unwrap(); + std::os::unix::fs::symlink(base.join("in2/b.scraft"), base.join("in/to-in2.scraft")).unwrap(); + std::os::unix::fs::symlink(base.join("outside/secret.scraft"), base.join("in2/leak.scraft")).unwrap(); + std::os::unix::fs::symlink(&base, base.join("in2/up")).unwrap(); + let roots = AutomationRoots::new(&[&base.join("in"), &base.join("in2")], &[]).unwrap().unwrap(); + assert_eq!(roots.check(s(base.join("in/to-in2.scraft")), Access::Read), Ok(())); + assert!(roots.check(s(base.join("in2/leak.scraft")), Access::Read).unwrap_err().contains("outside the read roots")); + assert!(roots.check(s(base.join("in2/up/outside/secret.scraft")), Access::Read).unwrap_err().contains("outside the read roots")); + let _ = std::fs::remove_dir_all(base); + } + #[cfg(windows)] #[test] fn windows_ignores_case_and_refuses_devices() { diff --git a/docs/mcp.md b/docs/mcp.md index 558852c1..644dfb5e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -21,6 +21,8 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo ```sh soundcraft-cli mcp --automation-read-root /work/project --automation-write-root /work/project/out +soundcraft-cli mcp --automation-read-root /work/project --automation-read-root /work/shared-samples \ + --automation-write-root /work/project/out --automation-write-root /tmp/scratch ``` ```json @@ -31,20 +33,24 @@ soundcraft-cli mcp --automation-read-root /work/project --automation-write-root The two flags (VectorCraft and the other Craft apps take the same ones, so one client config works across the suite) keep the agent to the folders you give it: -- Every file a command reads must lie inside the **read root**: `open_session`, `session.open`, +- Every file a command reads must lie inside a **read root**: `open_session`, `session.open`, `import_audio` and `file.import_audio`, `file.import_midi`, `file.import_video`, `file.import_clip_groups`, `file.import_session_data`, `track.load_preset` and `mix.sampler_load`. The media a session file names is read too, so an opened session whose - audio paths lead outside the read root loads without that audio and lists it under `missing`. -- Every file a command writes must lie inside the **write root**: `save_session`, `session.save`, + audio paths lead outside the read roots loads without that audio and lists it under `missing`. +- Every file a command writes must lie inside a **write root**: `save_session`, `session.save`, `session.save_as`, `session.save_copy`, `session.save_template`, `bounce_mix` and `file.bounce_mix`, `file.bounce_stems`, `file.export_clips`, `file.export_midi`, `file.export_session_text`, `file.export_selected_tracks_as_session`, `file.export_sibelius`, `file.send_to_sibelius`, `file.print_score`, `file.export_clip_groups` and `track.save_preset`, together with what a session save writes beside the session file (`Audio Files/`, `Backups/`) and its temporary file. A score command given no `path` writes to - the temp folder, which is outside the write root unless you made it the root, so it is refused. -- Read and write access are separate. Give the same folder twice for both; a root left out grants + the temp folder, which is outside the write roots unless you made it one, so it is refused. +- Each flag may be given more than once (#874): a path inside any of the read roots may be read, + one inside any of the write roots written, so one session can read a project folder and a shared + samples folder, or write to an output folder and a scratch folder, without widening a root to + their common parent. A folder given twice counts once. +- Read and write access are separate. Give the same folder to both flags for both; a kind left out grants none of its access (with only `--automation-read-root`, nothing can be written to disk). Without either flag nothing changes. - A path is made absolute against the server's working directory, then its links are followed: a @@ -55,12 +61,13 @@ works across the suite) keep the agent to the folders you give it: inside `/work`. On Windows case is ignored, UNC paths (`\\server\share\...`) are resolved like drive-letter ones, and device names (`CON`, `NUL`, `COM1`...) are refused. - A refused path is an ordinary tool error the agent can read and correct, for example - `automation path rejected: outside the write root /work/project/out: /etc/x.wav` or + `automation path rejected: outside the write root /work/project/out: /etc/x.wav` (with several, + `outside the write roots /work/project/out, /tmp/scratch: ...`) or `automation filesystem access is not granted: write authority is absent: /work/project/out/x.wav`. A refused save path is reported as refused, never as "already exists". -- The roots must be existing folders; a missing one stops the server with an error. Each flag is - given once (`--flag ` or `--flag=`), and a misspelled `--automation-...` flag is an - error rather than an unconfined server. +- The roots must be existing folders; a missing one stops the server with an error. Each flag takes + one folder per use (`--flag ` or `--flag=`) and may be used again for more, and a + misspelled `--automation-...` flag is an error rather than an unconfined server. The flags confine the in-process server, so `--connect` together with them is an error: the server would otherwise talk to an app that isn't confined. SoundCraft's desktop app has no equivalent flags From 767feacf72a012023962c73b5b40cb43cf59aea0 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:47:27 +0200 Subject: [PATCH 30/38] Require replace: true for file.export_session_text to overwrite Fixes #929 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file.rs | 21 ++++++++++++++++++++- crates/engine/src/io.rs | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 87bba997..f6931f91 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -110,9 +110,10 @@ pub fn specs() -> Vec { crate::io::write_file(&path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) }), - cmd!(noundo "file.export_session_text", "Session Info as Text...", ["File", "Export"], None, "{path?}", always, |e, p| { + cmd!(noundo "file.export_session_text", "Session Info as Text...", ["File", "Export"], None, "{path?, replace?: false}", always, |e, p| { let text = crate::inspect::session_text(e); if let Some(path) = str_param(p, "path") { + refuse_replacing(e, "file.export_session_text", std::path::Path::new(path), p)?; crate::io::write_file(path, text.as_bytes())?; } Ok(json!({"text": text})) @@ -350,6 +351,24 @@ mod tests { path.to_string_lossy().into_owned() } + #[test] + fn export_session_text_needs_replace_to_overwrite() { + let dir = std::env::temp_dir().join(format!("sc-export-text-replace-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("info.txt"); + let path_str = path.to_string_lossy().into_owned(); + let mut e = Engine::default(); + e.execute("file.export_session_text", &json!({"path": path_str})).unwrap(); + std::fs::write(&path, "keep me").unwrap(); + let err = e.execute("file.export_session_text", &json!({"path": path_str})).unwrap_err(); + assert!(err.to_string().contains("replace: true"), "{err}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "keep me"); + e.execute("file.export_session_text", &json!({"path": path_str, "replace": true})).unwrap(); + assert_ne!(std::fs::read_to_string(&path).unwrap(), "keep me"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn import_audio_honours_new_tracks() { let dir = std::env::temp_dir().join(format!("sc-import-new-tracks-{}", std::process::id())); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index eff105ec..341287d3 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1109,7 +1109,7 @@ pub fn replaces(e: &Engine, command: &str, file: &Path) -> bool { "session.save" | "session.save_as" | "session.save_copy" => { file.exists() && !e.path.as_deref().is_some_and(|own| same_path(Path::new(own), file)) } - "session.save_template" | "file.export_selected_tracks_as_session" => file.exists(), + "session.save_template" | "file.export_selected_tracks_as_session" | "file.export_session_text" => file.exists(), _ => false, } } From 0235d11d4cc41a0aebc304b0224e7832747be72b Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:56:25 +0200 Subject: [PATCH 31/38] Don't register a source for a take that a punch range misses Fixes #920 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/io.rs | 58 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index eff105ec..bf36f781 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -1456,13 +1456,18 @@ fn add_recording_in( if s.track(tid).is_none() { continue; } - let resampled = buf.sample_rate != s.sample_rate.hz(); + let hz = s.sample_rate.hz(); + let resampled = buf.sample_rate != hz; + let mut buf = buf; + if resampled && buf.sample_rate > 0 { + buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, hz); + buf.sample_rate = hz; + } let name = path.file_stem().and_then(|x| x.to_str()).unwrap_or("Take"); - let src = add_source(s, name, buf, Some(&path.to_string_lossy()), FileFormat::Wav); - let Some(x) = s.sources.last_mut() else { continue }; - x.unsaved = resampled; - let len = i64::try_from(x.frames).unwrap_or(0); + let len = i64::try_from(buf.frames()).unwrap_or(0); let pass = pass.unwrap_or(len).max(1); + // Work out what each pass keeps first, so a take nothing is placed from isn't registered. + let mut kept_passes = Vec::new(); for offset in (0..len).step_by(usize::try_from(pass).unwrap_or(usize::MAX)).take(1_000) { let n = pass.min(len - offset); if offset > 0 && n < pass / 8 { @@ -1475,6 +1480,16 @@ fn add_recording_in( } None => (start, 0, n), }; + kept_passes.push((offset, clip_start, cut, clip_len)); + } + if kept_passes.is_empty() { + continue; + } + let src = add_source(s, name, buf, Some(&path.to_string_lossy()), FileFormat::Wav); + if let Some(x) = s.sources.last_mut() { + x.unsaved = resampled; + } + for (offset, clip_start, cut, clip_len) in kept_passes { if offset > 0 && let Some(t) = s.track_mut(tid) { @@ -1588,6 +1603,39 @@ mod record_tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn a_punch_that_misses_the_take_leaves_the_session_alone() { + let dir = std::env::temp_dir().join(format!("soundcraft-punch-miss-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut e = Engine::default(); + let vox = armed(&mut e, "Vox", "mono", "In 1"); + let take = dir.join("Audio Files/Take.wav"); + write_take(&take, 1_000); + let shared = add_source( + e.session_mut(), + "Take", + AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.0; 100]] }, + Some(&take.to_string_lossy()), + FileFormat::Wav, + ); + let sources = e.session().sources.len(); + let pool = e.session().pool.len(); + let plan = Recording { + start: 1_000, + pass: None, + punch: Some(Range::new(5_000, 6_000)), + rate: 48_000, + takes: vec![Take { track: vox, path: take.clone(), channels: 1 }], + }; + let placed = place_recording(&mut e, &plan, vec![(vox, take)]); + assert!(placed.is_empty()); + assert_eq!(e.session().sources.len(), sources); + assert_eq!(e.session().pool.len(), pool); + assert!(e.session().sources.iter().any(|x| x.id == shared)); + assert!(e.session().pool.contains(shared)); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn a_take_shows_the_pass_under_way_within_the_punch_while_it_records() { assert_eq!(live_span(1_000, None, None, 0), None); From 9ecf197aa7029b9aa1b7d437ac42dabfda27e564 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:06:48 +0200 Subject: [PATCH 32/38] Restore all engine side-effect state when a command panics Fixes #908 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mod.rs | 23 +++++++++++++++++++++++ crates/engine/src/lib.rs | 20 ++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index b6f379bd..b9d018b7 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -38,6 +38,27 @@ mod view_more; mod vst3; mod zoom_toggle; +#[cfg(test)] +mod panic_probe { + use super::*; + use crate::cmd; + + pub fn specs() -> Vec { + vec![cmd!(noundo "test.panic_probe", "Panic Probe", [], None, "{}", always, probe)] + } + + fn probe(e: &mut Engine, p: &Value) -> Result { + if p.get("panic").is_none() { + return Ok(Value::Null); + } + e.clipboard.length = 123; + e.message("probe"); + e.transport_requests.push(crate::TransportRequest::Play); + e.last_selection = Some((soundcraft_time::Range::new(0, 10), Vec::new())); + panic!("probe") + } +} + pub use file::bounce_job; pub(crate) use edit::remember_selection; @@ -136,6 +157,8 @@ pub fn command_specs() -> &'static [CommandSpec] { v.extend(sampler::specs()); v.extend(video::specs()); v.extend(zoom_toggle::specs()); + #[cfg(test)] + v.extend(panic_probe::specs()); v }) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 03d173dd..e3133291 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -316,6 +316,7 @@ impl Engine { let before = Arc::clone(&self.doc); let rev = self.revision; let dirty = self.dirty; + let side_effects = (self.clipboard.clone(), self.messages.clone(), self.transport_requests.clone(), self.last_selection.clone()); let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (spec.run)(self, params))); let result = match result { Ok(r) => r, @@ -323,6 +324,7 @@ impl Engine { self.doc = before; self.revision = rev.wrapping_add(1); self.dirty = dirty; + (self.clipboard, self.messages, self.transport_requests, self.last_selection) = side_effects; return Err(EngineError::Internal(id.to_string(), panic_message(&*p))); } }; @@ -521,3 +523,21 @@ mod dirty_tests { assert!(!e.is_dirty()); } } + +#[cfg(test)] +mod panic_tests { + use super::*; + + #[test] + fn a_panicking_command_restores_side_effect_state() { + let mut e = Engine::default(); + let (clipboard, messages, requests, selection) = + (e.clipboard.clone(), e.messages.clone(), e.transport_requests.clone(), e.last_selection.clone()); + let r = e.execute("test.panic_probe", &json!({"panic": true})); + assert!(matches!(r, Err(EngineError::Internal(..)))); + assert_eq!(e.clipboard, clipboard); + assert_eq!(e.messages, messages); + assert_eq!(e.transport_requests, requests); + assert_eq!(e.last_selection, selection); + } +} From cc493b2d8b5d8611e2b361be3a4c28c177ac4dda Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:56:02 +0000 Subject: [PATCH 33/38] Keep delay-line storage when the compensated latency changes Fixes #889 Co-Authored-By: Claude Opus 5.5 --- crates/mix/src/lib.rs | 70 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index c04559af..055034b2 100644 --- a/crates/mix/src/lib.rs +++ b/crates/mix/src/lib.rs @@ -248,21 +248,37 @@ struct Strip { /// at most this, whatever its plugins claim, so a bogus value can't stall offline renders. const MAX_LATENCY: usize = 1 << 20; -/// A multichannel delay line (samples), resized only when the delay changes. +/// A multichannel delay line (samples). The rings only grow (keeping their contents), so a delay +/// change in the callback moves the read offset instead of reallocating and zeroing. #[derive(Default)] struct DelayLine { bufs: Vec>, + /// Write position in the rings. idx: usize, + /// Current delay (samples). len: usize, } impl DelayLine { fn set(&mut self, len: usize, ch: usize) { let len = len.min(MAX_LATENCY); - if len != self.len || self.bufs.len() != ch { - self.len = len; - self.idx = 0; - self.bufs = vec![vec![0.0; len]; ch]; + self.len = len; + let cap = self.bufs.first().map_or(0, Vec::len); + if self.bufs.len() != ch { + self.bufs.resize_with(ch, || vec![0.0; cap]); + } + let need = if len == 0 { 0 } else { len + 1 }; + if need > cap { + let new_cap = need.next_power_of_two().min(MAX_LATENCY + 1); + let start = self.idx.min(cap); + for ring in &mut self.bufs { + let mut grown = Vec::with_capacity(new_cap); + grown.extend_from_slice(ring.get(start..).unwrap_or(&[])); + grown.extend_from_slice(ring.get(..start).unwrap_or(&[])); + grown.resize(new_cap, 0.0); + *ring = grown; + } + self.idx = cap; } } fn process(&mut self, io: &mut [Vec], frames: usize) { @@ -270,20 +286,29 @@ impl DelayLine { return; } let start = self.idx; + let mut end = start; for (c, ch) in io.iter_mut().enumerate() { let Some(ring) = self.bufs.get_mut(c) else { continue }; - let mut i = start; + let cap = ring.len(); + if cap <= self.len { + continue; + } + let mut i = start % cap; for x in ch.iter_mut().take(frames) { - if let Some(r) = ring.get_mut(i) { - std::mem::swap(r, x); + if let Some(w) = ring.get_mut(i) { + *w = *x; + } + if let Some(r) = ring.get((i + cap - self.len) % cap) { + *x = *r; } i += 1; - if i >= self.len { + if i >= cap { i = 0; } } + end = i; } - self.idx = (start + frames) % self.len.max(1); + self.idx = end; } fn clear(&mut self) { self.bufs.iter_mut().for_each(|b| b.fill(0.0)); @@ -2303,6 +2328,31 @@ mod tests { s } + #[test] + fn delay_line_change_keeps_buffers_and_history() { + // #889: a latency change must not reallocate or zero the delay line in the callback. + let mut d = DelayLine::default(); + d.set(480, 2); + let ptrs: Vec<_> = d.bufs.iter().map(|b| b.as_ptr()).collect(); + let mut io = vec![(1..=480).map(|v| v as f32).collect::>(); 2]; + d.process(&mut io, 480); + assert!(io.iter().all(|c| c.iter().all(|&x| x == 0.0))); + // Shorter delay: the line keeps its storage and outputs what it already holds. + d.set(240, 2); + let after: Vec<_> = d.bufs.iter().map(|b| b.as_ptr()).collect(); + assert_eq!(ptrs, after, "delay line reallocated on a shorter delay"); + let mut io = vec![vec![0.0f32; 240]; 2]; + d.process(&mut io, 240); + let want: Vec = (241..=480).map(|v| v as f32).collect(); + for c in &io { + assert_eq!(c, &want, "held samples lost on a latency change"); + } + // Back to the longer delay: still the same storage. + d.set(480, 2); + let after: Vec<_> = d.bufs.iter().map(|b| b.as_ptr()).collect(); + assert_eq!(ptrs, after, "delay line reallocated going back to its size"); + } + #[test] fn stored_instrument_params_reach_the_plugin() { // #152: a saved instrument level of -12 dB must render quieter than -6 dB. From 525a4e02fe41d5b144e3a199ce44ffba54897693 Mon Sep 17 00:00:00 2001 From: Samuel Bacaro Date: Fri, 23 Oct 2026 19:28:45 -0300 Subject: [PATCH 34/38] Bound offline render commands: dedupe tracks and a total sample budget AudioSuite-style commands looped over tracks/clips and kept every result as a new pool source (which undo snapshots also hold). Each render was capped per-track, but: - tracks_param did not remove duplicates, so listing the same track id eight times rendered it eight times; - audiosuite.process's signal_generator branch checked only per track, so many distinct tracks could exceed memory in one command. Fixes: - tracks_param now dedupes track ids with a HashSet. - generate() subtracts frames*channels from a MAX_RENDER_SAMPLES budget across all tracks and refuses with "too much audio to generate in one command" before allocating. Regression tests cover duplicate track ids and the cross-track budget. Fixes #896 --- crates/engine/src/cmd/audiosuite.rs | 47 ++++++++++++++++++++++++++++- crates/engine/src/cmd/mod.rs | 8 ++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/audiosuite.rs b/crates/engine/src/cmd/audiosuite.rs index 302d1eb3..7f942841 100644 --- a/crates/engine/src/cmd/audiosuite.rs +++ b/crates/engine/src/cmd/audiosuite.rs @@ -140,9 +140,15 @@ fn process(e: &mut Engine, p: &Value) -> Result { fn generate(e: &mut Engine, p: &Value, name: &str, params: &[(String, f32)], r: Range) -> Result { let sr = e.session().sample_rate; let tracks = tracks_param(e, "audiosuite.process", p)?; + let mut budget = soundcraft_mix::MAX_RENDER_SAMPLES; // Refuse before generating anything, rather than cut the clip short. for t in tracks.iter().filter_map(|t| e.session().track(*t)).filter(|t| t.kind == TrackKind::Audio) { - crate::io::refuse_longer(e.session(), "audiosuite.process", &t.name, "generate on", r.len(), crate::io::render_limit(t.channels()))?; + let channels = t.channels().max(1); + crate::io::refuse_longer(e.session(), "audiosuite.process", &t.name, "generate on", r.len(), crate::io::render_limit(channels))?; + // Each track keeps its generated buffer for the life of the command (and undo holds it), so + // cap the total samples generated across tracks, not just each one. + let frames = usize::try_from(r.len()).unwrap_or(0).min(crate::io::render_limit(channels)); + budget = budget.checked_sub(frames * channels).ok_or_else(|| bad("audiosuite.process", "too much audio to generate in one command"))?; } let mut clips = Vec::new(); for track in tracks { @@ -257,4 +263,43 @@ mod tests { let ranges: Vec<_> = e.session().tracks[0].clips().iter().map(|c| (c.start, c.length)).collect(); assert_eq!(ranges, vec![(0, 32), (32, 64)]); } + + #[test] + fn duplicate_track_ids_generate_once_not_per_listing() { + let mut e = Engine::new(Session::default()); + let t = e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Tone")); + e.execute("edit.select", &json!({"tracks": [t.0], "start": 0, "end": 4800})).unwrap(); + // The same track eight times over: it must generate once, not eight times (#896). + let out = e + .execute( + "audiosuite.process", + &json!({"process": "signal_generator", "tracks": [t.0, t.0, t.0, t.0, t.0, t.0, t.0, t.0], "params": {"freq": 440.0}}), + ) + .unwrap(); + assert_eq!(out["processed"], 1); + assert_eq!(e.session().tracks[0].clips().len(), 1, "one clip, not one per duplicate listing"); + } + + #[test] + fn generate_refuses_a_total_budget_across_tracks() { + let mut e = Engine::new(Session::default()); + // Two tracks each near the per-track limit: the second would push the total past the budget. + for n in 0..2 { + e.session_mut().add_track(TrackKind::Audio, ChannelFormat::Mono, Some(&format!("T{n}"))); + } + let t: Vec<_> = e.session().tracks.iter().map(|t| t.id.0).collect(); + // Select both tracks so signal_generator targets them. + e.execute("edit.select", &json!({"tracks": [t[0], t[1]], "start": 0, "end": 4800})).unwrap(); + e.execute("audiosuite.process", &serde_json::json!({"process": "signal_generator", "track": t[0], "params": {"freq": 440.0}})).unwrap(); + assert_eq!(e.session().tracks[0].clips().len(), 1, "one track generates one clip"); + // Two distinct tracks over the total budget are refused (MAX_RENDER_SAMPLES is f32 samples + // across all channels; a single track over the limit is already refused per-track). + let len = (soundcraft_mix::MAX_RENDER_SAMPLES as i64) / 3 * 2; + let out = e.execute( + "audiosuite.process", + &serde_json::json!({"process": "signal_generator", "tracks": [t[0], t[1]], "params": {"freq": 440.0}, "start": 0, "end": len}), + ); + let ok = matches!(out, Err(crate::EngineError::BadParams(_, ref m)) if m.contains("too long to generate") || m.contains("too much audio")); + assert!(ok, "two tracks over budget, but got: {out:?}"); + } } diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index b6f379bd..a0149556 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -224,13 +224,19 @@ pub fn tracks_param(e: &Engine, cmd: &str, p: &Value) -> Result> { if let Some(arr) = p.get("tracks").and_then(Value::as_array) { let s = e.session(); let mut out = Vec::new(); + let mut seen = std::collections::HashSet::new(); for v in arr { let id = match (v.as_u64(), v.as_str()) { (Some(n), _) => s.track(TrackId(n)).map(|t| t.id), (_, Some(name)) => s.track_by_name(name).map(|t| t.id), _ => None, }; - out.push(id.ok_or_else(|| bad(cmd, format!("unknown track {v}")))?); + let id = id.ok_or_else(|| bad(cmd, format!("unknown track {v}")))?; + // Duplicates multiply the work (each entry renders and keeps its own result), so drop + // repeated ids instead of letting them blow the budget. + if seen.insert(id) { + out.push(id); + } } return Ok(out); } From dfb3815eb3e372bb3dff40c3cf56d8b4d04f11a3 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 00:50:07 -0400 Subject: [PATCH 35/38] Move the #264 toolbar scroll test into the shared tests/it binary Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/{ => it}/toolbar_scroll.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) rename crates/ui-egui/tests/{ => it}/toolbar_scroll.rs (94%) diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index d9eb9fe6..f8fc9354 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -42,6 +42,7 @@ mod send_window; mod shared_controls; mod shortcuts; mod side_lists_access; +mod toolbar_scroll; mod toolbar_tempo; mod track_height; mod track_list; diff --git a/crates/ui-egui/tests/toolbar_scroll.rs b/crates/ui-egui/tests/it/toolbar_scroll.rs similarity index 94% rename from crates/ui-egui/tests/toolbar_scroll.rs rename to crates/ui-egui/tests/it/toolbar_scroll.rs index 3ac616cf..4a6b7c52 100644 --- a/crates/ui-egui/tests/toolbar_scroll.rs +++ b/crates/ui-egui/tests/it/toolbar_scroll.rs @@ -1,4 +1,5 @@ -// Regression test for #264: the Edit toolbar clipped its tools in a narrow window; it now scrolls horizontally. +//! Regression test for #264: the Edit toolbar clipped its tools in a narrow window; it now scrolls horizontally. + use egui::{Event, Pos2, Shape, vec2}; use serde_json::json; use soundcraft_ui_egui::{Services, SoundApp}; From 99579a5421a29c256cbef0e3236786d59f0c6642 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 00:51:56 -0400 Subject: [PATCH 36/38] Move the Scroll Into View test into the shared tests/it binary Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/{ => it}/scroll_into_view.rs | 0 2 files changed, 1 insertion(+) rename crates/ui-egui/tests/{ => it}/scroll_into_view.rs (100%) diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index d9eb9fe6..ba3b2699 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -38,6 +38,7 @@ mod rename_dialog; mod sampler_window; mod save_as_replace; mod save_changes; +mod scroll_into_view; mod send_window; mod shared_controls; mod shortcuts; diff --git a/crates/ui-egui/tests/scroll_into_view.rs b/crates/ui-egui/tests/it/scroll_into_view.rs similarity index 100% rename from crates/ui-egui/tests/scroll_into_view.rs rename to crates/ui-egui/tests/it/scroll_into_view.rs From b0f07331a2103fc081dfaeeabef7b140b9863020 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 00:53:41 -0400 Subject: [PATCH 37/38] Clip::trim_start_to keeps the end when an extreme start would overflow the length Co-Authored-By: Claude Opus 5.5 --- crates/model/src/clip.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/model/src/clip.rs b/crates/model/src/clip.rs index 11e80b0f..f43c10d8 100644 --- a/crates/model/src/clip.rs +++ b/crates/model/src/clip.rs @@ -247,7 +247,9 @@ impl Clip { // #907: these are public APIs, so they have to survive arguments a command would never // pass (commands clamp to `MAX_POSITION` first). `Samples` is `i64`, so an extreme // `new_start` against an extreme `start` wrapped in release and panicked in debug. - let new_start = new_start.clamp(-MAX_POSITION, MAX_POSITION).min(self.end().saturating_sub(1)); + // Never so far back that the length would pass `MAX_POSITION`: trimming the start keeps the end. + let end = self.end(); + let new_start = new_start.max(end.saturating_sub(MAX_POSITION)).clamp(-MAX_POSITION, MAX_POSITION).min(end.saturating_sub(1)); let delta = new_start.saturating_sub(self.start); if let ClipContent::Audio { offset, .. } | ClipContent::Video { offset, .. } = &mut self.content { *offset = offset.saturating_add(delta); @@ -495,6 +497,7 @@ mod tests { c.trim_start_to(new_start); assert!((-(MAX_POSITION as i128)..=(MAX_POSITION as i128)).contains(&(c.start as i128)), "start {new_start}: {}", c.start); assert!((1..=MAX_POSITION).contains(&c.length), "start {new_start}: length {}", c.length); + assert_eq!(c.end(), MAX_POSITION / 2 + 1000, "start {new_start} moved the end"); } for &new_end in &[Samples::MIN, -(MAX_POSITION * 2), -1, 0, MAX_POSITION, MAX_POSITION * 2, Samples::MAX] { let mut c = Clip::audio(ClipId(2), "b", SourceId(1), 1000, -(MAX_POSITION / 2), 1000); From 77d96affc37eb7f36471f9a065b40c2a00dcc17b Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 00:55:05 -0400 Subject: [PATCH 38/38] Panic rollback test: start from non-default side-effect state Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e0604d2c..f19ccb85 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -531,6 +531,11 @@ mod panic_tests { #[test] fn a_panicking_command_restores_side_effect_state() { let mut e = Engine::default(); + // Non-default values, so clearing them instead of restoring them would fail. + e.clipboard.length = 42; + e.messages.push("prior".into()); + e.transport_requests.push(TransportRequest::Stop); + e.last_selection = Some((soundcraft_time::Range::new(20, 30), Vec::new())); let (clipboard, messages, requests, selection) = (e.clipboard.clone(), e.messages.clone(), e.transport_requests.clone(), e.last_selection.clone()); let r = e.execute("test.panic_probe", &json!({"panic": true}));