diff --git a/Cargo.lock b/Cargo.lock index 6158b44b..94af97cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3800,6 +3800,7 @@ dependencies = [ "soundcraft-video", "soundcraft-vst3-host", "toml", + "zbus", ] [[package]] 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..2fc4891e 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 //! ``` @@ -110,7 +112,7 @@ fn info(args: &[String]) -> ExitCode { Ok(b) => b, Err(e) => return fail(format!("{path}: {e}")), }; - let ext = std::path::Path::new(path).extension().and_then(|x| x.to_str()); + let ext = std::path::Path::new(path).extension().and_then(std::ffi::OsStr::to_str); match soundcraft_audio_io::probe(&bytes, ext) { Ok(i) => { outln!("{}", serde_json::to_string_pretty(&json!({"format": i.format, "sample_format": i.sample_format, "sample_rate": i.sample_rate, "channels": i.channels, "frames": i.frames, "seconds": i.duration_secs(), "bwf": i.bwf})).unwrap_or_default()); @@ -134,7 +136,7 @@ fn convert(args: &[String]) -> ExitCode { Ok(b) => b, Err(e) => return fail(format!("{input}: {e}")), }; - let ext = std::path::Path::new(input).extension().and_then(|x| x.to_str()); + let ext = std::path::Path::new(input).extension().and_then(std::ffi::OsStr::to_str); let (_, mut buf) = match soundcraft_audio_io::decode(&bytes, ext) { Ok(x) => x, Err(e) => return fail(format!("{input}: {e}")), @@ -146,7 +148,7 @@ fn convert(args: &[String]) -> ExitCode { if args.iter().any(|a| a == "--normalize") { soundcraft_dsp::offline::normalize(&mut buf.channels, -0.1, false); } - let out_ext = std::path::Path::new(output).extension().and_then(|x| x.to_str()).unwrap_or("wav").to_ascii_lowercase(); + let out_ext = std::path::Path::new(output).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("wav").to_ascii_lowercase(); let Some(format) = soundcraft_audio_io::encode_format_for(&out_ext) else { return fail(format!("{output}: cannot write `.{out_ext}` files (supported: {})", soundcraft_audio_io::ENCODE_EXTENSIONS)); }; @@ -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/it/cli.rs b/apps/soundcraft-cli/tests/it/cli.rs index c10bcf60..ebc92cc5 100644 --- a/apps/soundcraft-cli/tests/it/cli.rs +++ b/apps/soundcraft-cli/tests/it/cli.rs @@ -2,7 +2,7 @@ // Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md). use std::io::Write; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; use serde_json::Value; @@ -27,7 +27,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(); @@ -101,6 +110,74 @@ 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 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"); @@ -117,8 +194,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/apps/soundcraft-web/index.html b/apps/soundcraft-web/index.html index af72b21b..124e0b71 100644 --- a/apps/soundcraft-web/index.html +++ b/apps/soundcraft-web/index.html @@ -2,7 +2,7 @@ - + SoundCraft diff --git a/apps/soundcraft/src/main.rs b/apps/soundcraft/src/main.rs index 808cdef8..cd27ec44 100644 --- a/apps/soundcraft/src/main.rs +++ b/apps/soundcraft/src/main.rs @@ -182,7 +182,7 @@ fn services() -> Services { if let Some(dir) = path.parent().filter(|dir| dir.is_dir()) { d = d.set_directory(dir); } - if let Some(n) = path.file_name().and_then(|n| n.to_str()) { + if let Some(n) = path.file_name().and_then(std::ffi::OsStr::to_str) { d = d.set_file_name(n); } d.save_file().map(|p| p.to_string_lossy().into_owned()) 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) { diff --git a/crates/automation/tests/it/automation_roots.rs b/crates/automation/tests/it/automation_roots.rs index 58d4aad2..a2c41e1c 100644 --- a/crates/automation/tests/it/automation_roots.rs +++ b/crates/automation/tests/it/automation_roots.rs @@ -57,7 +57,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)) } @@ -186,7 +186,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/clap-host/src/plugin.rs b/crates/clap-host/src/plugin.rs index 2fbf2726..c6bcb9ed 100644 --- a/crates/clap-host/src/plugin.rs +++ b/crates/clap-host/src/plugin.rs @@ -340,7 +340,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 }; @@ -354,4 +353,8 @@ impl PluginEditor for ClapEditor { } out } + + fn state_changed(&mut self) -> bool { + self.link.host().dirty.swap(false, Ordering::Relaxed) + } } diff --git a/crates/clap-host/src/scan.rs b/crates/clap-host/src/scan.rs index 92a6f6d3..6bfbadd6 100644 --- a/crates/clap-host/src/scan.rs +++ b/crates/clap-host/src/scan.rs @@ -51,7 +51,7 @@ pub fn os_search_paths(home: Option, common_program_files: Option bool { - p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("clap")) + p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("clap")) } /// Recursively collects `.clap` files and bundles under `dirs` (missing directories are skipped). diff --git a/crates/dsp/src/lib.rs b/crates/dsp/src/lib.rs index afa17dad..5b17a590 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/autosave.rs b/crates/engine/src/autosave.rs index fd56478b..09a63f4f 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -181,11 +181,15 @@ impl Autosave { fn create(root: &Path, name: &str) -> Result { let stamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap_or_default().as_millis(); let base = format!("{} {stamp}-{}", sanitize_name(name), std::process::id()); - std::fs::create_dir_all(root).map_err(|err| io_error(root, err))?; + create_folder(root).map_err(|err| io_error(root, err))?; let mut dir = root.join(&base); let mut n = 1; loop { - match std::fs::create_dir(&dir) { + // Not recursive, so a folder that already exists is reported and the next name tried. + let mut builder = std::fs::DirBuilder::new(); + #[cfg(unix)] + std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700); + match builder.create(&dir) { Ok(()) => break, Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { n += 1; @@ -224,9 +228,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 } }; @@ -409,6 +411,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)?; @@ -534,6 +548,50 @@ 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. + 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(); + 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); + } + #[test] fn a_take_still_recording_at_a_crash_is_placed_on_recovery() { use crate::io::Take; diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 35bb2df3..9a44cca5 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -50,7 +50,7 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { }; let n = clips.len(); lanes.sort_by_key(|(i, _)| *i); - let stem = p.file_stem().and_then(|x| x.to_str()).unwrap_or("Clip Group").to_string(); + let stem = p.file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Clip Group").to_string(); let audio_dir_name = format!("{} Audio", crate::io::sanitize_name(&stem)); let dir = p.parent().map(Path::to_path_buf).unwrap_or_default(); let mut audio = BTreeMap::new(); @@ -80,26 +80,17 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { Ok(n) } -/// Import a clip group file at `at`, one lane per track starting at `first` (new audio tracks -/// are made when the session runs out). Returns the ids of the placed clips. -pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) -> Result> { - let id = "file.import_clip_groups"; - let text = crate::io::read_text(path)?; - let file: GroupFile = serde_json::from_str(&text).map_err(|err| EngineError::BadParams(id.into(), format!("not a clip group file: {err}")))?; - if file.format != FORMAT { - return Err(EngineError::BadParams(id.into(), "not a SoundCraft clip group file".into())); - } - if file.lanes.iter().map(|l| l.len().max(1)).sum::() > MAX_CLIPS || file.audio.len() > MAX_CLIPS { - return Err(EngineError::BadParams(id.into(), "clip group file has too many clips".into())); - } - let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); - // Decode audio first so a bad file changes nothing, and each file once however many ids name it. +fn decode_group_audio( + audio_entries: &BTreeMap, + dir: &Path, + rate: u32, + cmd_id: &str, +) -> Result<(Vec<(String, soundcraft_audio_io::AudioBuffer)>, BTreeMap)> { let (mut decoded, mut index, mut file_of) = (Vec::new(), BTreeMap::new(), BTreeMap::new()); - let rate = e.session().sample_rate.hz(); - for (old, rel) in &file.audio { + for (old, rel) in audio_entries { let full = dir.join(rel); - if !full.starts_with(&dir) || rel.contains("..") { - return Err(EngineError::BadParams(id.into(), format!("audio path escapes the group folder: {rel}"))); + if !full.starts_with(dir) || rel.contains("..") { + return Err(EngineError::BadParams(cmd_id.into(), format!("audio path escapes the group folder: {rel}"))); } let key = crate::io::file_key(&full); let n = match index.get(&key) { @@ -116,6 +107,25 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - }; file_of.insert(*old, n); } + Ok((decoded, file_of)) +} + +/// Import a clip group file at `at`, one lane per track starting at `first` (new audio tracks +/// are made when the session runs out). Returns the ids of the placed clips. +pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) -> Result> { + let id = "file.import_clip_groups"; + let text = crate::io::read_text(path)?; + let file: GroupFile = serde_json::from_str(&text).map_err(|err| EngineError::BadParams(id.into(), format!("not a clip group file: {err}")))?; + if file.format != FORMAT { + return Err(EngineError::BadParams(id.into(), "not a SoundCraft clip group file".into())); + } + if file.lanes.iter().map(|l| l.len().max(1)).sum::() > MAX_CLIPS || file.audio.len() > MAX_CLIPS { + return Err(EngineError::BadParams(id.into(), "clip group file has too many clips".into())); + } + let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); + // Decode audio first so a bad file changes nothing, and each file once however many ids name it. + let rate = e.session().sample_rate.hz(); + let (decoded, file_of) = decode_group_audio(&file.audio, &dir, rate, id)?; let src_rate = f64::from(file.sample_rate.max(1)); let s = e.session_mut(); let ratio = f64::from(s.sample_rate.hz()) / src_rate; @@ -123,7 +133,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - let sources: Vec = decoded .into_iter() .map(|(rel, buf)| { - let name = Path::new(&rel).file_name().and_then(|x| x.to_str()).unwrap_or("clip group").to_string(); + let name = Path::new(&rel).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("clip group").to_string(); crate::io::add_source(s, &name, buf, None, FileFormat::Wav) }) .collect(); 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/file.rs b/crates/engine/src/cmd/file.rs index 40e3d3db..d9ac3119 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -113,9 +113,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})) @@ -241,7 +242,7 @@ fn import_audio(e: &mut Engine, p: &Value) -> Result { let mut out = Vec::new(); for path in &paths { let bytes = crate::io::read_file(path)?; - let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or("Audio").to_string(); + let name = std::path::Path::new(path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("Audio").to_string(); let r = crate::io::import_audio_bytes(e, &name, &bytes, Some(path.as_str()), target, at)?; out.push(r); } @@ -302,7 +303,7 @@ pub fn bounce_job(e: &Engine, p: &Value) -> Result { let path = str_param(p, "path").ok_or_else(|| bad("file.bounce_mix", "`path` required"))?.to_string(); let r = range_param(e, "file.bounce_mix", p)?; let r = if r.is_empty() { soundcraft_time::Range::new(0, e.session().content_end().max(1)) } else { r }; - let ext = std::path::Path::new(&path).extension().and_then(|x| x.to_str()).unwrap_or("wav").to_ascii_lowercase(); + let ext = std::path::Path::new(&path).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("wav").to_ascii_lowercase(); let format_name = str_param(p, "format").unwrap_or(ext.as_str()); let format = soundcraft_audio_io::encode_format_for(format_name).ok_or_else(|| { bad("file.bounce_mix", format!("cannot write `{format_name}` files (supported: {})", soundcraft_audio_io::ENCODE_EXTENSIONS)) @@ -353,6 +354,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/cmd/health.rs b/crates/engine/src/cmd/health.rs index 260503ec..36819c99 100644 --- a/crates/engine/src/cmd/health.rs +++ b/crates/engine/src/cmd/health.rs @@ -12,6 +12,55 @@ pub fn specs() -> Vec { ] } +struct ClipContext<'a> { + track: &'a soundcraft_model::Track, + playlist: &'a soundcraft_model::Playlist, + active: bool, + session_sample_rate: u32, +} + +fn check_audio_clip( + ctx: &ClipContext<'_>, + clip: &soundcraft_model::Clip, + source: soundcraft_model::SourceId, + offset: i64, + metadata: Option<&soundcraft_model::Source>, + audio: Option<&soundcraft_model::SourceAudio>, + issues: &mut Vec, +) { + let mut add = |code: &str, message: &str| { + issues.push(json!({"code": code, "message": message, "track": ctx.track.name, "track_id": ctx.track.id.0, + "playlist": ctx.playlist.name, "active": ctx.active, "clip": clip.name, "clip_id": clip.id.0, + "source_id": source.0, "path": metadata.map(|m| m.path.as_str())})); + }; + if metadata.is_none() { + add("missing_source", "Source metadata is missing. Reimport the audio and replace this clip."); + } + let Some(audio) = audio else { + add("unavailable_audio", "Audio is not loaded; this clip will be silent. Restore the media and reopen the session, or reimport it."); + return; + }; + if audio.buffer.sample_rate != ctx.session_sample_rate { + add("sample_rate_mismatch", "Loaded audio differs from the session sample rate. Reimport it to convert to the session rate."); + } + if audio.buffer.channels.is_empty() || audio.frames() == 0 { + add("empty_audio", "Loaded audio has no playable frames. Reimport a valid audio file."); + return; + } + if audio.buffer.channels.iter().any(|channel| channel.len() != audio.frames()) { + add("uneven_channels", "Loaded audio channels have different lengths. Reimport a valid audio file; bounds below use the shortest channel."); + } + if !clip.stretch.is_finite() || clip.stretch <= 0.0 || clip.length <= 0 { + add("invalid_clip", "Clip length or stretch is invalid. Replace this clip."); + } + let stretch = if clip.stretch.is_finite() && clip.stretch > 0.0 { clip.stretch } else { 1.0 }; + // Match the mixer's last requested source sample, including elastic playback. + let last = offset as f64 + clip.length.saturating_sub(1).max(0) as f64 / stretch; + if offset < 0 || last >= audio.frames() as f64 { + add("source_bounds", "Clip extends outside the loaded audio; part will be silent. Trim the clip or replace its source."); + } +} + fn report(e: &mut Engine, _: &Value) -> Result { let s = e.session(); let sources: BTreeMap<_, _> = s.sources.iter().map(|source| (source.id, source)).collect(); @@ -24,51 +73,16 @@ fn report(e: &mut Engine, _: &Value) -> Result { for track in &s.tracks { for (index, playlist) in track.playlists.iter().enumerate() { let active = index == track.active_playlist; + let ctx = ClipContext { track, playlist, active, session_sample_rate: s.sample_rate.hz() }; for clip in &playlist.clips { let ClipContent::Audio { source, offset } = clip.content else { continue }; clips += 1; if active { active_end = active_end.max(clip.end()); } - let metadata = sources.get(&source); - let audio = s.pool.get(source); - let mut add = |code: &str, message: &str| { - issues.push(json!({"code": code, "message": message, "track": track.name, "track_id": track.id.0, - "playlist": playlist.name, "active": active, "clip": clip.name, "clip_id": clip.id.0, - "source_id": source.0, "path": metadata.map(|m| m.path.as_str())})); - }; - if metadata.is_none() { - add("missing_source", "Source metadata is missing. Reimport the audio and replace this clip."); - } - let Some(audio) = audio else { - add( - "unavailable_audio", - "Audio is not loaded; this clip will be silent. Restore the media and reopen the session, or reimport it.", - ); - continue; - }; - if audio.buffer.sample_rate != s.sample_rate.hz() { - add("sample_rate_mismatch", "Loaded audio differs from the session sample rate. Reimport it to convert to the session rate."); - } - if audio.buffer.channels.is_empty() || audio.frames() == 0 { - add("empty_audio", "Loaded audio has no playable frames. Reimport a valid audio file."); - continue; - } - if audio.buffer.channels.iter().any(|channel| channel.len() != audio.frames()) { - add( - "uneven_channels", - "Loaded audio channels have different lengths. Reimport a valid audio file; bounds below use the shortest channel.", - ); - } - if !clip.stretch.is_finite() || clip.stretch <= 0.0 || clip.length <= 0 { - add("invalid_clip", "Clip length or stretch is invalid. Replace this clip."); - } - let stretch = if clip.stretch.is_finite() && clip.stretch > 0.0 { clip.stretch } else { 1.0 }; - // Match the mixer's last requested source sample, including elastic playback. - let last = offset as f64 + clip.length.saturating_sub(1).max(0) as f64 / stretch; - if offset < 0 || last >= audio.frames() as f64 { - add("source_bounds", "Clip extends outside the loaded audio; part will be silent. Trim the clip or replace its source."); - } + let metadata = sources.get(&source).copied(); + let audio = s.pool.get(source).map(|a| a.as_ref()); + check_audio_clip(&ctx, clip, source, offset, metadata, audio, &mut issues); } } } diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index b6f379bd..41edf68c 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; @@ -81,7 +102,7 @@ impl CommandSpec { CommandInfo { id: self.id.into(), label: self.label.into(), - menu: self.menu.iter().map(|s| s.to_string()).collect(), + menu: self.menu.iter().map(ToString::to_string).collect(), shortcut: self.shortcut.map(str::to_string), params: self.params.into(), enabled: en.is_ok(), @@ -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 }) } @@ -224,13 +247,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); } diff --git a/crates/engine/src/cmd/plugin_state.rs b/crates/engine/src/cmd/plugin_state.rs index bc86ff17..db580091 100644 --- a/crates/engine/src/cmd/plugin_state.rs +++ b/crates/engine/src/cmd/plugin_state.rs @@ -48,6 +48,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 { @@ -55,7 +64,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, @@ -97,6 +105,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/cmd/sampler.rs b/crates/engine/src/cmd/sampler.rs index eb401bbd..f4634e3d 100644 --- a/crates/engine/src/cmd/sampler.rs +++ b/crates/engine/src/cmd/sampler.rs @@ -56,7 +56,7 @@ fn load(e: &mut Engine, p: &Value) -> Result { } } let bytes = crate::io::read_file(&path)?; - let name = Path::new(&path).file_name().and_then(|n| n.to_str()).unwrap_or("Sample").to_string(); + let name = Path::new(&path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("Sample").to_string(); attach(e, t, &name, &bytes, Some(&path), values) } @@ -69,7 +69,7 @@ pub(crate) fn attach(e: &mut Engine, t: TrackId, name: &str, bytes: &[u8], path: return Err(bad(id, "only instrument tracks have a sampler")); } let info = soundcraft_dsp::plugin_info(SAMPLER).ok_or_else(|| bad(id, "the sampler is not available"))?; - let ext = Path::new(name).extension().and_then(|x| x.to_str()).map(str::to_ascii_lowercase); + let ext = Path::new(name).extension().and_then(std::ffi::OsStr::to_str).map(str::to_ascii_lowercase); let (format, buf) = soundcraft_audio_io::decode(bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?; if buf.frames() == 0 || buf.num_channels() == 0 { return Err(EngineError::Io(format!("{name}: no audio"))); diff --git a/crates/engine/src/edit.rs b/crates/engine/src/edit.rs index e7a8e977..c32f8767 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); @@ -405,7 +407,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 +416,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; @@ -870,6 +876,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(); @@ -880,6 +900,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(); 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/crates/engine/src/io.rs b/crates/engine/src/io.rs index 4b6601b6..56d3d211 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -5,6 +5,7 @@ use serde_json::{Value, json}; use soundcraft_audio_io::{AudioBuffer, EncodeOptions, FileFormat}; use soundcraft_model::{BusId, ChannelFormat, Clip, ClipContent, Session, Source, SourceAudio, SourceId, TrackId, TrackKind}; use soundcraft_time::{Range, Samples}; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -74,7 +75,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()))) } @@ -114,7 +118,7 @@ pub fn add_source(s: &mut Session, name: &str, mut buf: AudioBuffer, path: Optio buf.sample_rate = sr; } let id = SourceId(s.alloc()); - let stem = Path::new(name).file_stem().and_then(|x| x.to_str()).unwrap_or(name).to_string(); + let stem = Path::new(name).file_stem().and_then(OsStr::to_str).unwrap_or(name).to_string(); let stored = match path { Some(p) => p.to_string(), None => unique_audio_rel(s, &stem), @@ -145,7 +149,7 @@ fn stamp_at_rate(stamp: u64, file_rate: u32, session_rate: u32) -> u64 { /// Decode and place an audio file. With no target track, a new track of matching width is made. pub fn import_audio_bytes(e: &mut Engine, name: &str, bytes: &[u8], path: Option<&str>, target: Option, at: Samples) -> Result { - let ext = Path::new(name).extension().and_then(|x| x.to_str()).map(str::to_ascii_lowercase); + let ext = Path::new(name).extension().and_then(OsStr::to_str).map(str::to_ascii_lowercase); let (info, buf) = soundcraft_audio_io::decode(bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?; let buf = at_rate(buf, e.session().sample_rate.hz(), MAX_IMPORT_SAMPLES) .ok_or_else(|| EngineError::Io(format!("{name}: file is too large to import")))?; @@ -164,7 +168,7 @@ pub fn import_audio_bytes(e: &mut Engine, name: &str, bytes: &[u8], path: Option let track = match target { Some(t) => t, None => { - let stem = Path::new(name).file_stem().and_then(|x| x.to_str()).unwrap_or("Audio").to_string(); + let stem = Path::new(name).file_stem().and_then(OsStr::to_str).unwrap_or("Audio").to_string(); s.add_track(TrackKind::Audio, ChannelFormat::for_channels(channels), Some(&stem)) } }; @@ -188,8 +192,8 @@ pub fn load_sampler_bytes(e: &mut Engine, track: TrackId, name: &str, bytes: &[u /// never copied) plus its audio on a new audio track when it has a usable audio stream. Fails /// only when the file has neither a readable picture nor audio. pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Samples) -> Result { - let stem = Path::new(path).file_stem().and_then(|n| n.to_str()).unwrap_or("Video").to_string(); - let ext = Path::new(path).extension().and_then(|x| x.to_str()).unwrap_or("mp4").to_ascii_lowercase(); + let stem = Path::new(path).file_stem().and_then(OsStr::to_str).unwrap_or("Video").to_string(); + let ext = Path::new(path).extension().and_then(OsStr::to_str).unwrap_or("mp4").to_ascii_lowercase(); let bytes: Arc<[u8]> = bytes.into(); let movie = soundcraft_video::Movie::open(Arc::clone(&bytes)).map(|m| m.info().clone()); let audio_name = format!("{stem} audio.{}", if ext == "mov" { "mp4" } else { ext.as_str() }); @@ -210,7 +214,7 @@ pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Sample let vid = SourceId(s.alloc()); s.videos.push(soundcraft_model::VideoSource { id: vid, - name: Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or("movie").to_string(), + name: Path::new(path).file_name().and_then(OsStr::to_str).unwrap_or("movie").to_string(), path: abs, width: info.width, height: info.height, @@ -992,7 +996,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess sync_dir(&audio_dir); } s.folder = Some(dir.clone()); - if !overwrite && let Some(stem) = p.file_stem().and_then(|x| x.to_str()) { + if !overwrite && let Some(stem) = p.file_stem().and_then(OsStr::to_str) { s.name = stem.to_string(); } let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; @@ -1052,7 +1056,7 @@ fn copy_new(from: &Path, to: &Path) -> std::io::Result<()> { /// Copy the session file about to be replaced to `Backups/ backup .`, keeping the /// newest ten. fn back_up(file: &Path) { - let (Some(dir), Some(stem), Some(ext)) = (file.parent(), file.file_stem().and_then(|x| x.to_str()), file.extension().and_then(|x| x.to_str())) + let (Some(dir), Some(stem), Some(ext)) = (file.parent(), file.file_stem().and_then(OsStr::to_str), file.extension().and_then(OsStr::to_str)) else { return; }; @@ -1103,7 +1107,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, } } @@ -1195,7 +1199,7 @@ pub(crate) fn decode_file(files: &mut DecodedFiles, p: &Path, rate: u32) -> std: .entry(file_key(p)) .or_insert_with(|| { let bytes = read_file(p).map_err(|_| p.display().to_string())?; - decode_source(&bytes, p.extension().and_then(|x| x.to_str()), rate).map_err(|err| format!("{}: {err}", p.display())) + decode_source(&bytes, p.extension().and_then(OsStr::to_str), rate).map_err(|err| format!("{}: {err}", p.display())) }) .clone() } @@ -1227,7 +1231,7 @@ pub fn set_deliver(h: DeliverHook) { pub fn write_file(path: &str, bytes: &[u8]) -> Result<()> { if cfg!(target_arch = "wasm32") { let deliver = DELIVER.get().ok_or_else(|| EngineError::Io("saving files is not available in this build".into()))?; - let name = Path::new(path).file_name().and_then(|n| n.to_str()).filter(|n| !n.is_empty()).unwrap_or("Untitled"); + let name = Path::new(path).file_name().and_then(OsStr::to_str).filter(|n| !n.is_empty()).unwrap_or("Untitled"); return deliver(name, bytes).map_err(EngineError::Io); } crate::file_access::check_write(path).map_err(EngineError::Io)?; @@ -1272,7 +1276,7 @@ pub fn session_bundle(session: &Session) -> Result<(Vec, Session)> { /// Save as a bundle named after `path`'s file name, delivered through the host ([`write_file`]). fn save_session_bundle(e: &mut Engine, path: &str) -> Result { - let stem = Path::new(path).file_stem().and_then(|x| x.to_str()).filter(|x| !x.is_empty()).map(str::to_string); + let stem = Path::new(path).file_stem().and_then(OsStr::to_str).filter(|x| !x.is_empty()).map(str::to_string); let mut s = e.session().clone(); if let Some(stem) = stem { s.name = stem; @@ -1311,7 +1315,7 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result match decode_source(&bytes, Path::new(&src.path).extension().and_then(|x| x.to_str()), s.sample_rate.hz()) { + Some(bytes) => match decode_source(&bytes, Path::new(&src.path).extension().and_then(OsStr::to_str), s.sample_rate.hz()) { Ok(audio) => Some(audio), Err(err) => { missing.push(format!("{}: {err}", src.path)); @@ -1330,7 +1334,7 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result Result<(String, std::collections::HashMap 0 { + buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, hz); + buf.sample_rate = hz; + } + let name = path.file_stem().and_then(OsStr::to_str).unwrap_or("Take"); + 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 { @@ -1469,6 +1478,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) { @@ -1582,6 +1601,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); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 03d173dd..05e31d66 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))); } }; @@ -394,6 +396,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; @@ -449,7 +456,7 @@ fn install_panic_hook() { } fn panic_message(p: &(dyn std::any::Any + Send)) -> String { - p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) + p.downcast_ref::<&str>().map(ToString::to_string).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) } #[cfg(test)] @@ -521,3 +528,26 @@ 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(); + // 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})); + 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); + } +} diff --git a/crates/midi/src/smf.rs b/crates/midi/src/smf.rs index 3b4508bc..693dc34d 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)]); + } } diff --git a/crates/mix/src/lib.rs b/crates/mix/src/lib.rs index f73f0a71..ba30c94c 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)); @@ -2305,6 +2330,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. diff --git a/crates/model/src/clip.rs b/crates/model/src/clip.rs index 850cf044..f43c10d8 100644 --- a/crates/model/src/clip.rs +++ b/crates/model/src/clip.rs @@ -244,13 +244,18 @@ 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. + // 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); } // 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 +272,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 +485,25 @@ 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); + 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); + 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()); + } + } } diff --git a/crates/model/src/mixer.rs b/crates/model/src/mixer.rs index bef493e8..7f0a24e9 100644 --- a/crates/model/src/mixer.rs +++ b/crates/model/src/mixer.rs @@ -196,7 +196,7 @@ impl ChannelFormat { pub fn channel_label(self, ch: usize) -> String { match self.speakers().get(ch) { Some(s) => s.label().to_string(), - None if self.is_ambisonic() => ["W", "Y", "Z", "X"].get(ch).map_or_else(|| format!("{}", ch + 1), |s| s.to_string()), + None if self.is_ambisonic() => ["W", "Y", "Z", "X"].get(ch).map_or_else(|| format!("{}", ch + 1), ToString::to_string), None => format!("{}", ch + 1), } } diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index b72818dc..cac69d99 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -288,7 +288,7 @@ impl Default for EditState { timeline_selection: Range::default(), flags: ["view.clip.name", "view.clip.gain_line", "waveform.peak", "view.clip.overlap_shadows", "view.marker.ruler_lines"] .iter() - .map(|s| s.to_string()) + .map(ToString::to_string) .collect(), values: std::collections::BTreeMap::new(), } diff --git a/crates/playback/src/lib.rs b/crates/playback/src/lib.rs index 41bb89ed..892c7749 100644 --- a/crates/playback/src/lib.rs +++ b/crates/playback/src/lib.rs @@ -362,7 +362,8 @@ 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); + // `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 }; if channels < nch { @@ -684,6 +685,17 @@ impl Player { self.side().capture_states() } + /// 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 /// the parameter edits made in open editors since the last call. pub fn idle(&self) -> Vec { @@ -812,6 +824,24 @@ 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 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)) + .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()?; @@ -848,7 +878,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); @@ -1223,6 +1253,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; @@ -1370,6 +1420,55 @@ 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_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); + 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 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_states_of(&[(a, 0), synth, (b, 0)]), want); + } + + #[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.changed_states().is_empty()); + } + #[test] fn side_creates_each_missing_instance_once() { use soundcraft_model::{ChannelFormat, Insert, TrackKind}; diff --git a/crates/time/src/format.rs b/crates/time/src/format.rs index 3c11c844..1244db86 100644 --- a/crates/time/src/format.rs +++ b/crates/time/src/format.rs @@ -124,7 +124,7 @@ pub fn parse_position( } TimeFormat::BarsBeats => { let parts: Vec = - t.split(['|', '.', ' ']).filter(|p| !p.is_empty()).map(|p| p.parse::()).collect::>().map_err(|_| err())?; + t.split(['|', '.', ' ']).filter(|p| !p.is_empty()).map(str::parse::).collect::>().map_err(|_| err())?; let bb = BarBeat { bar: *parts.first().ok_or_else(err)?, beat: parts.get(1).copied().unwrap_or(1).max(1), 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); 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/credits.rs b/crates/ui-egui/src/credits.rs index bd58c366..a53128cb 100644 --- a/crates/ui-egui/src/credits.rs +++ b/crates/ui-egui/src/credits.rs @@ -210,10 +210,7 @@ impl Default for View { } } -/// About ▸ Contributors: a name toggle, a sort, and the list as a grab bag or a table. -pub fn contributors_ui(ui: &mut egui::Ui) { - let id = egui::Id::new("credits_view"); - let mut v = ui.data_mut(|d| d.get_temp::(id)).unwrap_or_default(); +fn contributors_toolbar(ui: &mut egui::Ui, v: &mut View) { ui.horizontal_wrapped(|ui| { ui.label("Show"); for m in NameMode::ALL { @@ -242,6 +239,24 @@ pub fn contributors_ui(ui: &mut egui::Ui) { v.table = true; } }); +} + +fn contributors_grab_bag(ui: &mut egui::Ui, list: &[&Contributor], names: NameMode) { + ui.horizontal_wrapped(|ui| { + for (i, c) in list.iter().enumerate() { + if i > 0 { + ui.label(RichText::new("·").weak()); + } + ui.hyperlink_to(c.name(names), format!("https://github.com/{}", c.login)).on_hover_text(c.summary()); + } + }); +} + +/// About ▸ Contributors: a name toggle, a sort, and the list as a grab bag or a table. +pub fn contributors_ui(ui: &mut egui::Ui) { + let id = egui::Id::new("credits_view"); + let mut v = ui.data_mut(|d| d.get_temp::(id)).unwrap_or_default(); + contributors_toolbar(ui, &mut v); let list = sorted(CONTRIBUTORS, v.names, v.key, v.ascending); ui.label(RichText::new(format!("{} contributors · {} commits", list.len(), group(TOTAL_COMMITS))).small().weak()); ui.separator(); @@ -251,14 +266,7 @@ pub fn contributors_ui(ui: &mut egui::Ui) { } else if v.table { table(ui, &list, &mut v); } else { - ui.horizontal_wrapped(|ui| { - for (i, c) in list.iter().enumerate() { - if i > 0 { - ui.label(RichText::new("·").weak()); - } - ui.hyperlink_to(c.name(v.names), format!("https://github.com/{}", c.login)).on_hover_text(c.summary()); - } - }); + contributors_grab_bag(ui, &list, v.names); } }); ui.data_mut(|d| d.insert_temp(id, v)); diff --git a/crates/ui-egui/src/edit_window.rs b/crates/ui-egui/src/edit_window.rs index 7e24a4c7..dbd74137 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; @@ -239,6 +294,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 +347,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) } @@ -373,9 +450,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 +916,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 +964,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() @@ -1456,6 +1537,10 @@ 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() { + let _ = app.run("view.scroll_into_view", json!({"track": id.0})); + } ui.separator(); ui.menu_button("Track Height", |ui| { for h in TrackHeight::ALL { @@ -1511,9 +1596,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 +1671,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 +1694,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 +1865,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())); @@ -2471,13 +2572,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 } @@ -2776,11 +2882,76 @@ 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::*; 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. @@ -3444,6 +3615,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/lib.rs b/crates/ui-egui/src/lib.rs index 3fdd9892..6a8d1e8f 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; @@ -295,6 +296,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)>, @@ -322,6 +327,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, @@ -335,6 +342,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 @@ -381,6 +411,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(), @@ -399,6 +431,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, @@ -600,6 +633,13 @@ impl SoundApp { if plan.is_empty() { return Err("no audio track is record-enabled".into()); } + // 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() { + 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() .map(|(track, inputs, path)| soundcraft_engine::io::Take { track: *track, path: path.clone(), channels: inputs.len() }) @@ -612,10 +652,21 @@ 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}"); } } @@ -643,15 +694,36 @@ 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. The state is read once per gesture (see + /// `editor_states_due`). fn apply_editor_edits(&mut self) { - let Some(p) = &self.player else { return }; - 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 { 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); } + 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). @@ -659,7 +731,7 @@ impl SoundApp { 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()); @@ -722,7 +794,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(); @@ -776,7 +848,7 @@ impl SoundApp { for d in std::fs::read_dir(dir).ok()?.flatten() { for f in std::fs::read_dir(d.path()).into_iter().flatten().flatten() { let p = f.path(); - if p.extension().and_then(|x| x.to_str()) == Some("scraft") + if p.extension().and_then(std::ffi::OsStr::to_str) == Some("scraft") && let Ok(m) = f.metadata().and_then(|m| m.modified()) && best.as_ref().is_none_or(|(t, _)| m > *t) { @@ -1330,6 +1402,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() { @@ -1417,6 +1490,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()); @@ -1526,6 +1634,126 @@ 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])); + } + + /// 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(); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 99e4b503..c38ef84d 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), @@ -86,7 +87,7 @@ pub fn ui_aliases() -> Vec<(&'static str, &'static str)> { /// Mix-view menu paths, allocated once. Rebuilding them on every menu draw leaked a copy per frame. fn mix_view_aliases() -> &'static [(&'static str, &'static str)] { - static ALIASES: std::sync::OnceLock> = std::sync::OnceLock::new(); + static ALIASES: std::sync::OnceLock> = std::sync::OnceLock::new(); ALIASES.get_or_init(|| { [ "Inserts A-E", @@ -633,7 +634,7 @@ pub fn run_ui_command(app: &mut SoundApp, id: &str, p: &Value) -> Option { app.ui.mix_views = - ["inserts_ae", "inserts_fj", "sends_ae", "sends_fj", "io", "comments", "color"].iter().map(|s| s.to_string()).collect() + ["inserts_ae", "inserts_fj", "sends_ae", "sends_fj", "io", "comments", "color"].iter().map(ToString::to_string).collect() } "minimal" => app.ui.mix_views.clear(), "" => return Some(Err("`section` required".into())), @@ -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/src/mix_window.rs b/crates/ui-egui/src/mix_window.rs index 4ee3fb83..1540bcfb 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,6 +160,12 @@ 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 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; + 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/src/panels.rs b/crates/ui-egui/src/panels.rs index a208511f..5522e4cf 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() { @@ -371,6 +414,29 @@ fn big_counter(app: &mut SoundApp, ctx: &egui::Context) { app.ui.show_big_counter = open; } +fn memory_location_row(app: &mut SoundApp, ui: &mut Ui, n: u32, name: &str, loc: &str, len: &str, kind: &str) { + if ui.button(n.to_string()).on_hover_text("Recall").clicked() { + let _ = app.run("markers.recall", json!({"number": n})); + } + let key = egui::Id::new(("mem_name", n)); + let mut edit: String = ui.ctx().memory(|m| m.data.get_temp(key)).unwrap_or_else(|| name.to_string()); + let r = ui.add(egui::TextEdit::singleline(&mut edit).desired_width(140.0)); + if r.changed() { + ui.ctx().memory_mut(|m| m.data.insert_temp(key, edit.clone())); + } + if r.lost_focus() && edit != name { + let _ = app.run("markers.edit", json!({"number": n, "rename": edit})); + ui.ctx().memory_mut(|m| m.data.remove::(key)); + } + ui.label(egui::RichText::new(loc).font(mono(11.0))); + ui.label(egui::RichText::new(len).font(mono(11.0))); + ui.label(kind); + if ui.small_button("✕").on_hover_text("Delete").clicked() { + let _ = app.run("markers.delete", json!({"number": n})); + } + ui.end_row(); +} + pub(crate) fn memory_locations(app: &mut SoundApp, ui: &mut Ui) { ui.horizontal(|ui| { if ui.button("+ Marker").clicked() { @@ -402,27 +468,8 @@ pub(crate) fn memory_locations(app: &mut SoundApp, ui: &mut Ui) { ui.label(egui::RichText::new(h).strong()); } ui.end_row(); - for (n, name, loc, len, kind) in rows { - if ui.button(n.to_string()).on_hover_text("Recall").clicked() { - let _ = app.run("markers.recall", json!({"number": n})); - } - let key = egui::Id::new(("mem_name", n)); - let mut edit: String = ui.ctx().memory(|m| m.data.get_temp(key)).unwrap_or_else(|| name.clone()); - let r = ui.add(egui::TextEdit::singleline(&mut edit).desired_width(140.0)); - if r.changed() { - ui.ctx().memory_mut(|m| m.data.insert_temp(key, edit.clone())); - } - if r.lost_focus() && edit != name { - let _ = app.run("markers.edit", json!({"number": n, "rename": edit})); - ui.ctx().memory_mut(|m| m.data.remove::(key)); - } - ui.label(egui::RichText::new(loc).font(mono(11.0))); - ui.label(egui::RichText::new(len).font(mono(11.0))); - ui.label(kind); - if ui.small_button("✕").on_hover_text("Delete").clicked() { - let _ = app.run("markers.delete", json!({"number": n})); - } - ui.end_row(); + for (n, name, loc, len, kind) in &rows { + memory_location_row(app, ui, *n, name, loc, len, kind); } }); }); @@ -669,7 +716,7 @@ fn preset_menu( return; }; let mut names: Vec = std::fs::read_dir(&dir) - .map(|rd| rd.flatten().filter_map(|e| e.path().file_stem().and_then(|x| x.to_str()).map(str::to_string)).collect()) + .map(|rd| rd.flatten().filter_map(|e| e.path().file_stem().and_then(std::ffi::OsStr::to_str).map(str::to_string)).collect()) .unwrap_or_default(); names.sort(); for n in &names { @@ -708,3 +755,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()); + } +} diff --git a/crates/ui-egui/src/recent.rs b/crates/ui-egui/src/recent.rs index 12be3980..597cc99c 100644 --- a/crates/ui-egui/src/recent.rs +++ b/crates/ui-egui/src/recent.rs @@ -13,6 +13,7 @@ use crate::SoundApp; use serde_json::json; +use std::ffi::OsStr; use std::path::Path; /// How many sessions the menu keeps. @@ -45,8 +46,8 @@ pub fn note(list: &mut Vec, path: &str) -> bool { /// "name — folder" for a session path. fn label(path: &str) -> String { let p = Path::new(path); - let name = p.file_stem().and_then(|n| n.to_str()).unwrap_or(path); - match p.parent().and_then(|d| d.file_name()).and_then(|n| n.to_str()) { + let name = p.file_stem().and_then(OsStr::to_str).unwrap_or(path); + match p.parent().and_then(Path::file_name).and_then(OsStr::to_str) { Some(folder) => format!("{name} — {folder}"), None => name.to_string(), } diff --git a/crates/ui-egui/src/sampler_window.rs b/crates/ui-egui/src/sampler_window.rs index b259343d..45f9697b 100644 --- a/crates/ui-egui/src/sampler_window.rs +++ b/crates/ui-egui/src/sampler_window.rs @@ -8,6 +8,7 @@ use crate::theme::{Tokens, mono, regular}; use egui::{Align2, Color32, CursorIcon, Rect, Sense, Stroke, StrokeKind, Ui, pos2, vec2}; use serde_json::json; use soundcraft_model::{Insert, TrackId}; +use std::ffi::OsStr; use std::path::Path; /// The built-in Sampler's plugin id. @@ -21,7 +22,7 @@ const MARKERS: [(&str, bool); 3] = [("start", false), ("loop_start", true), ("lo /// Whether a file name looks like audio the sampler can load. pub fn is_audio_name(name: &str) -> bool { - Path::new(name).extension().and_then(|x| x.to_str()).is_some_and(|x| AUDIO_EXTS.contains(&x.to_ascii_lowercase().as_str())) + Path::new(name).extension().and_then(OsStr::to_str).is_some_and(|x| AUDIO_EXTS.contains(&x.to_ascii_lowercase().as_str())) } /// Sampler parameters that are MIDI notes. diff --git a/crates/ui-egui/src/spot.rs b/crates/ui-egui/src/spot.rs index c9b4102b..e92798fe 100644 --- a/crates/ui-egui/src/spot.rs +++ b/crates/ui-egui/src/spot.rs @@ -113,9 +113,44 @@ fn recorded_at(s: &Session, time_reference: u64) -> Samples { i64::try_from(time_reference).unwrap_or(0).saturating_sub(s.timecode_start) } +fn spot_field_row(ui: &mut Ui, s: &Session, d: &mut Spot, i: usize, label: &str) { + let show = |at: Samples, f: TimeFormat| s.format_position(at, f); + ui.label(label); + if d.editable(i) { + let id = Id::new(("spot_field", i)); + if !ui.memory(|m| m.has_focus(id)) { + d.text[i] = show(d.value(i), d.format); + } + let r = ui.add(TextEdit::singleline(&mut d.text[i]).id(id).font(mono(13.0)).desired_width(150.0)); + if r.changed() + && let Ok(v) = s.parse_position(&d.text[i], d.format) + && (-MAX_POSITION..=MAX_POSITION).contains(&v) + { + d.set(i, v); + } + if std::mem::take(&mut d.fresh) { + r.request_focus(); + select_all(ui, id, &d.text[i]); + } + } else { + ui.label(RichText::new(show(d.value(i), d.format)).font(mono(13.0))); + } + ui.end_row(); +} + +fn original_time_stamp_row(ui: &mut Ui, s: &Session, d: &mut Spot, at: Samples) { + ui.label("Original Time Stamp"); + ui.horizontal(|ui| { + ui.label(RichText::new(s.format_position(at, TimeFormat::Timecode)).font(mono(13.0))); + if ui.button("Use").on_hover_text("Spot to where it was recorded").clicked() { + d.start = at; + } + }); + ui.end_row(); +} + /// The dialog's fields; the caller adds OK and Cancel. pub fn fields(ui: &mut Ui, s: &Session, d: &mut Spot) { - let show = |at: Samples, f: TimeFormat| s.format_position(at, f); egui::Grid::new("spot").num_columns(2).spacing(vec2(10.0, 6.0)).show(ui, |ui| { ui.label("Time Scale"); egui::ComboBox::from_id_salt("spot_scale").selected_text(d.format.label()).show_ui(ui, |ui| { @@ -128,40 +163,13 @@ pub fn fields(ui: &mut Ui, s: &Session, d: &mut Spot) { if i == 1 && matches!(d.target, Target::Trim { .. }) { continue; } - ui.label(label); - if d.editable(i) { - let id = Id::new(("spot_field", i)); - if !ui.memory(|m| m.has_focus(id)) { - d.text[i] = show(d.value(i), d.format); - } - let r = ui.add(TextEdit::singleline(&mut d.text[i]).id(id).font(mono(13.0)).desired_width(150.0)); - if r.changed() - && let Ok(v) = s.parse_position(&d.text[i], d.format) - && (-MAX_POSITION..=MAX_POSITION).contains(&v) - { - d.set(i, v); - } - if std::mem::take(&mut d.fresh) { - r.request_focus(); - select_all(ui, id, &d.text[i]); - } - } else { - ui.label(RichText::new(show(d.value(i), d.format)).font(mono(13.0))); - } - ui.end_row(); + spot_field_row(ui, s, d, i, label); } ui.label("Duration"); ui.label(RichText::new(format_length(d.start, d.len, d.format, s.sample_rate, &s.tempo, s.frame_rate)).font(mono(13.0))); ui.end_row(); if let Some(at) = d.stamp { - ui.label("Original Time Stamp"); - ui.horizontal(|ui| { - ui.label(RichText::new(show(at, TimeFormat::Timecode)).font(mono(13.0))); - if ui.button("Use").on_hover_text("Spot to where it was recorded").clicked() { - d.start = at; - } - }); - ui.end_row(); + original_time_stamp_row(ui, s, d, at); } }); } 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/crates/ui-egui/src/toolbar.rs b/crates/ui-egui/src/toolbar.rs index 41d83ac4..c3169790 100644 --- a/crates/ui-egui/src/toolbar.rs +++ b/crates/ui-egui/src/toolbar.rs @@ -43,10 +43,40 @@ pub fn show(app: &mut SoundApp, ui: &mut Ui) { }); } +struct ModeCellState { + cur: EditMode, + snap: bool, + locked: bool, +} + +fn edit_mode_tooltip(m: EditMode, locked: bool, cmd: &str, fkey: &str) -> String { + match m { + EditMode::Shuffle if locked => format!("Shuffle is locked ({cmd}-click to unlock)"), + EditMode::Shuffle => format!("Shuffle mode ({fkey}); {cmd}-click locks it out"), + EditMode::Grid => format!("Grid mode ({fkey}; again for Relative); Shift-click: Snap to Grid in the other modes"), + _ => format!("{} mode ({fkey}); Shift-click adds Snap to Grid", if m == EditMode::Slip { "Slip" } else { "Spot" }), + } +} + +fn draw_mode_cell(p: &egui::Painter, cell: Rect, resp: &egui::Response, t: &Tokens, m: EditMode, state: &ModeCellState, label: &str) { + let on = state.cur == m || (m == EditMode::Grid && state.cur == EditMode::GridRelative); + key(p, cell, resp, if on { t.mode_on } else { t.mode_off }); + let fg = if on { t.mode_on_text } else { t.mode_off_text }; + if m == EditMode::Grid && state.snap { + p.rect_stroke(cell, 1.0, Stroke::new(1.5, t.mode_on), StrokeKind::Inside); + } + if m == EditMode::Shuffle && state.locked { + icons::draw(p, Rect::from_center_size(cell.center(), vec2(15.0, 15.0)), "lock", fg); + } else { + let txt = if m == EditMode::Grid && state.cur == EditMode::GridRelative { "REL GRID" } else { label }; + p.text(cell.center(), Align2::CENTER_CENTER, txt, bold(10.5), fg); + } +} + fn edit_modes(app: &mut SoundApp, ui: &mut Ui) { let t = Tokens::current(); let ed = &app.engine.session().edit; - let (cur, snap, locked) = (ed.edit_mode, ed.flag(SNAP_TO_GRID), ed.flag(SHUFFLE_LOCK)); + let state = ModeCellState { cur: ed.edit_mode, snap: ed.flag(SNAP_TO_GRID), locked: ed.flag(SHUFFLE_LOCK) }; let cmd = crate::shortcuts::shortcut_label("Cmd", ui.ctx().os() == egui::os::OperatingSystem::Mac); let modes = [ (EditMode::Shuffle, 0, 0, "SHUFFLE", "F1"), @@ -63,28 +93,13 @@ fn edit_modes(app: &mut SoundApp, ui: &mut Ui) { for (m, cx, cy, label, fkey) in modes { let cell = Rect::from_min_size(pos2(inner.min.x + cw * cx as f32, inner.min.y + ch * cy as f32), vec2(cw, ch)).shrink(1.0); let resp = ui.interact(cell, ui.id().with(("mode", label)), Sense::click()); - let on = cur == m || (m == EditMode::Grid && cur == EditMode::GridRelative); - key(ui.painter(), cell, &resp, if on { t.mode_on } else { t.mode_off }); - let fg = if on { t.mode_on_text } else { t.mode_off_text }; - if m == EditMode::Grid && snap { - ui.painter().rect_stroke(cell, 1.0, Stroke::new(1.5, t.mode_on), StrokeKind::Inside); - } - if m == EditMode::Shuffle && locked { - icons::draw(ui.painter(), Rect::from_center_size(cell.center(), vec2(15.0, 15.0)), "lock", fg); - } else { - let txt = if m == EditMode::Grid && cur == EditMode::GridRelative { "REL GRID" } else { label }; - ui.painter().text(cell.center(), Align2::CENTER_CENTER, txt, bold(10.5), fg); - } + draw_mode_cell(ui.painter(), cell, &resp, &t, m, &state, label); + let on = state.cur == m || (m == EditMode::Grid && state.cur == EditMode::GridRelative); resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::RadioButton, true, on, label)); if resp.clicked() { mode_click(app, m, ui.input(|i| i.modifiers)); } - resp.on_hover_text(match m { - EditMode::Shuffle if locked => format!("Shuffle is locked ({cmd}-click to unlock)"), - EditMode::Shuffle => format!("Shuffle mode ({fkey}); {cmd}-click locks it out"), - EditMode::Grid => format!("Grid mode ({fkey}; again for Relative); Shift-click: Snap to Grid in the other modes"), - _ => format!("{} mode ({fkey}); Shift-click adds Snap to Grid", if m == EditMode::Slip { "Slip" } else { "Spot" }), - }); + resp.on_hover_text(edit_mode_tooltip(m, state.locked, &cmd, fkey)); } }); } diff --git a/crates/ui-egui/src/video_window.rs b/crates/ui-egui/src/video_window.rs index 61809e78..3e66e165 100644 --- a/crates/ui-egui/src/video_window.rs +++ b/crates/ui-egui/src/video_window.rs @@ -18,6 +18,43 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { } } +fn video_status_text(app: &SoundApp, pic: Option<&(MovieKey, Result, f64, f64)>) -> String { + match pic { + Some((_, Err(why), ..)) => why.clone(), + Some((k, ..)) => match app.video.info(*k) { + Some(Ok(i)) => format!("{}x{} {:.3} fps {}", i.width, i.height, i.frame_rate, i.codec_detail), + Some(Err(e)) => e.clone(), + None => "opening…".into(), + }, + None => String::new(), + } +} + +fn draw_video_surface(app: &SoundApp, painter: &egui::Painter, img: Rect, pic: Option<&(MovieKey, Result, f64, f64)>, on: bool) { + let msg = if !on { + Some("Video Track Offline".to_string()) + } else { + match pic { + None => None, + Some((_, Err(why), ..)) => Some(why.clone()), + Some((k, ..)) => match (app.video.current(*k), app.video.current_error(*k)) { + (Some(tex), _) => { + painter.image(tex.id(), img, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); + None + } + (None, Some(e)) => Some(e.to_string()), + (None, None) => match app.video.info(*k) { + Some(Err(e)) => Some(e.clone()), + _ => None, + }, + }, + } + }; + if let Some(m) = msg { + painter.text(img.center(), Align2::CENTER_CENTER, m, regular(12.0), Color32::from_gray(170)); + } +} + fn video(app: &mut SoundApp, ctx: &egui::Context) { app.video.poll(ctx); let pos = app.position(); @@ -38,15 +75,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { egui::Window::new("Video").open(&mut open).default_size(vec2(480.0, 480.0 / aspect + 28.0)).resizable(true).show(ctx, |ui| { ui.horizontal(|ui| { crate::widgets::checkbox(ui, &mut burn, "Timecode burn-in"); - let text = match &pic { - Some((_, Err(why), ..)) => why.clone(), - Some((k, ..)) => match app.video.info(*k) { - Some(Ok(i)) => format!("{}x{} {:.3} fps {}", i.width, i.height, i.frame_rate, i.codec_detail), - Some(Err(e)) => e.clone(), - None => "opening…".into(), - }, - None => String::new(), - }; + let text = video_status_text(app, pic.as_ref()); ui.label(egui::RichText::new(text).font(regular(10.5)).color(Color32::from_gray(150))); }); let avail = ui.available_rect_before_wrap(); @@ -60,28 +89,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { w = h * aspect; } let img = Rect::from_center_size(r.center(), vec2(w, h)); - let msg = if !on { - Some("Video Track Offline".to_string()) - } else { - match &pic { - None => None, - Some((_, Err(why), ..)) => Some(why.clone()), - Some((k, ..)) => match (app.video.current(*k), app.video.current_error(*k)) { - (Some(tex), _) => { - painter.image(tex.id(), img, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); - None - } - (None, Some(e)) => Some(e.to_string()), - (None, None) => match app.video.info(*k) { - Some(Err(e)) => Some(e.clone()), - _ => None, - }, - }, - } - }; - if let Some(m) = msg { - painter.text(img.center(), Align2::CENTER_CENTER, m, regular(12.0), Color32::from_gray(170)); - } + draw_video_surface(app, &painter, img, pic.as_ref(), on); if burn { let tc = s.format_position(pos, TimeFormat::Timecode); let size = (h / 12.0).clamp(10.0, 36.0); diff --git a/crates/ui-egui/src/windows.rs b/crates/ui-egui/src/windows.rs index 78a91081..62bb3ae2 100644 --- a/crates/ui-egui/src/windows.rs +++ b/crates/ui-egui/src/windows.rs @@ -341,7 +341,7 @@ fn workspace(app: &mut SoundApp, ctx: &egui::Context) { return None; } let is_dir = e.file_type().is_ok_and(|t| t.is_dir()); - let ext = std::path::Path::new(&name).extension().and_then(|x| x.to_str()).unwrap_or("").to_ascii_lowercase(); + let ext = std::path::Path::new(&name).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_ascii_lowercase(); (is_dir || ["wav", "aif", "aiff", "flac", "mp3", "ogg", "m4a", "m4b", "caf", "mid", "midi", "scraft"].contains(&ext.as_str())) .then_some((name, is_dir)) }) diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index d9eb9fe6..eb09a58b 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -38,10 +38,12 @@ 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; mod side_lists_access; +mod toolbar_scroll; mod toolbar_tempo; mod track_height; mod track_list; diff --git a/crates/ui-egui/tests/it/scroll_into_view.rs b/crates/ui-egui/tests/it/scroll_into_view.rs new file mode 100644 index 00000000..2a817357 --- /dev/null +++ b/crates/ui-egui/tests/it/scroll_into_view.rs @@ -0,0 +1,168 @@ +//! 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); +} + +#[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()); +} + +#[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}"); +} diff --git a/crates/ui-egui/tests/it/toolbar_scroll.rs b/crates/ui-egui/tests/it/toolbar_scroll.rs new file mode 100644 index 00000000..4a6b7c52 --- /dev/null +++ b/crates/ui-egui/tests/it/toolbar_scroll.rs @@ -0,0 +1,52 @@ +//! 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; + +/// 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" => Some((t.galley.rect.translate(t.pos.to_vec2()), c.clip_rect)), + _ => 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(&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). + 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(&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:?}" + ); +} diff --git a/crates/video/src/h264/cabac_mb.rs b/crates/video/src/h264/cabac_mb.rs index 76c9c9a5..3379e193 100644 --- a/crates/video/src/h264/cabac_mb.rs +++ b/crates/video/src/h264/cabac_mb.rs @@ -240,11 +240,11 @@ impl SliceDecoder<'_> { if c.decode_decision(base + inc) == 0 { return 0; } - let mut prefix = 1; - while prefix < 9 && c.decode_decision(base + (prefix + 2).min(6)) == 1 { + let mut prefix: i32 = 1; + while prefix < 9 && c.decode_decision(base + (prefix + 2).min(6) as usize) == 1 { prefix += 1; } - let mut val = prefix as i32; + let mut val = prefix; if prefix >= 9 { // EG3 suffix let mut k = 3; diff --git a/crates/video/src/h264/params.rs b/crates/video/src/h264/params.rs index 99232ac7..0970a106 100644 --- a/crates/video/src/h264/params.rs +++ b/crates/video/src/h264/params.rs @@ -575,7 +575,7 @@ impl Pps { ensure!(sps_id < 32, "seq_parameter_set_id out of range"); let sps = spss .get(sps_id as usize) - .and_then(|s| s.as_ref()) + .and_then(Option::as_ref) .ok_or_else(|| crate::h264::Error::MissingParameterSet(format!("SPS {sps_id} for PPS {id}")))?; let entropy_coding_mode = r.read_flag()?; let bottom_field_pic_order_in_frame_present = r.read_flag()?; diff --git a/crates/video/src/h264/slicedec.rs b/crates/video/src/h264/slicedec.rs index a118b964..7e5529dd 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::{BLK4_XY, QPC_TABLE, RASTER_TO_BLK4, ZIGZAG4, ZIGZAG8}; 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 { @@ -1451,7 +1455,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()?; @@ -1509,4 +1513,12 @@ mod tests { assert_eq!(m0, [40, -4]); assert_eq!(m1, [0, 0]); } + + #[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)); + } } diff --git a/crates/video/src/prores/decode.rs b/crates/video/src/prores/decode.rs index fcb8b07f..01b1900f 100644 --- a/crates/video/src/prores/decode.rs +++ b/crates/video/src/prores/decode.rs @@ -200,7 +200,7 @@ pub fn decode_frame_into(data: &[u8], opts: &DecodeOptions, out: &mut Frame) -> let mut ac = alpha.as_mut().map(|a| a.chunks_mut(band_h * width)); while let (Some(y), Some(cb), Some(cr)) = (yc.next(), cbc.next(), crc.next()) { let rows = y.len() / width; - let a = ac.as_mut().and_then(|it| it.next()); + let a = ac.as_mut().and_then(Iterator::next); bands.push(Band { y, cb, cr, a, rows }); } } diff --git a/crates/vst3-host/src/scan.rs b/crates/vst3-host/src/scan.rs index f6a2b211..e9ea2962 100644 --- a/crates/vst3-host/src/scan.rs +++ b/crates/vst3-host/src/scan.rs @@ -79,7 +79,7 @@ pub fn os_search_paths(os: Os, home: Option, common_program_files: Opti /// True when the path names a VST3 plugin (`*.vst3`, case-insensitive). pub fn is_vst3_path(p: &Path) -> bool { - p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("vst3")) + p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("vst3")) } /// Recursively collects `.vst3` bundles and files under `dirs` (missing directories are skipped). @@ -181,7 +181,7 @@ pub fn bundle_binary_for(os: Os, path: &Path) -> Option { .flatten() .map(|e| e.path()) .filter(|p| p.is_file()) - .filter(|p| ext.is_none_or(|e| p.extension().and_then(|x| x.to_str()).is_some_and(|x| x.eq_ignore_ascii_case(e)))) + .filter(|p| ext.is_none_or(|e| p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|x| x.eq_ignore_ascii_case(e)))) .take(64) .collect(); files.sort(); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9efe3789..72904533 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -83,9 +83,15 @@ 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. + +`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 diff --git a/docs/mcp.md b/docs/mcp.md index 2a08e879..93496258 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 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 | diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 61679c8e..9ec31a55 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -29,11 +29,28 @@ 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 ;; +release_arch() { + local arch="$1" + case "$arch" in + amd64 | x86_64) echo x86_64 ;; + arm64 | aarch64) echo aarch64 ;; + *) echo "unsupported architecture $arch" >&2; return 2 ;; + esac + return +} +# 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. +case "${CARGO_BUILD_TARGET:-}" in + "") ARCH="$(release_arch "$(uname -m)")" ;; + 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. +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" @@ -41,7 +58,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 +81,19 @@ 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 + # 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*" executable, "*"$want"*FreeBSD* | ELF*"$want"*FreeBSD*interpreter*) ;; + *) 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" ls -lh "$DIST" diff --git a/packaging/linux/package.sh b/packaging/linux/package.sh index 72d95f8b..e23fed92 100755 --- a/packaging/linux/package.sh +++ b/packaging/linux/package.sh @@ -131,6 +131,10 @@ if has appimage; then cp "$ROOT/assets/app-icon/hicolor/256x256/apps/$APP_ID.png" "$APPDIR/$APP_ID.png" ln -s "$APP_ID.png" "$APPDIR/.DirIcon" + verify_sha256() { + local sha="$1" file="$2" + printf '%s %s\n' "$sha" "$file" | sha256sum -c - + } TOOL="${APPIMAGETOOL:-$(command -v appimagetool || true)}" if [[ -z "$TOOL" ]]; then if [[ -z "$APPIMAGETOOL_SHA256" ]]; then @@ -142,21 +146,21 @@ if has appimage; then download="$TOOL.download" curl --proto '=https' -fsSL -o "$download" \ "https://github.com/AppImage/appimagetool/releases/download/$APPIMAGETOOL_VERSION/appimagetool-$ARCH.AppImage" - printf '%s %s\n' "$APPIMAGETOOL_SHA256" "$download" | sha256sum -c - + verify_sha256 "$APPIMAGETOOL_SHA256" "$download" chmod 755 "$download" mv "$download" "$TOOL" fi - printf '%s %s\n' "$APPIMAGETOOL_SHA256" "$TOOL" | sha256sum -c - + verify_sha256 "$APPIMAGETOOL_SHA256" "$TOOL" fi RUNTIME="$CARGO_TARGET_DIR/appimage-runtime-$APPIMAGE_RUNTIME_VERSION-$ARCH" if [[ ! -f "$RUNTIME" ]]; then download="$RUNTIME.download" curl --proto '=https' -fsSL -o "$download" \ "https://github.com/AppImage/type2-runtime/releases/download/$APPIMAGE_RUNTIME_VERSION/runtime-$ARCH" - printf '%s %s\n' "$APPIMAGE_RUNTIME_SHA256" "$download" | sha256sum -c - + verify_sha256 "$APPIMAGE_RUNTIME_SHA256" "$download" mv "$download" "$RUNTIME" fi - printf '%s %s\n' "$APPIMAGE_RUNTIME_SHA256" "$RUNTIME" | sha256sum -c - + verify_sha256 "$APPIMAGE_RUNTIME_SHA256" "$RUNTIME" # Absolute, because appimagetool runs in $DIST below (CARGO_TARGET_DIR or APPIMAGETOOL may be # relative). OUT="$(cd "$DIST" && pwd)/$BASENAME.AppImage" diff --git a/xtask/src/main.rs b/xtask/src/main.rs index c2da6561..79b67325 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -97,7 +97,7 @@ fn assets() -> Result<(), String> { let attribution = std::fs::read_to_string(root().join("ATTRIBUTION.md")).map_err(|e| format!("ATTRIBUTION.md: {e}"))?; let mut missing = Vec::new(); for f in files.lines() { - let ext = Path::new(f).extension().and_then(|x| x.to_str()).unwrap_or("").to_ascii_lowercase(); + let ext = Path::new(f).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_ascii_lowercase(); if !ASSET_EXT.contains(&ext.as_str()) { continue; }