diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 29521ad73..8ce4b85aa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,7 +26,7 @@ that variable set. ```sh cargo run -p effectcraft -- --demo # the desktop app, with the demo project cargo test --workspace -cargo xtask ci # what every commit must pass +cargo xtask ci # what every commit must pass (dev-profile checks and tests) ``` The default `dev` profile keeps debug info to line tables: builds are fast and panics/backtraces diff --git a/Cargo.lock b/Cargo.lock index 03e789f08..37233ef58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1422,6 +1422,7 @@ dependencies = [ name = "effectcraft" version = "0.23.1" dependencies = [ + "chrono", "cpal", "craft-update", "effectcraft-engine", @@ -1439,6 +1440,7 @@ dependencies = [ "rfd", "serde", "serde_json", + "sysinfo", "winit", "winresource", "zbus", @@ -3068,7 +3070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1bd49230192a3797a9a4d6abe9b3eed6f7fa4c8a8a4947977c6f80025f92cbd8" dependencies = [ "rustix 1.1.5", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -3668,7 +3670,7 @@ dependencies = [ "simd_cesu8", "thiserror 2.0.21", "walkdir", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -3814,7 +3816,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" dependencies = [ "cfg-if", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -4120,6 +4122,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "ntapi" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3b335231dfd352ffb0f8017f3b6027a4917f7df785ea2143d8af2adc66980ae" +dependencies = [ + "winapi", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -4434,6 +4445,16 @@ dependencies = [ "objc2-core-foundation", ] +[[package]] +name = "objc2-io-kit" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33fafba39597d6dc1fb709123dfa8289d39406734be322956a69f0931c73bb15" +dependencies = [ + "libc", + "objc2-core-foundation", +] + [[package]] name = "objc2-io-surface" version = "0.3.2" @@ -4664,7 +4685,7 @@ dependencies = [ "libc", "redox_syscall 0.5.18", "smallvec", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -6071,6 +6092,20 @@ dependencies = [ "walkdir", ] +[[package]] +name = "sysinfo" +version = "0.37.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16607d5caffd1c07ce073528f9ed972d88db15dd44023fa57142963be3feb11f" +dependencies = [ + "libc", + "memchr", + "ntapi", + "objc2-core-foundation", + "objc2-io-kit", + "windows 0.61.3", +] + [[package]] name = "tag_ptr" version = "0.1.0" @@ -7224,16 +7259,38 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows" +version = "0.61.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893" +dependencies = [ + "windows-collections 0.2.0", + "windows-core 0.61.2", + "windows-future 0.2.1", + "windows-link 0.1.3", + "windows-numerics 0.2.0", +] + [[package]] name = "windows" version = "0.62.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "527fadee13e0c05939a6a05d5bd6eec6cd2e3dbd648b9f8e447c6518133d8580" dependencies = [ - "windows-collections", + "windows-collections 0.3.2", "windows-core 0.62.2", - "windows-future", - "windows-numerics", + "windows-future 0.3.2", + "windows-numerics 0.3.1", +] + +[[package]] +name = "windows-collections" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" +dependencies = [ + "windows-core 0.61.2", ] [[package]] @@ -7255,6 +7312,19 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-core" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link 0.1.3", + "windows-result 0.3.4", + "windows-strings 0.4.2", +] + [[package]] name = "windows-core" version = "0.62.2" @@ -7263,9 +7333,20 @@ checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" dependencies = [ "windows-implement", "windows-interface", - "windows-link", + "windows-link 0.2.1", "windows-result 0.4.1", - "windows-strings", + "windows-strings 0.5.1", +] + +[[package]] +name = "windows-future" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", + "windows-threading 0.1.0", ] [[package]] @@ -7275,8 +7356,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e1d6f90251fe18a279739e78025bd6ddc52a7e22f921070ccdc67dde84c605cb" dependencies = [ "windows-core 0.62.2", - "windows-link", - "windows-threading", + "windows-link 0.2.1", + "windows-threading 0.2.1", ] [[package]] @@ -7301,12 +7382,28 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "windows-link" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-numerics" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", +] + [[package]] name = "windows-numerics" version = "0.3.1" @@ -7314,7 +7411,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e2e40844ac143cdb44aead537bbf727de9b044e107a0f1220392177d15b0f26" dependencies = [ "windows-core 0.62.2", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -7323,9 +7420,9 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" dependencies = [ - "windows-link", + "windows-link 0.2.1", "windows-result 0.4.1", - "windows-strings", + "windows-strings 0.5.1", ] [[package]] @@ -7337,13 +7434,31 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-result" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows-result" version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" dependencies = [ - "windows-link", + "windows-link 0.2.1", +] + +[[package]] +name = "windows-strings" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" +dependencies = [ + "windows-link 0.1.3", ] [[package]] @@ -7352,7 +7467,7 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -7388,7 +7503,7 @@ version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -7422,13 +7537,22 @@ dependencies = [ "windows_x86_64_msvc 0.52.6", ] +[[package]] +name = "windows-threading" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66463ad2e0ea3bbf808b7f1d371311c80e115c0b71d60efc142cafbcfb057a6" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows-threading" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3949bd5b99cafdf1c7ca86b43ca564028dfe27d66958f2470940f73d86d75b37" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index b8634178e..f52401a00 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -31,6 +31,7 @@ //! effectcraft-cli bench --roto [--frames N] [--sam2 model.safetensors] [--mobilesam mobile_sam.pt] [--fixes] [--clip FRAMES --truth MASKS] [--json] Roto Brush trackers on synthetic clips with known mattes //! (classical engine, MobileSAM and SAM 2.1 (any installed size) when installed; IoU, flicker, re-appearance, seconds per frame) //! effectcraft-cli script FILE.jsx [F.ecproj] | --eval CODE run an After Effects-style script +//! effectcraft-cli replay SCRIPT.jsonl [--keep-going] replay a repro script (docs/agents.md) //! effectcraft-cli mcp [--autosave | --bridge PORT] MCP server on stdio //! //! Project: --project F.ecproj (or a positional *.ecproj) | --demo | --empty (default: demo; mcp: empty) @@ -97,7 +98,7 @@ fn stdout_broken() -> bool { STDOUT_FAILED.get().is_some_and(|k| *k != std::io::ErrorKind::BrokenPipe) } -const USAGE: &str = "usage: effectcraft-cli [args] [--json] +const USAGE: &str = "usage: effectcraft-cli [args] [--json] info project + engine summary commands [--filter TEXT] [--enabled] list engine commands exec [--params JSON] run one engine command (exec --list: list them, as `commands`) @@ -138,6 +139,9 @@ const USAGE: &str = "usage: effectcraft-cli Result<(), Failure> { Ok(()) } +/// `replay SCRIPT.jsonl`: run a repro script against a headless session (its start line picks +/// the project) and report every step. Exits 1 when the replay fails. +fn replay_cmd(args: &Args, json_out: bool) -> Result<(), Failure> { + use effectcraft_engine::replay; + let Some(file) = args.pos.first() else { return usage_err("replay needs a .jsonl script") }; + if args.opt("--bridge").is_some() || args.project.is_some() || args.flag("--demo") || args.flag("--empty") { + return usage_err("replay runs headless from the script's start line ({\"start\": \"new\" | \"demo\" | {\"project\": path}})"); + } + use std::io::Read; + let path = std::path::Path::new(file); + let input = std::fs::File::open(path).map_err(|e| Failure::Error(format!("cannot read {file}: {e}")))?; + // Bound the read itself: metadata can be stale or describe a pipe rather than its contents. + let mut script = String::new(); + input.take(replay::MAX_SCRIPT_BYTES as u64 + 1).read_to_string(&mut script).map_err(|e| Failure::Error(format!("cannot read {file}: {e}")))?; + if script.len() > replay::MAX_SCRIPT_BYTES { + return Err(Failure::Error(format!("{file} is larger than {} MB", replay::MAX_SCRIPT_BYTES >> 20))); + } + let mut s = session(args)?; + let opts = replay::Options { keep_going: args.flag("--keep-going"), base_dir: path.parent().map(std::path::Path::to_path_buf) }; + let report = replay::run(&mut s, &script, &opts); + if json_out { + emit(&serde_json::to_value(&report).unwrap_or_else(|e| json!({"error": e.to_string()})), true); + } else { + for st in &report.steps { + let what = st.command.as_deref().unwrap_or(st.kind); + match (&st.error, st.ok) { + (None, _) => say!("line {:>4} ok {what}", st.line), + (Some(e), true) => say!("line {:>4} ok {what} (tolerated error: {e})", st.line), + (Some(e), false) => say!("line {:>4} FAIL {what}: {e}", st.line), + } + } + } + let Some(first) = &report.first_failure else { + if !json_out { + say!("replay ok: {} steps", report.steps.len()); + } + return Ok(()); + }; + let msg = format!( + "failed at line {} ({}){}: {}{}", + first.line, + first.command.as_deref().unwrap_or(first.kind), + if report.panicked { ", a command panicked" } else { "" }, + first.error.as_deref().unwrap_or(""), + if report.skipped > 0 { format!(" ({} later steps not run)", report.skipped) } else { String::new() } + ); + if json_out { + note!("effectcraft-cli replay: {msg}"); + std::process::exit(1); + } + Err(Failure::Error(msg)) +} + /// Print a result: compact JSON with `--json`, else pretty JSON. fn emit(v: &Value, json_out: bool) { if json_out { @@ -614,6 +672,7 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { "commands" => list_commands_cmd(args, json_out)?, "exec" if args.flag("--list") => list_commands_cmd(args, json_out)?, "script" => script_cmd(args, json_out)?, + "replay" => replay_cmd(args, json_out)?, "exec" => { let Some(id) = args.pos.first().cloned() else { return usage_err("exec needs a command id") }; let params = match (args.opt("--params"), args.pos.get(1).map(String::as_str)) { diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 363eb8e52..4510f16c9 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -1223,3 +1223,39 @@ fn render_exr_compression_and_depth() { assert!(reply["error"].as_str().unwrap().contains("exrBitDepth: half|float"), "{reply}"); let _ = std::fs::remove_dir_all(&root); } + +#[test] +fn replay_reports_steps_and_the_first_failure() { + let repro = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../tests/repros/comp-zero-frame-rate.jsonl"); + let v = ok_json(&["replay", repro.to_str().unwrap()]); + assert_eq!(v["ok"], true, "{v}"); + assert!(v["steps"].as_array().unwrap().len() > 5); + let bad = tmp("bad-repro.jsonl"); + std::fs::write( + &bad, + "{\"start\": \"new\"}\n{\"command\": \"comp.new\"}\n{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/width\", \"equals\": 1}}\nnot json\n", + ) + .unwrap(); + let (code, v) = run_json(&["replay", bad.to_str().unwrap()]); + assert_eq!(code, 1); + assert_eq!(v["ok"], false); + assert_eq!(v["first_failure"]["line"], 3, "{v}"); + assert_eq!(v["skipped"], 1); + let out = bin().args(["replay", bad.to_str().unwrap(), "--keep-going"]).output().unwrap(); + assert_eq!(out.status.code(), Some(1)); + let text = String::from_utf8_lossy(&out.stdout); + assert!(text.contains("line 2 ok comp.new") && text.contains("line 4 FAIL invalid: not JSON"), "{text}"); + assert!(String::from_utf8_lossy(&out.stderr).contains("failed at line 3 (comp.info)")); + // The start line picks the project. + assert_eq!(bin().args(["replay", bad.to_str().unwrap(), "--demo"]).output().unwrap().status.code(), Some(2)); +} + +/// Text editors can prefix an exported JSON-lines repro with a UTF-8 BOM. +#[test] +fn replay_accepts_a_utf8_bom() { + let path = tmp("bom-repro.jsonl"); + std::fs::write(&path, "\u{feff}{\"command\":\"comp.new\"}\n").unwrap(); + let (code, report) = run_json(&["replay", path.to_str().unwrap()]); + assert_eq!(code, 0, "{report}"); + assert_eq!(report["ok"], true); +} diff --git a/apps/effectcraft/Cargo.toml b/apps/effectcraft/Cargo.toml index 3030ee96d..21bd46755 100644 --- a/apps/effectcraft/Cargo.toml +++ b/apps/effectcraft/Cargo.toml @@ -25,6 +25,9 @@ rfd = { workspace = true } pollster = { workspace = true } cpal = { workspace = true } log = { workspace = true } +chrono = { workspace = true } +# MIT; safe process handles and target-only memory observation for the crash helper. +sysinfo = { version = "=0.37.2", default-features = false, features = ["system"] } [target.'cfg(target_os = "linux")'.dependencies] # Native GNOME/Wayland controls without a bundled title font. diff --git a/apps/effectcraft/src/crash_observation.rs b/apps/effectcraft/src/crash_observation.rs new file mode 100644 index 000000000..b67dc07fe --- /dev/null +++ b/apps/effectcraft/src/crash_observation.rs @@ -0,0 +1,264 @@ +//! Bounded observations made outside the failed GUI. Measurements are evidence, not a cause. +use std::io::{self, Write}; +use std::sync::mpsc::{self, Receiver}; +use std::time::{Duration, Instant, SystemTime}; + +use sysinfo::{MemoryRefreshKind, Pid, ProcessRefreshKind, ProcessesToUpdate, System}; + +pub fn utc() -> String { + chrono::DateTime::::from(SystemTime::now()).to_rfc3339_opts(chrono::SecondsFormat::Millis, true) +} + +#[derive(Default)] +struct Memory { + samples: u64, + resident_peak: u64, + virtual_peak: u64, + available_min: Option, + total: u64, + last_at: Option, + last_uptime_secs: Option, +} + +impl Memory { + fn sample(&mut self, resident: u64, virtual_bytes: u64, available: u64, total: u64) { + self.samples = self.samples.saturating_add(1); + self.resident_peak = self.resident_peak.max(resident); + self.virtual_peak = self.virtual_peak.max(virtual_bytes); + // Zero total means the platform could not supply physical-memory information. + if total != 0 { + self.available_min = Some(self.available_min.map_or(available, |old| old.min(available))); + self.total = total; + } + self.last_at = Some(utc()); + } +} + +pub struct Observation { + pid: Pid, + system: System, + birth: Option, + started: Instant, + started_at: String, + detected_at: Option, + last_sample: Instant, + memory: Memory, + // Only Windows can retrieve an unrelated process's ExitStatus through a retained handle. + exit: Receiver>, + status: Option, + alive: Option, +} + +fn target_system(pid: Pid) -> System { + let mut system = System::new(); + system.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::nothing().with_memory()); + system +} + +impl Observation { + /// Acquire the Windows handle before acknowledging reporter readiness, so even an immediate + /// GUI abort leaves an observable process object. Never enumerate commands or environments. + pub fn start(pid: u32) -> Self { + let pid = Pid::from_u32(pid); + let system = target_system(pid); + let birth = system.process(pid).map(|p| p.start_time()); + let (send, exit) = mpsc::sync_channel(1); + #[cfg(windows)] + { + let waiting = target_system(pid); + let _ = std::thread::Builder::new().name("ec-exit-status".into()).spawn(move || { + let status = waiting.process(pid).and_then(|p| p.wait()); + let _ = send.send(status); + }); + } + #[cfg(not(windows))] + drop(send); + let now = Instant::now(); + let mut result = Self { + pid, + system, + birth, + started: now, + started_at: utc(), + detected_at: None, + last_sample: now, + memory: Memory::default(), + exit, + status: None, + alive: None, + }; + result.sample(); + result + } + + fn sample(&mut self) { + self.system.refresh_processes_specifics(ProcessesToUpdate::Some(&[self.pid]), false, ProcessRefreshKind::nothing().with_memory()); + self.system.refresh_memory_specifics(MemoryRefreshKind::nothing().with_ram()); + let process = self.system.process(self.pid).filter(|p| Some(p.start_time()) == self.birth && p.exists()); + self.alive = self.birth.map(|_| process.is_some()); + if let Some(process) = process { + self.memory.last_uptime_secs = Some(process.run_time()); + self.memory.sample(process.memory(), process.virtual_memory(), self.system.available_memory(), self.system.total_memory()); + } + if let Ok(status) = self.exit.try_recv() { + self.status = status; + } + self.last_sample = Instant::now(); + } + + pub fn tick(&mut self) { + if self.last_sample.elapsed() >= Duration::from_secs(1) { + self.sample(); + } + } + + /// Pipe closure can precede the OS signalling process termination. Bound that grace period; + /// a broken pipe or early reporter drop must never be called a confirmed process exit. + pub fn finish(&mut self) { + self.detected_at = Some(utc()); + if self.status.is_none() + && let Ok(status) = self.exit.recv_timeout(Duration::from_millis(750)) + { + self.status = status; + } + self.sample(); + } + + pub fn confirmed_exit(&self) -> bool { + self.status.is_some() + } + + pub fn write(&self, report: &mut impl Write) -> io::Result<()> { + writeln!( + report, + "Detection UTC: {}\nObserver started UTC: {}\nObserved duration: {} ms", + self.detected_at.as_deref().unwrap_or("not captured"), + self.started_at, + self.started.elapsed().as_millis() + )?; + writeln!( + report, + "Process observation: {}", + if self.confirmed_exit() { + "process termination observed" + } else if self.alive == Some(true) { + "GUI still present when the control channel ended" + } else { + "target absent from refresh or process identity/status unavailable (not proof of termination)" + } + )?; + match &self.status { + Some(status) => { + writeln!(report, "OS exit status: {status}")?; + #[cfg(windows)] + if let Some(code) = status.code() { + write_windows_status(report, code as u32)?; + } + } + None => writeln!(report, "OS exit status: unavailable (not inferred from the control pipe)")?, + } + let memory = &self.memory; + writeln!(report, "\nMemory evidence (1 s sampling; sampled maxima, not OS peaks):\nTarget-present sampling attempts: {}", memory.samples)?; + if let Some(uptime) = memory.last_uptime_secs { + writeln!(report, "GUI uptime at last target-present observation: {uptime} s")?; + } + #[cfg(windows)] + let virtual_label = "Peak sampled private committed bytes"; + #[cfg(not(windows))] + let virtual_label = "Peak sampled virtual bytes"; + if memory.samples != 0 { + writeln!( + report, + "Last target sampling attempt UTC: {}\nPeak sampled resident bytes: {}\n{virtual_label}: {}", + memory.last_at.as_deref().unwrap_or("unavailable"), + memory.resident_peak, + memory.virtual_peak + )?; + } + if let Some(available) = memory.available_min { + writeln!(report, "Minimum sampled system available RAM bytes: {available}\nSystem total RAM bytes at last sample: {}", memory.total)?; + } + writeln!( + report, + "Samples can miss short spikes or retain stale/zero readings when an OS query fails; per-field success is not exposed by sysinfo. Windows private commit is not GPU VRAM; on other platforms virtual memory is not physical RAM. System commit limit/usage and GPU memory budgets were not collected. No memory-pressure threshold establishes the cause of this incident.\nThe memory observer does not collect process command lines, environments, documents, native dumps or system event logs. The existing session-log tail below may contain paths or other logged details; review it before sharing." + ) + } +} + +#[cfg(any(windows, test))] +fn write_windows_status(report: &mut impl Write, code: u32) -> io::Result<()> { + let meaning = match code { + 0xc0000005 => "STATUS_ACCESS_VIOLATION", + 0xc0000017 => "STATUS_NO_MEMORY", + 0xc000012d => "STATUS_COMMITMENT_LIMIT", + 0xc0000374 => "STATUS_HEAP_CORRUPTION", + 0xc0000409 => "STATUS_STACK_BUFFER_OVERRUN / fast fail", + 0x40000015 => "STATUS_FATAL_APP_EXIT", + _ => "unclassified", + }; + writeln!( + report, + "Windows status bits: 0x{code:08X} ({meaning})\nThese are process exit bits, not a verified exception record. Software can set an exit code explicitly; an access violation does not identify the faulting module or prove a driver crash." + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_target_without_exit_status_does_not_confirm_termination() { + let (send, exit) = mpsc::sync_channel(1); + drop(send); + let now = Instant::now(); + let observation = Observation { + pid: Pid::from_u32(1), + system: System::new(), + birth: Some(1), + started: now, + started_at: utc(), + detected_at: Some(utc()), + last_sample: now, + memory: Memory::default(), + exit, + status: None, + alive: Some(false), + }; + assert!(!observation.confirmed_exit()); + let mut report = Vec::new(); + observation.write(&mut report).unwrap(); + let report = String::from_utf8(report).unwrap(); + assert!(report.contains("not proof of termination")); + assert!(!report.contains("process termination observed")); + } + + #[test] + fn memory_high_water_marks_are_bounded_and_not_a_causal_diagnosis() { + let mut memory = Memory::default(); + memory.sample(40, 80, 30, 100); + memory.sample(20, 90, 50, 100); + assert_eq!((memory.samples, memory.resident_peak, memory.virtual_peak, memory.available_min), (2, 40, 90, Some(30))); + memory.sample(0, 0, 0, 0); + assert_eq!(memory.available_min, Some(30)); + } + + #[test] + fn windows_status_bits_preserve_unsigned_codes_without_claiming_a_driver_crash() { + for (code, name) in + [(0xc0000005, "STATUS_ACCESS_VIOLATION"), (0xc0000017, "STATUS_NO_MEMORY"), (0xc000012d, "STATUS_COMMITMENT_LIMIT"), (42, "unclassified")] + { + let mut text = Vec::new(); + write_windows_status(&mut text, code).unwrap(); + let text = String::from_utf8(text).unwrap(); + assert!(text.contains(name)); + assert!(text.contains(&format!("0x{code:08X}"))); + assert!(text.contains("not a verified exception record")); + } + } + + #[test] + fn utc_timestamps_are_parseable_and_zero_offset() { + let time = chrono::DateTime::parse_from_rfc3339(&utc()).unwrap(); + assert_eq!(time.offset().local_minus_utc(), 0); + } +} diff --git a/apps/effectcraft/src/crash_report.rs b/apps/effectcraft/src/crash_report.rs index 32306aa5c..6ca42aab7 100644 --- a/apps/effectcraft/src/crash_report.rs +++ b/apps/effectcraft/src/crash_report.rs @@ -2,23 +2,31 @@ //! //! Allocation failure can abort without a panic hook. The GUI therefore owns a pipe to a //! helper which never loads a project, font database or GPU. Only an explicit marker after -//! normal teardown suppresses the report. EOF alone identifies an unexpected exit, not its -//! cause. System-wide exhaustion can still prevent the helper or its native dialog working. +//! normal teardown suppresses the report. EOF alone identifies a lost control channel, not +//! a process exit or its cause. System-wide exhaustion can still prevent the helper or its native dialog working. +use std::collections::VecDeque; use std::ffi::{OsStr, OsString}; use std::fs::{File, OpenOptions}; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::process::{Child, ChildStdin, Command, Stdio}; +use std::sync::mpsc::{self, RecvTimeoutError}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +#[path = "crash_observation.rs"] +mod observation; + const FLAG: &str = "--internal-crash-reporter"; const CLEAN: &[u8] = b"EFFECTCRAFT_CLEAN_EXIT_V1\n"; const LOG_BYTES: usize = 64 * 1024; const HEADER_BYTES: usize = 128; +const READY: &[u8] = b"READY\n"; +const EVENT_BYTES: usize = 128; +const EVENT_COUNT: usize = 32; /// Owned only by the GUI, never handed to another child. Dropping it without `finish` closes -/// the pipe and reports an unexpected exit; there is intentionally no clean-exit Drop hook. +/// the pipe and reports the incident; there is intentionally no clean-exit Drop hook. pub struct Reporter { pipe: ChildStdin, child: Child, @@ -72,6 +80,20 @@ impl Shutdown { Ok(()) } + /// Keep only fixed-vocabulary, bounded lifecycle facts outside the GUI. Callers emit each + /// category once; their combined payload fits well below the smallest OS pipe buffer. + pub fn record(&self, event: &'static str) -> io::Result<()> { + if event.len() > 100 || event.contains(['\n', '\r']) { + return Err(io::Error::other("invalid diagnostic observation")); + } + let mut slot = self.0.lock().map_err(|_| io::Error::other("crash reporter ownership was poisoned"))?; + if let Some(reporter) = slot.as_mut() { + writeln!(reporter.pipe, "EVENT {event}")?; + reporter.pipe.flush()?; + } + Ok(()) + } + /// Consume the reporter exactly once, after the applicable platform teardown boundary. pub fn finish(&self) -> io::Result<()> { let reporter = self.0.lock().map_err(|_| io::Error::other("crash reporter ownership was poisoned"))?.take(); @@ -105,12 +127,18 @@ pub fn start(id: &str, log: Option<&Path>, reports: &Path, interactive: bool) -> log.map_or_else(OsString::new, |p| p.as_os_str().to_owned()), reports.as_os_str().to_owned(), OsString::from(if interactive { "1" } else { "0" }), + OsString::from(std::process::id().to_string()), ]; spawn(&exe, &args) } fn spawn(exe: &Path, args: &[OsString]) -> io::Result { - let mut child = Command::new(exe).args(args).stdin(Stdio::piped()).stdout(Stdio::null()).spawn()?; + let mut child = Command::new(exe).args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?; + if let Err(error) = ready(&mut child) { + let _ = child.kill(); + let _ = finish_child(&mut child, Duration::from_millis(100)); + return Err(error); + } match child.stdin.take() { Some(pipe) => Ok(Reporter { pipe, child }), None => { @@ -121,6 +149,36 @@ fn spawn(exe: &Path, args: &[OsString]) -> io::Result { } } +/// A fixed acknowledgement means the helper holds the process observation before heavy GUI +/// work begins. A suspended helper cannot hold startup indefinitely. +fn ready(child: &mut Child) -> io::Result<()> { + let mut output = child.stdout.take().ok_or_else(|| io::Error::other("crash reporter has no readiness pipe"))?; + let (send, receive) = mpsc::sync_channel(1); + std::thread::Builder::new().name("ec-reporter-ready".into()).spawn(move || { + let mut bytes = [0; READY.len()]; + #[cfg(not(test))] + let result = + output.read_exact(&mut bytes).and_then(|()| if bytes == READY { Ok(()) } else { Err(io::Error::other("invalid reporter acknowledgement")) }); + // libtest prints its bounded banner before invoking the helper role. + #[cfg(test)] + let result = (|| { + for _ in 0..1024 { + bytes.rotate_left(1); + output.read_exact(&mut bytes[READY.len() - 1..])?; + if bytes == READY { + return Ok(()); + } + } + Err(io::Error::other("reporter acknowledgement missing")) + })(); + let _ = send.send(result); + // Keep the pipe open for libtest's trailing result (and future helper output), without + // storing it. Dropping it at READY would turn a clean helper exit into BrokenPipe. + let _ = io::copy(&mut output, &mut io::sink()); + })?; + receive.recv_timeout(Duration::from_secs(3)).map_err(|_| io::Error::other("crash reporter readiness timed out"))? +} + /// Handle the internal mode before logging, argument parsing, updates or GUI initialization. /// The callback is a native dialog on this helper's main thread, not on the failed GUI's. pub fn maybe_run(args: &[OsString], show: impl FnOnce(&str)) -> bool { @@ -128,7 +186,7 @@ pub fn maybe_run(args: &[OsString], show: impl FnOnce(&str)) -> bool { return false; } let parsed = match args { - [_, id, log, reports, interactive] => id + [_, id, log, reports, interactive, pid] => id .to_str() .filter(|id| valid_id(id)) .zip(match interactive.to_str() { @@ -136,27 +194,61 @@ pub fn maybe_run(args: &[OsString], show: impl FnOnce(&str)) -> bool { Some("1") => Some(true), _ => None, }) - .map(|(id, interactive)| (id, (!log.is_empty()).then(|| Path::new(log)), Path::new(reports), interactive)), + .zip(pid.to_str().and_then(|p| p.parse::().ok()).filter(|pid| *pid != 0)) + .map(|((id, interactive), pid)| (id, (!log.is_empty()).then(|| Path::new(log)), Path::new(reports), interactive, pid)), _ => None, }; - let Some((id, log, reports, interactive)) = parsed else { + let Some((id, log, reports, interactive, pid)) = parsed else { eprintln!("EffectCraft crash reporter: invalid internal arguments"); return true; }; // Keep the exact file open while the GUI is alive: a later rotation must not substitute // another instance's log. If rotation won the startup race, the session marker rejects it. let mut log = SessionLog::open(log, id); - match clean_exit(&mut io::stdin().lock()) { - Ok(true) => {} - result => { - let failure = result.err().map(|e| e.to_string()); - let saved = write_report(reports, id, &mut log, failure.as_deref()); - let description = description(&saved); - eprintln!("{description}"); - if interactive { - show(&description); + let mut observation = observation::Observation::start(pid); + // A GUI that dies during helper startup can already have closed this read end. Keep + // watching its control pipe and save the report rather than losing that early failure. + let _ = io::stdout().write_all(READY).and_then(|()| io::stdout().flush()); + // The reader has one bounded queue; the monitor never waits on a live pipe, so it can + // sample the target even when no log or lifecycle message is emitted. + let (send, receive) = mpsc::sync_channel(1); + let reader = std::thread::Builder::new().name("ec-reporter-pipe".into()).spawn(move || { + let mut input = io::stdin().lock(); + loop { + let message = read_message(&mut input); + let terminal = !matches!(message, Ok(Message::Event(_))); + if send.send(message).is_err() || terminal { + break; } } + }); + let mut events = VecDeque::with_capacity(EVENT_COUNT); + let failure = if reader.is_err() { + Some("control reader could not start".to_owned()) + } else { + loop { + observation.tick(); + match receive.recv_timeout(Duration::from_millis(250)) { + Ok(Ok(Message::Clean)) => return true, + Ok(Ok(Message::Event(event))) => { + if events.len() == EVENT_COUNT { + events.pop_front(); + } + events.push_back(format!("{} {event}", observation::utc())); + } + Ok(Ok(Message::Ended)) => break None, + Ok(Err(error)) => break Some(error.to_string()), + Err(RecvTimeoutError::Timeout) => continue, + Err(RecvTimeoutError::Disconnected) => break Some("control reader disconnected".to_owned()), + } + } + }; + observation.finish(); + let saved = write_report(reports, id, &mut log, failure.as_deref(), &observation, &events); + let description = description(&saved, observation.confirmed_exit()); + eprintln!("{description}"); + if interactive { + show(&description); } true } @@ -165,20 +257,43 @@ fn valid_id(id: &str) -> bool { !id.is_empty() && id.len() <= 80 && id.bytes().all(|c| c.is_ascii_digit() || c == b'-') } -/// Fixed-size comparison: arbitrary/truncated input never grows a buffer or counts as clean. -fn clean_exit(input: &mut impl Read) -> io::Result { - let mut marker = [0u8; CLEAN.len()]; - let mut filled = 0; - while filled < marker.len() { - let remaining = marker.get_mut(filled..).ok_or_else(|| io::Error::other("invalid control marker offset"))?; - match input.read(remaining) { - Ok(0) => return Ok(false), - Ok(n) => filled += n, - Err(e) if e.kind() == io::ErrorKind::Interrupted => {} - Err(e) => return Err(e), +enum Message { + Clean, + Event(String), + Ended, +} + +/// Every line and the retained event ring are bounded, including malformed control input. +fn read_message(input: &mut impl Read) -> io::Result { + let mut line = [0u8; EVENT_BYTES]; + for i in 0..line.len() { + let byte = line.get_mut(i).ok_or_else(|| io::Error::other("invalid control offset"))?; + loop { + match input.read(std::slice::from_mut(byte)) { + Ok(0) => return Ok(Message::Ended), + Ok(_) => break, + Err(error) if error.kind() == io::ErrorKind::Interrupted => continue, + Err(error) => return Err(error), + } + } + if *byte == b'\n' { + let text = line.get(..=i).ok_or_else(|| io::Error::other("invalid control line"))?; + if text == CLEAN { + return Ok(Message::Clean); + } + if let Some(event) = text.strip_prefix(b"EVENT ") { + let event = std::str::from_utf8(event).map_err(|_| io::Error::other("non-UTF8 diagnostic observation"))?; + return Ok(Message::Event(event.trim_end().to_owned())); + } + return Ok(Message::Ended); } } - Ok(marker == CLEAN) + Err(io::Error::other("control line exceeded its fixed limit")) +} + +#[cfg(test)] +fn clean_exit(input: &mut impl Read) -> io::Result { + Ok(matches!(read_message(input)?, Message::Clean)) } enum SessionLog { @@ -228,7 +343,14 @@ impl SessionLog { } } -fn write_report(reports: &Path, id: &str, log: &mut SessionLog, pipe_error: Option<&str>) -> io::Result { +fn write_report( + reports: &Path, + id: &str, + log: &mut SessionLog, + pipe_error: Option<&str>, + observation: &observation::Observation, + events: &VecDeque, +) -> io::Result { if !valid_id(id) { return Err(io::Error::other("invalid report identity")); } @@ -244,8 +366,13 @@ fn write_report(reports: &Path, id: &str, log: &mut SessionLog, pipe_error: Opti )?; writeln!( report, - "The GUI's control pipe ended without its clean-exit marker.\nThe exit status and exact cause are not available to this reporter.\nMemory exhaustion, a native crash or external termination are possible.\nThis report was saved locally; it was not sent anywhere." + "The GUI control channel ended without its clean-exit marker.\nThe exact cause is not established. GPU fallback, memory pressure and process exit are separate observations, not a proven causal chain.\nThis report was saved locally; it was not sent anywhere." )?; + observation.write(&mut report)?; + writeln!(report, "\nRetained lifecycle observations (last {EVENT_COUNT}; UTC is helper receipt time):")?; + for event in events { + writeln!(report, "{event}")?; + } if let Some(error) = pipe_error { writeln!(report, "Control pipe error: {error}")?; } @@ -256,13 +383,15 @@ fn write_report(reports: &Path, id: &str, log: &mut SessionLog, pipe_error: Opti Ok(path) } -fn description(saved: &io::Result) -> String { +fn description(saved: &io::Result, confirmed_exit: bool) -> String { let diagnostic = match saved { Ok(path) => format!("A diagnostic report was saved to:\n{}", path.display()), Err(error) => format!("A diagnostic report could not be saved: {error}"), }; + let incident = + if confirmed_exit { "EffectCraft stopped unexpectedly." } else { "EffectCraft lost its diagnostic control channel; process exit is not confirmed." }; format!( - "EffectCraft stopped unexpectedly.\n\n{diagnostic}\n\nThe exact cause is unknown. Reopen EffectCraft to check for an existing auto-save. Changes since the last auto-save may be lost.\n\nNo report has been sent anywhere." + "{incident}\n\n{diagnostic}\n\nThe exact cause is unknown. If EffectCraft has closed, reopen it to check for an existing auto-save. Changes since the last auto-save may be lost.\n\nNo report has been sent anywhere." ) } @@ -276,6 +405,10 @@ mod tests { static NEXT: AtomicUsize = AtomicUsize::new(0); const ROLE: &str = "EFFECTCRAFT_CRASH_REPORT_TEST_ROLE"; + fn test_report(dir: &Path, id: &str, log: &mut SessionLog) -> io::Result { + write_report(dir, id, log, None, &observation::Observation::start(std::process::id()), &VecDeque::new()) + } + fn folder() -> PathBuf { let p = std::env::temp_dir().join(format!("ec-crash-report-{}-{}", run_id(), NEXT.fetch_add(1, Ordering::Relaxed))); std::fs::create_dir_all(&p).unwrap(); @@ -283,7 +416,7 @@ mod tests { } fn test_name(name: &str) -> String { - // Also usable as a standalone std-only rustc test while the app's build cache is busy. + // Also usable from an isolated harness with the reporter dependencies. let prefix = module_path!().split_once("::").map_or("", |(_, rest)| rest); format!("{prefix}::{name}") } @@ -299,10 +432,12 @@ mod tests { .env("EC_REPORT_DIR", dir) .env("EC_REPORT_ID", id) .env("EC_REPORT_INTERACTIVE", if interactive { "1" } else { "0" }) + .env("EC_REPORT_PID", std::process::id().to_string()) .stdin(Stdio::piped()) - .stdout(Stdio::null()) + .stdout(Stdio::piped()) .spawn() .unwrap(); + ready(&mut child).unwrap(); Reporter { pipe: child.stdin.take().unwrap(), child } } @@ -321,7 +456,7 @@ mod tests { for mut input in [b"".as_slice(), &CLEAN[..CLEAN.len() - 1], b"clean", b"EFFECTCRAFT_CLEAN_EXIT_V0\n"] { assert!(!clean_exit(&mut input).unwrap()); } - assert!(!clean_exit(&mut &vec![b'x'; 100_000][..]).unwrap()); + assert!(clean_exit(&mut &vec![b'x'; 100_000][..]).is_err()); } #[test] @@ -336,9 +471,9 @@ mod tests { let mut old = OpenOptions::new().append(true).open(dir.join("effectcraft.1.log")).unwrap(); old.write_all(&vec![b'a'; LOG_BYTES * 2]).unwrap(); old.write_all(b"\nGPU context retired\n").unwrap(); - let report = write_report(&dir, id, &mut log, None).unwrap(); + let report = test_report(&dir, id, &mut log).unwrap(); let text = std::fs::read_to_string(report).unwrap(); - assert!(text.len() < LOG_BYTES + 2048); + assert!(text.len() < LOG_BYTES + 4096); assert!(text.contains("GPU context retired")); assert!(!text.contains("OTHER SESSION")); std::fs::remove_dir_all(dir).unwrap(); @@ -350,7 +485,7 @@ mod tests { let path = dir.join("log"); std::fs::write(&path, "private other session").unwrap(); for (id, source) in [("1-1", Some(path.as_path())), ("1-2", None)] { - let report = write_report(&dir, id, &mut SessionLog::open(source, id), None).unwrap(); + let report = test_report(&dir, id, &mut SessionLog::open(source, id)).unwrap(); let text = std::fs::read_to_string(report).unwrap(); assert!(text.contains("Recent log unavailable")); assert!(!text.contains("private other session")); @@ -365,7 +500,7 @@ mod tests { std::fs::write(&path, "crash-report session 1-2\noriginal session\n").unwrap(); let mut log = SessionLog::open(Some(&path), "1-2"); std::fs::write(&path, "crash-report session 3-4\nprivate other session\n").unwrap(); - let report = write_report(&dir, "1-2", &mut log, None).unwrap(); + let report = test_report(&dir, "1-2", &mut log).unwrap(); let text = std::fs::read_to_string(report).unwrap(); assert!(text.contains("Recent log unavailable")); assert!(!text.contains("private other session")); @@ -388,13 +523,13 @@ mod tests { fn report_write_failure_is_described_and_existing_report_is_not_overwritten() { let dir = folder(); let mut log = SessionLog::Unavailable("no log".into()); - let first = write_report(&dir, "2-1", &mut log, None).unwrap(); + let first = test_report(&dir, "2-1", &mut log).unwrap(); let before = std::fs::read(&first).unwrap(); - let result = write_report(&dir, "2-1", &mut log, None); + let result = test_report(&dir, "2-1", &mut log); assert!(result.is_err()); - assert!(description(&result).contains("could not be saved")); + assert!(description(&result, false).contains("could not be saved")); assert_eq!(std::fs::read(first).unwrap(), before); - assert!(write_report(&dir, "../escape", &mut log, None).is_err()); + assert!(test_report(&dir, "../escape", &mut log).is_err()); std::fs::remove_dir_all(dir).unwrap(); } @@ -470,7 +605,10 @@ mod tests { assert!(!status.success()); wait_file(&dir.join("ready-4-1")); let text = std::fs::read_to_string(dir.join("crash-4-1.txt")).unwrap(); - assert!(text.contains("exact cause are not available")); + assert!(text.contains("exact cause is not established")); + assert!(text.contains("Detection UTC:")); + #[cfg(windows)] + assert!(text.contains("Windows status bits:")); std::fs::remove_dir_all(dir).unwrap(); } @@ -531,15 +669,70 @@ mod tests { std::fs::remove_dir_all(dir).unwrap(); } + #[test] + fn lost_pipe_keeps_bounded_lifecycle_evidence_without_claiming_process_exit() { + let dir = folder(); + let reporter = Shutdown::default(); + reporter.install(helper(&dir, "9-1")).unwrap(); + for _ in 0..40 { + reporter.record("GPU observation: wgpu validation error").unwrap(); + } + drop(reporter); + wait_file(&dir.join("ready-9-1")); + let text = std::fs::read_to_string(dir.join("crash-9-1.txt")).unwrap(); + assert!(text.contains("GUI still present")); + assert!(!text.contains("process termination observed")); + assert_eq!(text.matches("GPU observation: wgpu validation error").count(), EVENT_COUNT); + assert!(text.contains("Target-present sampling attempts:")); + assert!(text.len() < 8192); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn clean_exit_after_lifecycle_events_does_not_write_a_report() { + let dir = folder(); + let reporter = Shutdown::default(); + reporter.install(helper(&dir, "9-2")).unwrap(); + reporter.record("Backend selection source: default").unwrap(); + reporter.finish().unwrap(); + wait_file(&dir.join("ready-9-2")); + assert!(!dir.join("crash-9-2.txt").exists()); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + #[cfg(windows)] + fn windows_report_preserves_real_unsigned_exit_bits() { + let dir = folder(); + let status = Command::new(std::env::current_exe().unwrap()) + .args(["--exact", &test_name("aborting_parent"), "--ignored", "--nocapture"]) + .env(ROLE, "windows-status") + .env("EC_REPORT_DIR", &dir) + .stdout(Stdio::null()) + .status() + .unwrap(); + assert_eq!(status.code().map(|code| code as u32), Some(0xc0000017)); + wait_file(&dir.join("ready-9-3")); + let text = std::fs::read_to_string(dir.join("crash-9-3.txt")).unwrap(); + assert!(text.contains("Windows status bits: 0xC0000017 (STATUS_NO_MEMORY)")); + assert!(text.contains("not a verified exception record")); + assert!(text.contains("process termination observed")); + std::fs::remove_dir_all(dir).unwrap(); + } + #[test] fn internal_mode_rejects_invalid_arguments_without_launching_the_app() { assert!(!maybe_run(&[], |_| panic!("no dialog"))); assert!(!maybe_run(&[OsString::from("--version")], |_| panic!("no dialog"))); assert!(maybe_run(&[OsString::from(FLAG)], |_| panic!("no dialog"))); - let args = [FLAG, "../escape", "", "", "1"].map(OsString::from); + let args = [FLAG, "../escape", "", "", "1", "1"].map(OsString::from); assert!(maybe_run(&args, |_| panic!("no dialog"))); - let args = [FLAG, "1-1", "", "", "invalid"].map(OsString::from); + let args = [FLAG, "1-1", "", "", "invalid", "1"].map(OsString::from); assert!(maybe_run(&args, |_| panic!("no dialog"))); + for pid in ["0", "not-a-pid", "4294967296"] { + let args = [FLAG, "1-1", "", "", "0", pid].map(OsString::from); + assert!(maybe_run(&args, |_| panic!("no dialog"))); + } } #[test] @@ -556,7 +749,14 @@ mod tests { let dir = PathBuf::from(std::env::var_os("EC_REPORT_DIR").unwrap()); let id = std::env::var("EC_REPORT_ID").unwrap(); let interactive = std::env::var("EC_REPORT_INTERACTIVE").unwrap(); - let args = [OsString::from(FLAG), OsString::from(&id), OsString::new(), dir.as_os_str().to_owned(), OsString::from(&interactive)]; + let args = [ + OsString::from(FLAG), + OsString::from(&id), + OsString::new(), + dir.as_os_str().to_owned(), + OsString::from(&interactive), + std::env::var_os("EC_REPORT_PID").unwrap(), + ]; assert!(maybe_run(&args, |description| { assert_eq!(interactive, "1", "non-interactive reporter must not show a dialog"); assert!(dir.join(format!("crash-{id}.txt")).exists()); @@ -577,6 +777,12 @@ mod tests { after_teardown(Some(&reporter.clone()), || {}).unwrap(); std::process::exit(0); // Models macOS not returning from its native event loop. } + if role == "windows-status" { + let _reporter = helper(&dir, "9-3"); + // The real OS status is intentionally set, not actual memory exhaustion. The + // report must preserve this distinction rather than claiming a verified exception. + std::process::exit(0xc0000017u32 as i32); + } assert_eq!(role, "abort"); let _reporter = helper(&dir, "4-1"); std::process::abort(); diff --git a/apps/effectcraft/src/diagnostic_state.rs b/apps/effectcraft/src/diagnostic_state.rs new file mode 100644 index 000000000..042e6e88c --- /dev/null +++ b/apps/effectcraft/src/diagnostic_state.rs @@ -0,0 +1,147 @@ +//! Fixed-vocabulary observations retained by the crash helper independently of the log tail. +//! Driver messages and environment text can contain private data; never forward either here. + +use std::ffi::OsStr; +use std::sync::Arc; +use std::sync::atomic::{AtomicU8, Ordering}; + +use crate::crash_report::Shutdown; + +pub fn record(reporter: &Shutdown, event: &'static str) { + log::info!("diagnostic observation: {event}"); + if let Err(error) = reporter.record(event) { + log::warn!("could not retain diagnostic observation: {error}"); + } +} + +/// Mirror only wgpu's documented names/aliases, without calling its parser: that parser +/// logs unknown tokens verbatim. Unknown, non-Unicode and oversized values remain private. +pub fn backend_override_events(value: &OsStr) -> Vec<&'static str> { + let Some(value) = value.to_str().filter(|value| value.len() <= 4096) else { + return vec!["WGPU_BACKEND: non-Unicode or oversized value omitted"]; + }; + let mut selected = [false; 6]; + let mut unknown = false; + for token in value.split(',').map(str::trim) { + let aliases: &[&[&str]] = &[&["vulkan", "vk"], &["dx12", "d3d12"], &["metal", "mtl"], &["opengl", "gles", "gl"], &["webgpu"], &["noop"]]; + if let Some(index) = aliases.iter().position(|names| names.iter().any(|name| token.eq_ignore_ascii_case(name))) { + if let Some(flag) = selected.get_mut(index) { + *flag = true; + } + } else { + unknown = true; + } + } + let events = [ + "WGPU_BACKEND requests: vulkan", + "WGPU_BACKEND requests: dx12", + "WGPU_BACKEND requests: metal", + "WGPU_BACKEND requests: gl", + "WGPU_BACKEND requests: webgpu", + "WGPU_BACKEND requests: noop", + ]; + let mut result: Vec<_> = selected.into_iter().zip(events).filter_map(|(present, event)| present.then_some(event)).collect(); + if unknown { + result.push("WGPU_BACKEND: unrecognized or empty tokens omitted"); + } + result +} + +/// At most eight events per device, even when a driver floods its callbacks. A wgpu OOM +/// observation is not proof of system RAM exhaustion; device loss is not a driver-crash verdict. +#[derive(Clone)] +pub struct GpuObservations { + seen: Arc, + reporter: Shutdown, +} + +impl GpuObservations { + pub fn new(reporter: &Shutdown) -> Self { + Self { seen: Arc::new(AtomicU8::new(0)), reporter: reporter.clone() } + } + + pub fn uncaptured(&self, error: &eframe::wgpu::Error) { + let (bit, event) = match error { + eframe::wgpu::Error::OutOfMemory { .. } => (1, "GPU observation: wgpu out-of-memory error (not proof of system RAM exhaustion)"), + eframe::wgpu::Error::Validation { .. } => (2, "GPU observation: wgpu validation error"), + eframe::wgpu::Error::Internal { .. } => (4, "GPU observation: wgpu internal error"), + }; + self.once(bit, event); + } + + pub fn scoped(&self, error: effectcraft_ui_egui::gpu_failure::ScopedError) { + use effectcraft_ui_egui::gpu_failure::ScopedError; + let (bit, event) = match error { + ScopedError::OutOfMemory => (32, "GPU scoped observation: out-of-memory (not proof of system RAM exhaustion)"), + ScopedError::Internal => (64, "GPU scoped observation: internal error (not confirmed out-of-memory)"), + ScopedError::Validation => (128, "GPU scoped observation: validation error"), + }; + self.once(bit, event); + } + + pub fn device_lost(&self, reason: eframe::wgpu::DeviceLostReason) { + match reason { + eframe::wgpu::DeviceLostReason::Unknown => self.once(8, "GPU observation: device lost, reason unknown (not proof of a driver crash)"), + eframe::wgpu::DeviceLostReason::Destroyed => self.once(16, "GPU observation: device-lost callback, explicit device destruction"), + }; + } + + fn once(&self, bit: u8, event: &'static str) -> bool { + if self.seen.fetch_or(bit, Ordering::Relaxed) & bit != 0 { + return false; + } + record(&self.reporter, event); + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn override_names_are_normalized_deduplicated_and_private() { + let events = backend_override_events(OsStr::new(" Vk, VULKAN, d3d12, MTL, gles, webgpu, noop, private-token")); + assert_eq!(events.len(), 7); + assert_eq!(events.first(), Some(&"WGPU_BACKEND requests: vulkan")); + assert!(!events.join(" ").contains("private-token")); + assert!(events.iter().all(|event| event.len() <= 100)); + assert_eq!(backend_override_events(OsStr::new("")).len(), 1); + assert_eq!(backend_override_events(OsStr::new(&"x".repeat(4097))), ["WGPU_BACKEND: non-Unicode or oversized value omitted"]); + } + + #[test] + fn gpu_categories_are_latched_independently() { + let observations = GpuObservations::new(&Shutdown::default()); + observations.uncaptured(&eframe::wgpu::Error::OutOfMemory { source: Box::new(std::io::Error::other("private driver detail")) }); + observations.device_lost(eframe::wgpu::DeviceLostReason::Unknown); + observations.device_lost(eframe::wgpu::DeviceLostReason::Unknown); + assert_eq!(observations.seen.load(Ordering::Relaxed), 9); + observations.device_lost(eframe::wgpu::DeviceLostReason::Destroyed); + observations.uncaptured(&eframe::wgpu::Error::Validation { + source: Box::new(std::io::Error::other("private driver detail")), + description: "private validation detail".into(), + }); + observations.uncaptured(&eframe::wgpu::Error::Internal { + source: Box::new(std::io::Error::other("private driver detail")), + description: "private internal detail".into(), + }); + assert_eq!(observations.seen.load(Ordering::Relaxed), 31); + use effectcraft_ui_egui::gpu_failure::ScopedError; + observations.scoped(ScopedError::Internal); + assert_eq!(observations.seen.load(Ordering::Relaxed), 95, "internal scope must not set scoped OOM bit"); + observations.scoped(ScopedError::OutOfMemory); + observations.scoped(ScopedError::Validation); + observations.scoped(ScopedError::OutOfMemory); + assert_eq!(observations.seen.load(Ordering::Relaxed), 255); + assert!(!observations.once(1, "GPU observation: duplicate")); + assert!(!observations.once(8, "GPU observation: duplicate")); + } + + #[test] + #[cfg(unix)] + fn non_unicode_override_is_not_copied() { + use std::os::unix::ffi::OsStrExt; + assert_eq!(backend_override_events(OsStr::from_bytes(b"private\xff")), ["WGPU_BACKEND: non-Unicode or oversized value omitted"]); + } +} diff --git a/apps/effectcraft/src/launch_guard.rs b/apps/effectcraft/src/launch_guard.rs index fac91c05b..2656179ac 100644 --- a/apps/effectcraft/src/launch_guard.rs +++ b/apps/effectcraft/src/launch_guard.rs @@ -8,9 +8,9 @@ //! until its window has kept drawing for [`SETTLE`] (or the app quits normally before then). A //! launch that finds the previous one's marker switches Window Graphics to OpenGL, which those //! drivers run, and says so once the window is up. `WGPU_BACKEND` (wgpu's own override) still -//! wins over both. The marker is locked while its launch runs, so a second copy opened meanwhile -//! isn't taken for a crash; and a window OpenGL can't open either puts Window Graphics back to -//! Automatic ([`opengl_failed`]) rather than failing every launch. +//! wins over both. A separate guard file is locked while its launch runs, so a second copy opened +//! meanwhile isn't taken for a crash; and a window OpenGL can't open either puts Window Graphics +//! back to Automatic ([`opengl_failed`]) rather than failing every launch. use std::fs::File; use std::path::{Path, PathBuf}; @@ -21,6 +21,46 @@ use eframe::wgpu::Backends; /// Left in the settings folder from a launch until its window has settled. const MARKER: &str = "launch-pending"; +const LOCK: &str = "launch.lock"; + +/// Keep this file in place: unlinking a locked file lets a second inode acquire a new lock. +fn acquire_guard(dir: &Path) -> Option { + let file = match std::fs::create_dir_all(dir) + .and_then(|()| std::fs::OpenOptions::new().read(true).write(true).create(true).truncate(false).open(dir.join(LOCK))) + { + Ok(file) => file, + Err(e) => { + log::warn!("launch guard unavailable: {e}"); + return None; + } + }; + match file.try_lock() { + Ok(()) => {} + Err(std::fs::TryLockError::WouldBlock) => return None, + Err(e) => { + log::warn!("launch guard lock unavailable: {e}"); + return None; + } + } + // Older versions lock the marker itself. Probe only the lock: Windows also prevents + // reads through another handle while it is held. An unreadable marker is not a stale one. + match File::open(dir.join(MARKER)) { + Ok(marker) => match marker.try_lock() { + Ok(()) => {} + Err(std::fs::TryLockError::WouldBlock) => return None, + Err(e) => { + log::warn!("launch marker lock unavailable: {e}"); + return None; + } + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { + log::warn!("launch marker unavailable: {e}"); + return None; + } + } + Some(file) +} /// How long the window keeps drawing after its first frame before the launch counts as made. /// A driver that fails as the window is shown and its surface re-created does so within the @@ -28,16 +68,39 @@ const MARKER: &str = "launch-pending"; /// switch back in Settings. pub const SETTLE: Duration = Duration::from_secs(3); +/// Why this launch requested its window graphics APIs. An interrupted startup is an +/// observation, not proof that a driver caused the previous exit. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub enum BackendSource { + #[default] + Default, + SavedPreference, + InterruptedStartup, + EnvironmentOverride, +} + +impl BackendSource { + pub fn event(self) -> &'static str { + match self { + Self::Default => "backend source: default", + Self::SavedPreference => "backend source: saved OpenGL preference", + Self::InterruptedStartup => "backend source: previous interrupted startup; OpenGL fallback", + Self::EnvironmentOverride => "backend source: WGPU_BACKEND present", + } + } +} + /// This launch's window graphics. #[derive(Debug, Default)] pub struct Launch { /// The graphics APIs the window may use (`None`: eframe's default). pub backends: Option, + pub source: BackendSource, /// Switched to OpenGL because the previous launch stopped as its window opened: the message /// to show. pub notice: Option, marker: Option, - /// Holds the marker's lock until the launch settles (or the process ends). + /// Holds the separate guard file's lock until the launch settles (or the process ends). lock: Option, /// When the window drew its first frame, until the launch settles. first_frame: Option, @@ -48,14 +111,22 @@ impl Launch { /// launch stopped as its window opened (where OpenGL exists: not macOS), and leave this /// launch's marker. `env_override`: `WGPU_BACKEND` is set. pub fn begin(dir: Option<&Path>, env_override: bool) -> Launch { - let Some(dir) = dir.filter(|_| !env_override) else { return Launch::default() }; + if env_override { + return Launch { source: BackendSource::EnvironmentOverride, ..Launch::default() }; + } + let Some(dir) = dir else { return Launch::default() }; let prefs_path = dir.join(PREFS_FILE); let mut prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default(); let marker = dir.join(MARKER); - // Locked by another launch that is still opening its window: not a crash. - let held = File::open(&marker).is_ok_and(|f| matches!(f.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + let Some(lock) = acquire_guard(dir) else { + let backends = (prefs.startup.window_graphics == "gl").then_some(Backends::GL); + let source = if backends.is_some() { BackendSource::SavedPreference } else { BackendSource::Default }; + return Launch { backends, source, ..Launch::default() }; + }; + // Read again while owning the guard: another launch may have just changed preferences. + prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default(); let mut notice = None; - if marker.exists() && !held && prefs.startup.window_graphics == "auto" && cfg!(not(target_os = "macos")) { + if marker.exists() && prefs.startup.window_graphics == "auto" && cfg!(not(target_os = "macos")) { prefs.startup.window_graphics = "gl".into(); match std::fs::create_dir_all(dir).and_then(|()| std::fs::write(&prefs_path, prefs.to_json())) { Ok(()) => log::warn!("the last launch stopped as its window opened: switching Window Graphics to OpenGL"), @@ -69,15 +140,21 @@ impl Launch { ); } let backends = (prefs.startup.window_graphics == "gl").then_some(Backends::GL); - let (marker, lock) = match std::fs::create_dir_all(dir).and_then(|()| File::create(&marker)) { - // Where locks aren't available the guard works as before. - Ok(f) => (Some(marker), f.try_lock().is_ok().then_some(f)), + let source = if notice.is_some() { + BackendSource::InterruptedStartup + } else if backends.is_some() { + BackendSource::SavedPreference + } else { + BackendSource::Default + }; + let marker = match File::create(&marker) { + Ok(_) => Some(marker), Err(e) => { log::warn!("launch marker: {e}"); - (None, None) + None } }; - Launch { backends, notice, marker, lock, first_frame: None } + Launch { backends, source, notice, marker, lock: Some(lock), first_frame: None } } /// The window drew a frame. Until the launch has settled, returns how long that still takes @@ -104,18 +181,19 @@ impl Launch { /// The launch made it: release and remove its marker. fn settled(&mut self) { - self.lock = None; if let Some(m) = self.marker.take() && let Err(e) = std::fs::remove_file(&m) { log::warn!("launch marker: {e}"); } + self.lock = None; } } /// The window couldn't open with OpenGL (an error, not a crash): back to Automatic, and no marker. pub fn opengl_failed(dir: Option<&Path>) { let Some(dir) = dir else { return }; + let Some(_lock) = acquire_guard(dir) else { return }; let prefs_path = dir.join(PREFS_FILE); let mut prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default(); prefs.startup.window_graphics = "auto".into(); @@ -142,6 +220,7 @@ mod tests { // may still crash as the window is shown); frames for `SETTLE` do. let mut l = Launch::begin(Some(&dir), false); assert_eq!((l.backends, l.notice.is_some()), (None, false)); + assert_eq!(l.source, BackendSource::Default); assert!(dir.join(MARKER).exists()); let t0 = Instant::now(); assert_eq!(l.frame_drawn_at(t0), Some(SETTLE)); @@ -159,14 +238,18 @@ mod tests { } else { assert_eq!(l.backends, Some(Backends::GL)); assert!(l.notice.is_some()); + assert_eq!(l.source, BackendSource::InterruptedStartup); assert_eq!(window_graphics(&dir), "gl", "saved in Settings"); l.exited(); let l = Launch::begin(Some(&dir), false); assert_eq!((l.backends, l.notice.is_some()), (Some(Backends::GL), false), "it stays, without the notice"); + assert_eq!(l.source, BackendSource::SavedPreference); } // `WGPU_BACKEND` wins, and leaves no marker. let _ = std::fs::remove_file(dir.join(MARKER)); - assert_eq!(Launch::begin(Some(&dir), true).backends, None); + let overridden = Launch::begin(Some(&dir), true); + assert_eq!(overridden.backends, None); + assert_eq!(overridden.source, BackendSource::EnvironmentOverride); assert!(!dir.join(MARKER).exists()); let _ = std::fs::remove_dir_all(&dir); } @@ -217,4 +300,232 @@ mod tests { } let _ = std::fs::remove_dir_all(&dir); } + + /// A fresh settings folder for one test. + fn fresh_dir(case: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("ec-launch-guard-{case}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// Window Graphics saved as OpenGL. + fn save_opengl(dir: &Path) { + let mut prefs = Prefs::default(); + prefs.startup.window_graphics = "gl".into(); + std::fs::write(dir.join(PREFS_FILE), prefs.to_json()).unwrap(); + } + + #[test] + fn a_second_copy_reports_the_saved_backend_without_owning_the_guard() { + let dir = fresh_dir("second-source"); + save_opengl(&dir); + let mut first = Launch::begin(Some(&dir), false); + let mut second = Launch::begin(Some(&dir), false); + assert!(second.lock.is_none()); + assert_eq!((second.backends, second.source), (Some(Backends::GL), BackendSource::SavedPreference)); + second.exited(); + first.exited(); + let _ = std::fs::remove_dir_all(dir); + } + + /// A second copy that quits normally while the first is still opening doesn't remove the first + /// one's marker (#477). + #[test] + fn a_second_copy_quitting_leaves_the_running_launchs_marker() { + let dir = fresh_dir("second-quits"); + let mut first = Launch::begin(Some(&dir), false); + let mut second = Launch::begin(Some(&dir), false); + second.exited(); + let kept = dir.join(MARKER).exists(); + first.exited(); + let _ = std::fs::remove_dir_all(&dir); + assert!(kept, "the second copy removed the running launch's marker"); + } + + /// A second copy whose OpenGL window fails doesn't remove a running launch's marker. + #[test] + fn failed_opengl_in_a_second_copy_leaves_the_running_launchs_marker() { + let dir = fresh_dir("second-gl-fails"); + let mut first = Launch::begin(Some(&dir), false); + let second = Launch::begin(Some(&dir), false); + opengl_failed(Some(&dir)); + let kept = dir.join(MARKER).exists(); + drop(second); + first.exited(); + let _ = std::fs::remove_dir_all(&dir); + assert!(kept, "OpenGL cleanup removed another running launch's marker"); + } + + /// With `WGPU_BACKEND` set, a stale marker and a saved OpenGL choice are left as they are. + #[test] + fn wgpu_backend_leaves_a_stale_marker_and_saved_opengl_alone() { + let dir = fresh_dir("env-override"); + save_opengl(&dir); + std::fs::write(dir.join(MARKER), b"stale-marker").unwrap(); + let before = std::fs::read(dir.join(PREFS_FILE)).unwrap(); + let mut l = Launch::begin(Some(&dir), true); + assert_eq!(l.backends, None); + assert!(l.notice.is_none()); + l.exited(); + assert!(dir.join(MARKER).exists()); + assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before); + let _ = std::fs::remove_dir_all(&dir); + } + + /// After OpenGL fails to open a window, the next launch is back on Automatic, without a notice. + #[test] + fn failed_opengl_puts_the_next_launch_back_on_automatic() { + let dir = fresh_dir("gl-recovers"); + save_opengl(&dir); + let l = Launch::begin(Some(&dir), false); + assert_eq!(l.backends, Some(Backends::GL)); + drop(l); + opengl_failed(Some(&dir)); + assert!(!dir.join(MARKER).exists()); + assert_eq!(Prefs::from_json(&std::fs::read_to_string(dir.join(PREFS_FILE)).unwrap()).startup.window_graphics, "auto"); + let mut l = Launch::begin(Some(&dir), false); + assert_eq!(l.backends, None); + assert!(l.notice.is_none()); + l.exited(); + let _ = std::fs::remove_dir_all(&dir); + } + + /// The guard file is never removed or replaced: unlinking a locked file would let the next + /// launch lock a new one while the old lock is still held. + #[cfg(unix)] + #[test] + fn the_guard_file_stays_the_same_file_across_launches() { + use std::os::unix::fs::MetadataExt; + let dir = fresh_dir("guard-identity"); + let mut a = Launch::begin(Some(&dir), false); + let before = std::fs::metadata(dir.join(LOCK)).unwrap(); + let t0 = Instant::now(); + assert_eq!(a.frame_drawn_at(t0), Some(SETTLE)); + assert_eq!(a.frame_drawn_at(t0 + SETTLE), None); + assert!(!dir.join(MARKER).exists()); + let after = std::fs::metadata(dir.join(LOCK)).unwrap(); + assert_eq!((before.dev(), before.ino()), (after.dev(), after.ino())); + let mut b = Launch::begin(Some(&dir), false); + assert!(b.lock.is_some()); + let reused = std::fs::metadata(dir.join(LOCK)).unwrap(); + assert_eq!((before.dev(), before.ino()), (reused.dev(), reused.ino())); + b.exited(); + let _ = std::fs::remove_dir_all(dir); + } + + /// A second copy's frames and quit neither settle nor unlock the running launch. + #[test] + fn a_second_copys_frames_dont_settle_the_running_launch() { + let dir = fresh_dir("second-frames"); + let mut a = Launch::begin(Some(&dir), false); + std::fs::write(dir.join(MARKER), b"owner-marker").unwrap(); + let mut b = Launch::begin(Some(&dir), false); + assert!(b.lock.is_none()); + assert!(b.marker.is_none()); + let t0 = Instant::now(); + assert_eq!(b.frame_drawn_at(t0), None); + assert_eq!(b.frame_drawn_at(t0 + SETTLE), None); + b.exited(); + assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"owner-marker"); + let probe = File::open(dir.join(LOCK)).unwrap(); + assert!(matches!(probe.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + a.exited(); + let _ = std::fs::remove_dir_all(dir); + } + + /// Older versions lock the marker itself: while one runs, a new launch changes nothing, and + /// once it has stopped without settling the next launch still switches to OpenGL. + #[test] + fn a_running_older_version_holding_the_marker_is_left_alone() { + let dir = fresh_dir("older-version"); + std::fs::write(dir.join(MARKER), b"old-owner").unwrap(); + std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap(); + let old = File::open(dir.join(MARKER)).unwrap(); + old.try_lock().unwrap(); + let before = std::fs::read(dir.join(PREFS_FILE)).unwrap(); + let mut b = Launch::begin(Some(&dir), false); + assert!(b.lock.is_none()); + assert!(b.marker.is_none()); + assert!(b.backends.is_none()); + assert!(b.notice.is_none()); + b.exited(); + opengl_failed(Some(&dir)); + assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before); + // Windows locks forbid reading through another handle. Inspect the contents only after + // releasing the old owner's lock, but before recovery can truncate the marker. + drop(old); + assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"old-owner"); + let mut recovered = Launch::begin(Some(&dir), false); + if cfg!(not(target_os = "macos")) { + assert_eq!(recovered.backends, Some(Backends::GL)); + } + recovered.exited(); + let _ = std::fs::remove_dir_all(dir); + } + + /// A Windows sharing violation is not evidence that a marker is stale or absent. + #[cfg(windows)] + #[test] + fn an_unopenable_marker_leaves_settings_and_marker_alone() { + use std::os::windows::fs::OpenOptionsExt; + + let dir = fresh_dir("marker-unopenable"); + std::fs::write(dir.join(MARKER), b"old-owner").unwrap(); + std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap(); + let old = std::fs::OpenOptions::new().read(true).write(true).share_mode(0).open(dir.join(MARKER)).unwrap(); + let before = std::fs::read(dir.join(PREFS_FILE)).unwrap(); + let mut launch = Launch::begin(Some(&dir), false); + assert!(launch.lock.is_none()); + assert!(launch.marker.is_none()); + assert!(launch.backends.is_none()); + assert!(launch.notice.is_none()); + launch.exited(); + opengl_failed(Some(&dir)); + assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before); + drop(old); + assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"old-owner"); + let _ = std::fs::remove_dir_all(dir); + } + + /// A guard file that can't be opened leaves Window Graphics and the marker as they are. + #[test] + fn an_unusable_guard_file_leaves_settings_and_marker_alone() { + let dir = fresh_dir("guard-unusable"); + std::fs::create_dir(dir.join(LOCK)).unwrap(); + std::fs::write(dir.join(MARKER), b"stale-marker").unwrap(); + std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap(); + let before = std::fs::read(dir.join(PREFS_FILE)).unwrap(); + let mut l = Launch::begin(Some(&dir), false); + assert!(l.backends.is_none()); + assert!(l.notice.is_none()); + assert!(l.lock.is_none()); + assert!(l.marker.is_none()); + l.exited(); + opengl_failed(Some(&dir)); + assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before); + assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"stale-marker"); + let _ = std::fs::remove_dir_all(dir); + } + + /// OpenGL cleanup from elsewhere waits until the running launch has let go; then it resets + /// Window Graphics to Automatic and removes the marker. + #[test] + fn failed_opengl_cleanup_waits_for_the_running_launch() { + let dir = fresh_dir("gl-cleanup-waits"); + save_opengl(&dir); + let launch = Launch::begin(Some(&dir), false); + assert_eq!(launch.backends, Some(Backends::GL)); + assert!(launch.lock.is_some()); + let before = std::fs::read(dir.join(PREFS_FILE)).unwrap(); + opengl_failed(Some(&dir)); + assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before); + assert!(dir.join(MARKER).exists(), "cleanup elsewhere can't remove a running launch's marker"); + drop(launch); + opengl_failed(Some(&dir)); + let after = Prefs::from_json(&std::fs::read_to_string(dir.join(PREFS_FILE)).unwrap()).startup.window_graphics; + assert_eq!(after, "auto", "once the launch has let go, failed OpenGL resets Window Graphics"); + assert!(!dir.join(MARKER).exists()); + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index 3d9455c81..5a602adcb 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -23,6 +23,7 @@ mod apple_events; mod audio_out; mod control_server; mod crash_report; +mod diagnostic_state; mod launch_guard; #[cfg(target_os = "macos")] mod native_menu; @@ -183,7 +184,7 @@ fn start_helpers_if_opted_in(session: &mut effectcraft_engine::Session) { fn main() -> eframe::Result { let args: Vec<_> = std::env::args_os().skip(1).collect(); if crash_report::maybe_run(&args, |description| { - rfd::MessageDialog::new().set_level(rfd::MessageLevel::Error).set_title("EffectCraft stopped unexpectedly").set_description(description).show(); + rfd::MessageDialog::new().set_level(rfd::MessageLevel::Error).set_title("EffectCraft diagnostic report").set_description(description).show(); }) { return Ok(()); } @@ -246,8 +247,15 @@ fn run_desktop(reporter: &crash_report::Shutdown) -> eframe::Result { #[cfg(target_os = "linux")] appimage::integrate_from_env(ICON_PNG); // Settings ▸ Startup & Repair ▸ Window Graphics, switched to OpenGL when the last launch - // stopped as its window opened (a crashing graphics driver, #243). - let mut launch = launch_guard::Launch::begin(effectcraft_host::config_dir().as_deref(), std::env::var_os("WGPU_BACKEND").is_some()); + // stopped as its window opened (the marker alone does not establish a cause). + let backend_override = std::env::var_os("WGPU_BACKEND"); + let mut launch = launch_guard::Launch::begin(effectcraft_host::config_dir().as_deref(), backend_override.is_some()); + diagnostic_state::record(reporter, launch.source.event()); + if let Some(value) = backend_override { + for event in diagnostic_state::backend_override_events(&value) { + diagnostic_state::record(reporter, event); + } + } let on_gl = launch.backends == Some(eframe::wgpu::Backends::GL); let notice = launch.notice.take(); let options = eframe::NativeOptions { @@ -281,16 +289,33 @@ fn run_desktop(reporter: &crash_report::Shutdown) -> eframe::Result { Box::new(move |cc| { EffectcraftApp::install_ime_reset_hooks(&cc.egui_ctx); match &cc.wgpu_render_state { - Some(rs) => log::info!("graphics: {:?}", rs.adapter.get_info()), - None => log::info!("graphics: no wgpu device"), + Some(rs) => { + let info = rs.adapter.get_info(); + log::info!("graphics: {info:?}"); + let event = match info.backend { + eframe::wgpu::Backend::Noop => "actual GPU backend: noop", + eframe::wgpu::Backend::Vulkan => "actual GPU backend: vulkan", + eframe::wgpu::Backend::Metal => "actual GPU backend: metal", + eframe::wgpu::Backend::Dx12 => "actual GPU backend: dx12", + eframe::wgpu::Backend::Gl => "actual GPU backend: gl", + eframe::wgpu::Backend::BrowserWebGpu => "actual GPU backend: webgpu", + }; + diagnostic_state::record(&desktop_reporter, event); + } + None => diagnostic_state::record(&desktop_reporter, "actual GPU backend: no wgpu device"), } // This executable owns the shared device's handlers. Install them before // EffectCraft's compositor pipelines are built, without changing handlers from // inside Gpu::new. eframe has already constructed its presentation renderer here. let gpu_failures = effectcraft_ui_egui::gpu_failure::GpuFailureBridge::new(&cc.egui_ctx); + let observations = diagnostic_state::GpuObservations::new(&desktop_reporter); + let scoped_observed = observations.clone(); + gpu_failures.set_scoped_error_observer(move |error| scoped_observed.scoped(error)); if let Some(rs) = &cc.wgpu_render_state { + let errors_observed = observations.clone(); let errors = gpu_failures.clone(); rs.device.on_uncaptured_error(std::sync::Arc::new(move |error| { + errors_observed.uncaptured(&error); let message = format!("uncaptured GPU error: {error}"); if errors.report(&message, false) { log::error!("{message}"); @@ -298,6 +323,7 @@ fn run_desktop(reporter: &crash_report::Shutdown) -> eframe::Result { })); let lost = gpu_failures.clone(); rs.device.set_device_lost_callback(move |reason, message| { + observations.device_lost(reason); lost.report(&format!("GPU device lost ({reason:?}): {message}"), true); }); } diff --git a/crates/automation/src/confine.rs b/crates/automation/src/confine.rs index 570726932..a9fc4665f 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -143,6 +143,8 @@ const WRITES: &[(&str, &[&str])] = &[ ("shortcuts.export", &["path"]), ("renderQueue.add", &["output", "path"]), ("file.exportTimeline", &["path"]), + ("journal.export", &["path"]), + ("help.exportRepro", &["path"]), // `path` / `output` may also be a file-name template, as for render.preRender. ("file.createProxy", &["path", "output"]), ]; @@ -280,6 +282,11 @@ impl Confinement { if matches!(id, "file.openRecent" | "file.importRecent" | "anim.applyRecentPreset") && !given { return Err(Error::BadArgs(format!("`{id}`: give `path` while file access is confined"))); } + // The default repro destination is the settings store, not an automation write root. + // Require a single explicit path so missing, null or array parameters cannot bypass it. + if id == "help.exportRepro" && params.get("path").and_then(Value::as_str).is_none() { + return Err(Error::BadArgs("`help.exportRepro`: give `path` inside an automation write root while file access is confined".into())); + } let mut lookups = vec![]; // An output the agent did not name is derived from where the project sits: the Default // Output Folder preference when that is set, beside the project file otherwise — and the diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 92b62da2f..8a1839ede 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1865,3 +1865,28 @@ fn automation_roots_check_relinked_footage_again() { assert!(!(px[0] > 200 && px[3] > 200), "the link pointed outside afterwards was read: {px:?}"); let _ = std::fs::remove_dir_all(&base); } + +/// Exporting a repro must not write to the settings store outside the automation roots. +#[test] +fn confined_repro_export_requires_an_explicit_writable_path() { + use crate::confine::Confinement; + use effectcraft_engine::config::{ConfigStore, MemoryConfig}; + use std::sync::Arc; + + let dir = tempfile::tempdir().unwrap(); + let store = Arc::new(MemoryConfig::default()); + let session = Session { config: Some(store.clone()), ..Default::default() }; + let mut s = McpServer::new(Backend::headless(session)); + s.backend().confine(Confinement::new(&[], &[dir.path().to_path_buf()]).unwrap()); + for params in [json!({}), json!({"path": null}), json!({"path": []}), json!({"path": [dir.path().join("repro.jsonl")]})] { + let (content, error) = call(&mut s, "execute_command", json!({"command": "help.exportRepro", "params": params})); + assert!(error, "{content:?}"); + assert!(store.list("Repros").is_empty(), "implicit settings-store writes must be refused"); + } + let path = dir.path().join("repro.jsonl"); + call_json(&mut s, "execute_command", json!({"command": "help.exportRepro", "params": {"path": path}})); + assert!(path.is_file()); + let journal = call_json(&mut s, "execute_command", json!({"command": "journal.export"})); + assert!(journal["script"].is_string(), "in-memory exports need no write root"); + assert!(store.list("Repros").is_empty()); +} diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 40603e8cd..09caa7cf4 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -31,6 +31,7 @@ pub(crate) mod frame_export; mod frontend; mod help; mod import_project; +mod journal; mod key_labels; mod key_transform; mod keys_more; @@ -202,6 +203,7 @@ pub fn command_specs() -> &'static [CommandSpec] { v.extend(nav_more::specs()); v.extend(render_queue::specs()); v.extend(help::specs()); + v.extend(journal::specs()); v.extend(query::specs()); v.extend(batch::specs()); v.extend(layer_menu::specs()); diff --git a/crates/engine/src/cmd/journal.rs b/crates/engine/src/cmd/journal.rs new file mode 100644 index 000000000..533cc498c --- /dev/null +++ b/crates/engine/src/cmd/journal.rs @@ -0,0 +1,53 @@ +//! The command journal as a replay script (see [`crate::replay`]): `journal.export` for agents +//! and `help.exportRepro` for bug reports. Neither is journaled. + +use serde_json::{Value, json}; + +use super::{CommandSpec, always, str_p}; +use crate::{Result, Session, replay}; + +/// `journal.export {path?}`: the script itself, or written to `path`. +fn export(s: &mut Session, p: &Value) -> Result { + if let Some(path) = str_p(p, "path") { + let (path, steps) = replay::save_repro(s, Some(path))?; + return Ok(json!({"path": path, "steps": steps})); + } + let script = replay::export(s, None); + Ok(json!({"steps": script.lines().count().saturating_sub(1), "script": script})) +} + +/// `help.exportRepro {path?}`: save the steps so far for a bug report (default: the settings +/// folder's `Repros`). +fn export_repro(s: &mut Session, p: &Value) -> Result { + let (path, steps) = replay::save_repro(s, str_p(p, "path"))?; + Ok(json!({"path": path, "steps": steps})) +} + +const fn spec(id: &'static str, label: &'static str, params: &'static str, run: super::Run) -> CommandSpec { + CommandSpec { id, label, menu: &[], shortcut: None, alt_shortcuts: &[], params, enabled: always, run, journal: false } +} + +pub fn specs() -> Vec { + #[allow(unused_mut)] + let mut v = vec![ + spec( + "journal.export", + "Export Command Journal", + "{path? (write the script there)} → {script, steps} or {path, steps}: the commands run since the project was last opened, as a replay script (docs/agents.md, Repro scripts)", + export, + ), + spec( + "help.exportRepro", + "Save Bug Repro...", + "{path? (default: Repros in the settings folder)} → {path, steps}: the steps since the project was opened, for a bug report (commands and parameters only)", + export_repro, + ), + ]; + // A command with a bug, for the crash-repro tests. + #[cfg(test)] + v.push(spec("test.panic", "Panic (test)", "{why?}", |_, p| { + let why = str_p(p, "why").unwrap_or("test").to_string(); + std::panic::panic_any(why) + })); + v +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 599aba1b7..f8edfbe08 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -21,6 +21,7 @@ pub mod menus; mod output_guard; pub mod preview; pub mod render_queue; +pub mod replay; pub mod scriptui; mod settings; mod system; diff --git a/crates/engine/src/preview.rs b/crates/engine/src/preview.rs index 76d21a806..7c0ee02cd 100644 --- a/crates/engine/src/preview.rs +++ b/crates/engine/src/preview.rs @@ -159,7 +159,8 @@ pub struct PreviewPreset { pub pre_roll: f64, pub post_roll: f64, pub play_from: PlayFrom, - /// Frames per second; `None` = Auto (the comp's rate). + /// Composition frames played per second; `None` = Auto (the comp's rate). + /// An override changes preview speed, including the audio's speed and pitch. pub frame_rate: Option, /// Frames skipped between the frames played (0 = every frame). pub skip: u32, diff --git a/crates/engine/src/replay.rs b/crates/engine/src/replay.rs new file mode 100644 index 000000000..5cca36918 --- /dev/null +++ b/crates/engine/src/replay.rs @@ -0,0 +1,748 @@ +//! Replay scripts: a bug or crash as a list of commands an agent (or a test) can run again. +//! +//! The session's command journal ([`Session::journal`]) becomes a JSON-lines script +//! (`journal.export`, `help.exportRepro`, and automatically when the last-resort guard catches a +//! panic, see [`after_command`]); `effectcraft-cli replay script.jsonl` runs one headless and +//! `crates/host/tests/it/repros.rs` runs every `tests/repros/*.jsonl` as a regression test. +//! +//! Format (one JSON object per line; blank lines and lines starting with `//` or `#` are +//! comments): +//! +//! ```text +//! {"start": "new" | "demo" | {"project": "path.ecproj"}, "allow_errors"?: bool} optional, first +//! {"command": "comp.new", "params": {...}, "expect_error"?: true | "substring"} +//! {"expect": {"query": "prop.get", "params": {...}, "pointer"?: "/value", "equals"?: ...}} +//! {"expect_no_error": true} +//! ``` +//! +//! A command must succeed unless it says `expect_error` (then it must fail, with that text in +//! the message when it is a string), or the start line says `allow_errors` (errors are reported +//! but tolerated; crash repros say so, since after the fix the crashing step may fail cleanly). +//! A panic always fails. `expect` runs a command (normally a query) and compares the value at +//! the JSON `pointer` of its result with `equals` (numbers within 1e-6); `expect_no_error` +//! checks that the previous command succeeded. Other keys (`version`, `app`, `note`, `crash`…) +//! are information for people. Relative `project` paths are relative to the script. +//! +//! Privacy: a repro holds command ids and their parameters only, never project or media file +//! contents. Parameters can include file paths, layer names and text typed into text layers. + +use std::cell::Cell; +use std::path::{Path, PathBuf}; + +use serde::Serialize; +use serde_json::{Map, Value, json}; + +use crate::{EngineError, Result, Session, guard}; + +/// The format version written into exported headers. +pub const VERSION: u64 = 1; +/// Largest script [`run`] accepts (bytes). +pub const MAX_SCRIPT_BYTES: usize = 64 << 20; +/// Most lines [`run`] reads. +pub const MAX_LINES: usize = 100_000; +/// Exported scripts stay below this size: the oldest steps are dropped (and counted). +const MAX_EXPORT_BYTES: usize = 32 << 20; +/// Folder of saved repros in the settings store (next to `Logs`). +pub const REPRO_DIR: &str = "Repros"; +/// Automatic crash repros kept (the oldest are removed). +const KEEP_CRASH_REPROS: usize = 20; +/// Commands that replace the whole project: an export starts at the last one. +const PROJECT_STARTS: &[&str] = &["file.newProject", "file.open", "file.openDemoProject"]; +/// Error and value texts in reports are cut to this many characters. +const MAX_TEXT: usize = 600; + +// ---------- nesting and crash capture (Session::execute) ---------- + +thread_local! { + /// Commands running on this thread: (all, journaled ones). + static DEPTH: Cell<(u32, u32)> = const { Cell::new((0, 0)) }; + /// A replay is running on this thread (its panics are reported, not saved as repros). + static REPLAYING: Cell = const { Cell::new(false) }; +} + +/// One running command (see [`Session::execute`]): commands it runs itself (a script's steps) +/// aren't journaled again, and only the outermost command saves a crash repro. +pub struct Nesting { + outer: (u32, u32), +} + +impl Nesting { + /// Record nesting so commands dispatched by a script are not exported twice. + pub fn enter(journaled: bool) -> Nesting { + let outer = DEPTH.with(Cell::get); + DEPTH.with(|d| d.set((outer.0.saturating_add(1), outer.1.saturating_add(u32::from(journaled))))); + Nesting { outer } + } + /// Run by a journaled command (which replays it). + pub fn inside_journaled(&self) -> bool { + self.outer.1 > 0 + } + /// Whether this command is responsible for capturing a nested panic. + pub fn outermost(&self) -> bool { + self.outer.0 == 0 + } +} + +impl Drop for Nesting { + fn drop(&mut self) { + DEPTH.with(|d| d.set(self.outer)); + } +} + +/// After the outermost command: when the guard caught a panic while it ran (`caught` is +/// [`guard::caught`] from before), save the journal plus this command as a crash repro in the +/// settings store and name it in the error. +pub fn after_command(s: &Session, id: &str, params: &Value, nesting: &Nesting, caught: u64, r: Result) -> Result { + if !nesting.outermost() || guard::caught() == caught || REPLAYING.with(Cell::get) { + return r; + } + let error = match &r { + Err(e) => e.to_string(), + Ok(_) => "a step it ran failed with an internal error".to_string(), + }; + let saved = save_crash_repro(s, id, params, &error); + match (r, saved) { + (Err(EngineError::Other(m)), Some(path)) => Err(EngineError::Other(format!("{m} (the steps that led here were saved to {path})"))), + (r, _) => r, + } +} + +/// Wall-clock seconds used only to label exported repros. +fn unix_secs() -> u64 { + web_time::SystemTime::now().duration_since(web_time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0) +} + +/// `Repros/--.jsonl`. +fn repro_name(prefix: &str, id: &str) -> String { + let id: String = id.chars().map(|c| if c.is_ascii_alphanumeric() || c == '.' { c } else { '_' }).take(60).collect(); + format!("{REPRO_DIR}/{prefix}-{:012}-{id}.jsonl", unix_secs()) +} + +/// Where a store entry is on disk (or its name in stores without a folder). +fn store_path(store: &dyn crate::config::ConfigStore, name: &str) -> String { + store.dir().map(|d| d.join(name).display().to_string()).unwrap_or_else(|| name.to_string()) +} + +/// Persist the failing command with its preceding journal and prune older crash repros. +fn save_crash_repro(s: &Session, id: &str, params: &Value, error: &str) -> Option { + let store = s.config.clone()?; + let mut text = export(s, Some(json!({"command": id, "error": cut(error)}))); + text.push_str(&command_line(id, params)); + text.push('\n'); + let name = repro_name("crash", id); + if let Err(e) = store.write(&name, &text) { + log::error!("cannot save the crash repro {name}: {e}"); + return None; + } + // Keep the newest few (names sort by time). + let mut old: Vec = store.list(REPRO_DIR).into_iter().filter(|n| n.starts_with("crash-")).collect(); + old.sort(); + let extra = old.len().saturating_sub(KEEP_CRASH_REPROS); + for n in old.into_iter().take(extra) { + let _ = store.remove(&format!("{REPRO_DIR}/{n}")); + } + let path = store_path(store.as_ref(), &name); + log::error!("crash repro saved to {path}"); + Some(path) +} + +/// Save the journal as a repro: at `path` (through the session's file services), else in the +/// settings store's `Repros` folder. Returns where it went and how many steps it holds. +pub fn save_repro(s: &Session, path: Option<&str>) -> Result<(String, usize)> { + let text = export(s, None); + let steps = text.lines().count().saturating_sub(1); + if let Some(path) = path { + s.services.write_file(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?; + return Ok((path.to_string(), steps)); + } + let store = s.config.clone().ok_or_else(|| EngineError::Other("there is no settings folder to save the repro in; pass `path`".into()))?; + let name = repro_name("repro", "session"); + store.write(&name, &text).map_err(|e| EngineError::Other(format!("cannot save {name}: {e}")))?; + Ok((store_path(store.as_ref(), &name), steps)) +} + +// ---------- export ---------- + +/// Serialize one command without including an empty parameter object. +fn command_line(id: &str, params: &Value) -> String { + let mut o = Map::new(); + o.insert("command".into(), json!(id)); + if !(params.is_null() || params.as_object().is_some_and(Map::is_empty)) { + o.insert("params".into(), params.clone()); + } + Value::Object(o).to_string() +} + +/// Saving writes the user's files and doesn't change the project: left out of repros. +fn skipped_in_export(id: &str) -> bool { + id.starts_with("file.save") || id.starts_with("file.increment") +} + +/// The journal as a replay script: a start line, then the steps since the project was last +/// replaced (`file.newProject` / `file.open` / `file.openDemoProject`; successful Revert records a resolved `file.open`). `crash` (the failing +/// command and its error) marks a crash repro, which tolerates errors on replay. +pub fn export(s: &Session, crash: Option) -> String { + let from = s.journal.iter().rposition(|(id, _)| PROJECT_STARTS.contains(&id.as_str())).unwrap_or(0); + let mut lines: Vec = + s.journal.get(from..).unwrap_or_default().iter().filter(|(id, _)| !skipped_in_export(id)).map(|(id, p)| command_line(id, p)).collect(); + let mut size: usize = lines.iter().map(|l| l.len().saturating_add(1)).sum(); + let mut dropped = 0usize; + while size > MAX_EXPORT_BYTES && !lines.is_empty() { + size = size.saturating_sub(lines.remove(0).len().saturating_add(1)); + dropped += 1; + } + let mut head = Map::new(); + head.insert("start".into(), json!("new")); + head.insert("version".into(), json!(VERSION)); + head.insert("app".into(), json!(format!("EffectCraft {}", env!("CARGO_PKG_VERSION")))); + head.insert("os".into(), json!(std::env::consts::OS)); + head.insert("exported".into(), json!(unix_secs())); + if let Some(c) = crash { + head.insert("allow_errors".into(), json!(true)); + head.insert("crash".into(), c); + } + if dropped > 0 { + head.insert("dropped".into(), json!(dropped)); + } + head.insert("note".into(), json!("commands and parameters only (no file contents); parameters can include file paths and text")); + let mut out = Value::Object(head).to_string(); + out.push('\n'); + for l in lines { + out.push_str(&l); + out.push('\n'); + } + out +} + +// ---------- parsing ---------- + +#[derive(Clone, Debug, PartialEq)] +enum Start { + New, + Demo, + Project(String), +} + +#[derive(Clone, Debug)] +enum Step { + Start(Start), + Command { + id: String, + params: Value, + expect_error: Option>, + }, + Expect { + query: String, + params: Value, + pointer: Option, + equals: Option, + }, + ExpectNoError, + /// A line that isn't a valid step (reported as a failure). + Invalid(String), +} + +fn params_of(v: Option<&Value>) -> std::result::Result { + match v { + None | Some(Value::Null) => Ok(json!({})), + Some(p @ Value::Object(_)) => Ok(p.clone()), + Some(_) => Err("`params` must be an object".into()), + } +} + +/// Decode the supported initial project states. +fn parse_start(v: &Value) -> std::result::Result { + match v { + Value::String(s) if s == "new" => Ok(Start::New), + Value::String(s) if s == "demo" => Ok(Start::Demo), + Value::Object(o) => match o.get("project").and_then(Value::as_str) { + Some(p) if !p.trim().is_empty() => Ok(Start::Project(p.to_string())), + _ => Err("`start` object needs a `project` path".into()), + }, + _ => Err("`start` must be \"new\", \"demo\" or {\"project\": path}".into()), + } +} + +/// One line → a step (`None`: a blank or comment line). +fn parse_line(text: &str, first: bool, allow_errors: &mut bool) -> Option { + let t = text.trim(); + if t.is_empty() || t.starts_with("//") || t.starts_with('#') { + return None; + } + let v: Value = match serde_json::from_str(t) { + Ok(v) => v, + Err(e) => return Some(Step::Invalid(format!("not JSON: {e}"))), + }; + let Some(o) = v.as_object() else { return Some(Step::Invalid("a line must be a JSON object".into())) }; + let step = if let Some(st) = o.get("start") { + if !first { + return Some(Step::Invalid("the `start` line must come first".into())); + } + *allow_errors = o.get("allow_errors").and_then(Value::as_bool).unwrap_or(false); + parse_start(st).map(Step::Start) + } else if let Some(c) = o.get("command") { + parse_command(c, o) + } else if let Some(e) = o.get("expect") { + parse_expect(e) + } else if o.get("expect_no_error").and_then(Value::as_bool) == Some(true) { + Ok(Step::ExpectNoError) + } else if o.len() == 1 && o.contains_key("comment") { + return None; + } else { + Err("expected a `start`, `command`, `expect` or `expect_no_error` line".to_string()) + }; + Some(step.unwrap_or_else(Step::Invalid)) +} + +/// Parse one command and its optional expected error without executing it. +fn parse_command(c: &Value, o: &Map) -> std::result::Result { + let id = c.as_str().filter(|s| !s.is_empty()).ok_or("`command` must be a command id")?; + let expect_error = match o.get("expect_error") { + None | Some(Value::Null | Value::Bool(false)) => None, + Some(Value::Bool(true)) => Some(None), + Some(Value::String(s)) => Some(Some(s.clone())), + Some(_) => return Err("`expect_error` must be true or a text the error contains".into()), + }; + Ok(Step::Command { id: id.to_string(), params: params_of(o.get("params"))?, expect_error }) +} + +/// Parse a query assertion, retaining the distinction between no expected value and JSON null. +fn parse_expect(e: &Value) -> std::result::Result { + let query = e.get("query").and_then(Value::as_str).filter(|s| !s.is_empty()).ok_or("`expect` needs a `query` command id")?; + let pointer = e.get("pointer").map(|p| p.as_str().map(str::to_string).ok_or("`pointer` must be a JSON pointer such as /value")).transpose()?; + Ok(Step::Expect { query: query.to_string(), params: params_of(e.get("params"))?, pointer, equals: e.get("equals").cloned() }) +} + +// ---------- running ---------- + +/// How to run a script. +#[derive(Clone, Debug, Default)] +pub struct Options { + /// Keep running after a failed step (default: stop at the first failure). + pub keep_going: bool, + /// Folder relative `start.project` paths are resolved against (the script's). + pub base_dir: Option, +} + +/// What happened to one line. +#[derive(Clone, Debug, Serialize)] +pub struct StepReport { + /// 1-based line number (0: the implicit start). + pub line: usize, + /// `start`, `command`, `expect`, `expect_no_error` or `invalid`. + pub kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + pub command: Option, + pub ok: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub panicked: bool, +} + +/// The outcome of [`run`]. +#[derive(Clone, Debug, Default, Serialize)] +pub struct Report { + pub ok: bool, + /// A step panicked (caught by the last-resort guard): always a failure. + pub panicked: bool, + pub steps: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub first_failure: Option, + /// Steps not run after the first failure. + pub skipped: usize, +} + +/// Cut a message to [`MAX_TEXT`] characters. +fn cut(s: &str) -> String { + match s.char_indices().nth(MAX_TEXT) { + Some((i, _)) => format!("{}…", s.get(..i).unwrap_or(s)), + None => s.to_string(), + } +} + +/// Run one command the way the app does, noting whether it panicked. +fn exec(s: &mut Session, id: &str, params: Value) -> (Result, bool) { + let before = guard::caught(); + let r = guard::guarded(&format!("replay of `{id}`"), || s.execute(id, params)); + s.events.clear(); + (r, guard::caught() != before) +} + +/// Numbers within 1e-6 (relative above 1), everything else exactly. +fn approx_eq(a: &Value, b: &Value) -> bool { + match (a, b) { + (Value::Number(x), Value::Number(y)) => match (x.as_f64(), y.as_f64()) { + (Some(x), Some(y)) => (x - y).abs() <= 1e-6 * x.abs().max(y.abs()).max(1.0), + _ => x == y, + }, + (Value::Array(x), Value::Array(y)) => x.len() == y.len() && x.iter().zip(y).all(|(a, b)| approx_eq(a, b)), + (Value::Object(x), Value::Object(y)) => x.len() == y.len() && x.iter().all(|(k, a)| y.get(k).is_some_and(|b| approx_eq(a, b))), + _ => a == b, + } +} + +/// Marks this thread as replaying while alive. +struct Replaying(bool); + +impl Replaying { + /// Suppress automatic crash exports for the duration of a replay. + fn enter() -> Replaying { + Replaying(REPLAYING.with(|r| r.replace(true))) + } +} + +impl Drop for Replaying { + fn drop(&mut self) { + REPLAYING.with(|r| r.set(self.0)); + } +} + +/// Create or open the initial project and retain whether its command panicked. +fn start_step(s: &mut Session, start: &Start, base: Option<&Path>) -> (String, Result, bool) { + match start { + Start::New => { + let (r, p) = exec(s, "file.newProject", json!({})); + ("file.newProject".into(), r, p) + } + Start::Demo => { + let (r, p) = exec(s, "file.openDemoProject", json!({})); + ("file.openDemoProject".into(), r, p) + } + Start::Project(path) => { + let full = match base { + Some(b) if Path::new(path).is_relative() => b.join(path).display().to_string(), + _ => path.clone(), + }; + let (r, p) = exec(s, "file.open", json!({"path": full})); + ("file.open".into(), r, p) + } + } +} + +/// Compare a query result and produce a bounded explanation when it fails. +fn expectation_error(r: Result, panicked: bool, pointer: Option<&str>, equals: Option<&Value>) -> Option { + let value = match r { + Ok(v) => v, + Err(e) => return Some(cut(&format!("{}{e}", if panicked { "panicked: " } else { "" }))), + }; + let got = match pointer { + Some(p) => value.pointer(p), + None => Some(&value), + }; + match (got, equals) { + (None, _) => Some(cut(&format!("no value at {} in {value}", pointer.unwrap_or("")))), + (Some(got), Some(want)) if !approx_eq(got, want) => Some(cut(&format!("expected {want}, got {got}"))), + _ => None, + } +} + +/// Execute one parsed line and remember whether the last command failed for `expect_no_error`. +fn run_step(s: &mut Session, line: usize, step: Step, opts: &Options, allow_errors: bool, last_command_failed: &mut Option) -> StepReport { + let mut sr = StepReport { line, kind: "command", command: None, ok: true, error: None, panicked: false }; + match step { + Step::Invalid(m) => { + sr.kind = "invalid"; + sr.ok = false; + sr.error = Some(m); + } + Step::Start(start) => { + sr.kind = "start"; + let (id, r, panicked) = start_step(s, &start, opts.base_dir.as_deref()); + sr.command = Some(id); + sr.panicked = panicked; + sr.ok = !panicked; + if let Err(e) = r { + sr.ok = false; + sr.error = Some(cut(&e.to_string())); + } + } + Step::Command { id, params, expect_error } => { + let (r, panicked) = exec(s, &id, params); + sr.command = Some(id); + sr.panicked = panicked; + *last_command_failed = Some(r.is_err()); + let err = r.err().map(|e| e.to_string()); + let (ok, error) = match (err, expect_error) { + (e, _) if panicked => (false, Some(format!("panicked: {}", e.unwrap_or_else(|| "a step failed with an internal error".into())))), + (None, None) => (true, None), + (None, Some(_)) => (false, Some("expected an error, but the command succeeded".into())), + (Some(e), Some(Some(want))) if !e.contains(want.as_str()) => (false, Some(format!("expected an error containing {want:?}, got: {e}"))), + (Some(_), Some(_)) => (true, None), + (Some(e), None) => (allow_errors, Some(e)), + }; + sr.ok = ok; + sr.error = error.as_deref().map(cut); + } + Step::Expect { query, params, pointer, equals } => { + sr.kind = "expect"; + let (r, panicked) = exec(s, &query, params); + sr.command = Some(query); + sr.panicked = panicked; + sr.error = expectation_error(r, panicked, pointer.as_deref(), equals.as_ref()); + sr.ok = sr.error.is_none() && !panicked; + } + Step::ExpectNoError => { + sr.kind = "expect_no_error"; + match last_command_failed { + Some(false) => {} + Some(true) => { + sr.ok = false; + sr.error = Some("the previous command failed".into()); + } + None => { + sr.ok = false; + sr.error = Some("no command before this line".into()); + } + } + } + } + sr +} + +/// Run a replay script against `s`. Never panics: malformed lines are failed steps. +pub fn run(s: &mut Session, script: &str, opts: &Options) -> Report { + let _replaying = Replaying::enter(); + let mut report = Report { ok: true, ..Default::default() }; + let fail = |report: &mut Report, sr: StepReport| { + report.ok = false; + report.panicked |= sr.panicked; + if report.first_failure.is_none() { + report.first_failure = Some(sr.clone()); + } + report.steps.push(sr); + }; + if script.len() > MAX_SCRIPT_BYTES { + let msg = format!("the script is larger than {} MB", MAX_SCRIPT_BYTES >> 20); + fail(&mut report, StepReport { line: 0, kind: "invalid", command: None, ok: false, error: Some(msg), panicked: false }); + return report; + } + // Hand-edited UTF-8 scripts may start with a BOM, especially on Windows. + let script = script.strip_prefix('\u{feff}').unwrap_or(script); + let mut allow_errors = false; + let mut steps: Vec<(usize, Step)> = vec![]; + for (i, line) in script.lines().enumerate().take(MAX_LINES) { + if let Some(step) = parse_line(line, steps.is_empty(), &mut allow_errors) { + steps.push((i + 1, step)); + } + } + if script.lines().nth(MAX_LINES).is_some() { + steps.push((MAX_LINES + 1, Step::Invalid(format!("the script has more than {MAX_LINES} lines; the rest was not read")))); + } + if !matches!(steps.first(), Some((_, Step::Start(_)) | (_, Step::Invalid(_)))) { + steps.insert(0, (0, Step::Start(Start::New))); + } + let mut last_command_failed: Option = None; + let mut stopped = false; + for (line, step) in steps { + if stopped { + report.skipped += 1; + continue; + } + let sr = run_step(s, line, step, opts, allow_errors, &mut last_command_failed); + if sr.ok { + report.steps.push(sr); + } else { + fail(&mut report, sr); + stopped = !opts.keep_going; + } + } + report +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use serde_json::json; + + use super::{Options, run}; + use crate::Session; + use crate::config::{ConfigStore, MemoryConfig}; + + fn replay(script: &str) -> super::Report { + run(&mut Session::new(), script, &Options::default()) + } + + #[test] + fn steps_expectations_and_failures_are_reported() { + let r = replay( + r#"{"start": "new"} +// a comment +{"command": "comp.new", "params": {"name": "A", "width": 320, "height": 180, "frameRate": 30, "duration": 2}} +{"expect_no_error": true} +{"command": "comp.new", "params": {"frameRate": 0}, "expect_error": "frame"} +{"expect": {"query": "comp.info", "pointer": "/width", "equals": 320}} +"#, + ); + assert!(r.ok, "{r:?}"); + assert_eq!(r.steps.len(), 5); + let r = replay( + "{\"command\": \"comp.new\"}\n{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/width\", \"equals\": 1}}\n{\"command\": \"comp.new\"}", + ); + assert!(!r.ok && !r.panicked && r.skipped == 1, "{r:?}"); + assert_eq!(r.first_failure.as_ref().map(|f| f.line), Some(2)); + // An unexpected success, an unknown command. + assert!(!replay(r#"{"command": "comp.new", "expect_error": true}"#).ok); + assert!(!replay(r#"{"command": "no.such.command"}"#).ok); + // Tolerated errors. + let r = replay("{\"start\": \"new\", \"allow_errors\": true}\n{\"command\": \"no.such.command\"}\n{\"expect_no_error\": true}"); + assert!(!r.ok && r.first_failure.as_ref().map(|f| f.kind) == Some("expect_no_error"), "{r:?}"); + } + + /// A UTF-8 BOM belongs to the text file, not the first JSON step. + #[test] + fn utf8_bom_preserves_explicit_and_implicit_start_lines() { + for body in ["{\"start\":\"new\"}\n{\"command\":\"comp.new\"}", "{\"command\":\"comp.new\"}"] { + let script = format!("\u{feff}{body}"); + let report = replay(&script); + assert!(report.ok, "{report:?}"); + assert_eq!(report.steps.last().unwrap().line, body.lines().count()); + } + } + + #[test] + fn malformed_lines_are_failures_not_crashes() { + for bad in [ + "not json", + "[1, 2]", + "{\"command\": 5}", + "{\"command\": \"comp.new\", \"params\": [1]}", + "{\"command\": \"comp.new\", \"expect_error\": 3}", + "{\"expect\": {}}", + "{\"expect\": {\"query\": \"comp.info\", \"pointer\": 4}}", + "{\"expect\": {\"query\": \"comp.info\", \"pointer\": \"/nope\"}}", + "{\"start\": 7}", + "{\"start\": {\"project\": \"\"}}", + "{\"command\": \"comp.new\"}\n{\"start\": \"new\"}", + "{\"what\": 1}", + "{\"expect_no_error\": true}", + "{\"start\": {\"project\": \"/no/such/file.ecproj\"}}", + ] { + let r = replay(bad); + assert!(!r.ok && !r.panicked, "{bad}: {r:?}"); + } + let r = replay(&format!("{{\"command\": \"{}\"}}", "\u{1F600}".repeat(1000))); + assert!(!r.ok && r.first_failure.and_then(|f| f.error).is_some_and(|e| e.ends_with('…'))); + assert!(replay("").ok, "an empty script is a new project"); + } + + #[test] + fn the_journal_exports_and_replays() { + let mut s = Session::new(); + s.execute("comp.new", json!({"name": "Old"})).unwrap(); + s.execute("file.openDemoProject", json!({})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#ff0000", "name": "Red"})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#00ff00", "name": "Green"})).unwrap(); + // A query isn't journaled; neither is saving. + s.execute("comp.info", json!({})).unwrap(); + let text = super::export(&s, None); + let lines: Vec<&str> = text.lines().collect(); + assert_eq!(lines.len(), 4, "{text}"); + assert!(lines[0].contains("\"start\":\"new\"") && lines[1].contains("file.openDemoProject"), "starts at the last project open: {text}"); + let mut again = Session::new(); + let r = run(&mut again, &text, &Options::default()); + assert!(r.ok, "{r:?}"); + let layers = |s: &Session| s.active_comp().map(|c| c.layers.len()); + assert_eq!(layers(&again), layers(&s)); + // journal.export returns it; with `path` it writes the file. + let v = s.execute("journal.export", json!({})).unwrap(); + assert_eq!(v["steps"], 3); + assert_eq!(v["script"].as_str().unwrap().lines().skip(1).collect::>(), lines[1..]); + let dir = std::env::temp_dir().join(format!("ec-replay-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("j.jsonl"); + s.execute("journal.export", json!({"path": path.to_str().unwrap()})).unwrap(); + assert!(std::fs::read_to_string(&path).unwrap().contains("layer.newSolid")); + let _ = std::fs::remove_dir_all(&dir); + assert_eq!(s.journal.iter().filter(|(id, _)| id.starts_with("journal.")).count(), 0); + } + + #[test] + fn help_export_repro_saves_into_the_settings_store() { + let mut s = Session::new(); + assert!(s.execute("help.exportRepro", json!({})).is_err(), "no settings folder and no path"); + let store = Arc::new(MemoryConfig::default()); + s.config = Some(store.clone()); + s.execute("comp.new", json!({})).unwrap(); + let v = s.execute("help.exportRepro", json!({})).unwrap(); + assert_eq!(v["steps"], 1); + let saved = store.list(super::REPRO_DIR); + assert_eq!(saved.len(), 1, "{saved:?}"); + assert!(store.read(v["path"].as_str().unwrap()).unwrap().contains("comp.new")); + } + + #[test] + fn a_panicking_command_saves_a_crash_repro_and_replays_as_a_failure() { + let mut s = Session::new(); + let store = Arc::new(MemoryConfig::default()); + s.config = Some(store.clone()); + s.execute("comp.new", json!({"name": "C"})).unwrap(); + // `test.panic` stands in for a command with a bug (see `commands::journal`). + let e = s.execute("test.panic", json!({"why": "boom"})).unwrap_err().to_string(); + assert!(e.contains("boom") && e.contains("Repros/crash-"), "{e}"); + let names = store.list(super::REPRO_DIR); + assert_eq!(names.len(), 1); + let text = store.read(&format!("{}/{}", super::REPRO_DIR, names[0])).unwrap(); + assert!(text.lines().next().unwrap().contains("\"crash\""), "{text}"); + assert!(text.lines().last().unwrap().contains("test.panic"), "{text}"); + // The replay fails on the panic (and doesn't save another repro). + let r = run(&mut Session::new(), &text, &Options::default()); + assert!(!r.ok && r.panicked, "{r:?}"); + assert_eq!(r.first_failure.as_ref().and_then(|f| f.command.as_deref()), Some("test.panic")); + // A failing replay inside a session with a store saves nothing more. + let mut t = Session::new(); + t.config = Some(store.clone()); + assert!(!run(&mut t, &text, &Options::default()).ok); + assert_eq!(store.list(super::REPRO_DIR).len(), 1); + // Only the newest crash repros are kept. + for i in 0..30 { + store.write(&format!("{}/crash-{i:012}-x.jsonl", super::REPRO_DIR), "{}").unwrap(); + } + let _ = s.execute("test.panic", json!({})); + assert_eq!(store.list(super::REPRO_DIR).len(), super::KEEP_CRASH_REPROS); + } + + #[test] + fn commands_run_by_a_journaled_command_are_not_journaled_twice() { + let mut s = Session::new(); + s.execute("comp.new", json!({})).unwrap(); + // engine.batch isn't journaled: its steps are. + s.execute("engine.batch", json!({"steps": [{"command": "layer.newNull"}, {"command": "layer.newNull"}]})).unwrap(); + assert_eq!(s.journal.iter().filter(|(id, _)| id == "layer.newNull").count(), 2); + // A journaled command's own steps replay with it. + s.execute("file.runScript", json!({"steps": [{"command": "layer.newNull"}]})).unwrap(); + let ids: Vec<&str> = s.journal.iter().map(|(id, _)| id.as_str()).collect(); + assert_eq!(ids, ["comp.new", "layer.newNull", "layer.newNull", "file.runScript"]); + } +} + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod revert_tests { + use super::{Options, export, run}; + use crate::Session; + use serde_json::json; + + /// Saving is omitted, so a successful Revert must keep the file it read as an open boundary. + #[test] + fn save_as_then_revert_exports_a_replayable_open() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("saved.ecproj"); + let mut s = Session::new(); + s.execute("comp.new", json!({"name": "Saved"})).unwrap(); + s.execute("file.saveAs", json!({"path": path})).unwrap(); + s.execute("layer.newNull", json!({})).unwrap(); + s.execute("file.revert", json!({})).unwrap(); + s.execute("layer.newNull", json!({"name": "After Revert"})).unwrap(); + let script = export(&s, None); + assert!(!script.contains("file.save") && !script.contains("file.revert"), "{script}"); + assert!(script.lines().nth(1).unwrap().contains("file.open"), "{script}"); + let saved = std::fs::read(&path).unwrap(); + let mut again = Session::new(); + let report = run(&mut again, &script, &Options::default()); + assert!(report.ok, "{report:?}"); + assert_eq!(again.path, s.path); + assert_eq!(again.active_comp().unwrap().layers.len(), s.active_comp().unwrap().layers.len()); + assert_eq!(saved, std::fs::read(path).unwrap(), "replay must not rewrite the saved project"); + } +} diff --git a/crates/engine/src/session/dispatch.rs b/crates/engine/src/session/dispatch.rs index aa83ac09f..a3f3f05d5 100644 --- a/crates/engine/src/session/dispatch.rs +++ b/crates/engine/src/session/dispatch.rs @@ -22,10 +22,20 @@ impl Session { } } // Last-resort guard: a panicking command reports an error instead of crashing the app. - let r = guard::guarded(&format!("command `{id}`"), || (spec.run)(self, ¶ms))?; + let nesting = crate::replay::Nesting::enter(spec.journal); + let caught = guard::caught(); + let r = guard::guarded(&format!("command `{id}`"), || (spec.run)(self, ¶ms)); + let r = crate::replay::after_command(self, id, ¶ms, &nesting, caught, r)?; self.learn_observe(id, ¶ms); - if spec.journal { - self.journal.push((id.to_string(), params)); + if spec.journal && !nesting.inside_journaled() { + // Revert and Open Recent resolve a file the parameters may not name. Store + // the successful read as an explicit open boundary while its resolved path is known. + let entry = if matches!(id, "file.revert" | "file.openRecent") { + self.path.as_ref().map(|path| ("file.open".to_string(), serde_json::json!({"path": path}))) + } else { + None + }; + self.journal.push(entry.unwrap_or_else(|| (id.to_string(), params))); if self.journal.len() > 10_000 { self.journal.drain(..1000); } diff --git a/crates/engine/src/system/guard.rs b/crates/engine/src/system/guard.rs index 699e6474d..ec32e2545 100644 --- a/crates/engine/src/system/guard.rs +++ b/crates/engine/src/system/guard.rs @@ -4,14 +4,28 @@ //! panics leaves the project as it was. This is a safety net, not a substitute for returning //! errors (AGENTS.md, "Never crash"). +use std::cell::Cell; + use crate::{EngineError, Result}; +thread_local! { + /// Panics [`guarded`] caught on this thread (see [`caught`]). + static CAUGHT: Cell = const { Cell::new(0) }; +} + +/// How many panics [`guarded`] has caught on this thread so far: compare two readings to learn +/// whether a call panicked (crash repros, `effectcraft-cli replay`). +pub fn caught() -> u64 { + CAUGHT.with(Cell::get) +} + /// Run `f`; a panic inside it becomes an error naming `what`. The panic itself is logged by /// the panic hook ([`crate::logging::install_panic_hook`]). pub fn guarded(what: &str, f: impl FnOnce() -> Result) -> Result { match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) { Ok(r) => r, Err(payload) => { + CAUGHT.with(|c| c.set(c.get().saturating_add(1))); let msg = payload .downcast_ref::<&str>() .map(|s| s.to_string()) diff --git a/crates/gpu/src/diagnostics.rs b/crates/gpu/src/diagnostics.rs new file mode 100644 index 000000000..a37adff22 --- /dev/null +++ b/crates/gpu/src/diagnostics.rs @@ -0,0 +1,102 @@ +//! Per-compositor, fixed-category observations from error scopes. No driver strings escape. + +#[cfg(any(test, not(target_arch = "wasm32")))] +use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::{Arc, OnceLock}; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ScopedError { + OutOfMemory, + Internal, + Validation, +} + +pub type ScopedErrorObserver = Arc; + +#[derive(Default)] +pub(crate) struct ScopedObservations { + observer: OnceLock, + #[cfg(any(test, not(target_arch = "wasm32")))] + seen: AtomicU8, +} + +impl ScopedObservations { + /// Register before rendering. Shared by all clones of the compositor, never global. + pub(crate) fn install(&self, observer: ScopedErrorObserver) -> bool { + self.observer.set(observer).is_ok() + } + + #[cfg(any(test, not(target_arch = "wasm32")))] + pub(crate) fn report_scopes(&self, oom: bool, internal: bool, validation: bool) { + for (present, category) in [(oom, ScopedError::OutOfMemory), (internal, ScopedError::Internal), (validation, ScopedError::Validation)] { + if present { + self.report(category); + } + } + } + + #[cfg(any(test, not(target_arch = "wasm32")))] + fn report(&self, error: ScopedError) { + let Some(observer) = self.observer.get() else { return }; + let bit = match error { + ScopedError::OutOfMemory => 1, + ScopedError::Internal => 2, + ScopedError::Validation => 4, + }; + if self.seen.fetch_or(bit, Ordering::Relaxed) & bit != 0 { + return; + } + // Diagnostics must not turn a recoverable GPU frame error into an application panic. + #[cfg(not(target_arch = "wasm32"))] + if std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| observer(error))).is_err() { + log::warn!("GPU scoped-error observer panicked"); + } + #[cfg(target_arch = "wasm32")] + observer(error); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn categories_are_independent_bounded_and_not_global() { + let observations = ScopedObservations::default(); + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + let copy = seen.clone(); + assert!(observations.install(Arc::new(move |error| copy.lock().unwrap().push(error)))); + assert!(!observations.install(Arc::new(|_| panic!("must not replace observer")))); + for _ in 0..100 { + observations.report(ScopedError::Internal); + observations.report(ScopedError::Validation); + observations.report(ScopedError::OutOfMemory); + } + ScopedObservations::default().report(ScopedError::OutOfMemory); + assert_eq!(*seen.lock().unwrap(), [ScopedError::Internal, ScopedError::Validation, ScopedError::OutOfMemory]); + } + + #[test] + fn internal_and_validation_scopes_do_not_claim_out_of_memory() { + let observations = ScopedObservations::default(); + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + let copy = seen.clone(); + observations.install(Arc::new(move |error| copy.lock().unwrap().push(error))); + observations.report_scopes(false, false, false); + assert!(seen.lock().unwrap().is_empty()); + observations.report_scopes(false, true, true); + assert_eq!(*seen.lock().unwrap(), [ScopedError::Internal, ScopedError::Validation]); + observations.report_scopes(true, true, true); + assert_eq!(*seen.lock().unwrap(), [ScopedError::Internal, ScopedError::Validation, ScopedError::OutOfMemory]); + } + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn observer_panic_does_not_escape_frame_recovery() { + let observations = ScopedObservations::default(); + observations.install(Arc::new(|_| panic!("synthetic observer panic"))); + observations.report(ScopedError::OutOfMemory); + observations.report(ScopedError::Internal); + assert_eq!(observations.seen.load(Ordering::Relaxed), 3); + } +} diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index ddbc8d54a..86a498c6e 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -58,6 +58,7 @@ mod bokeh; mod classic3d; mod context; pub mod deferred; +mod diagnostics; mod effects; mod fx_color; mod fx_depth; @@ -91,6 +92,7 @@ mod yuv; use std::sync::Arc; pub use context::{GpuContext, GpuImage, TransferStats}; +pub use diagnostics::{ScopedError, ScopedErrorObserver}; use effectcraft_effects::Buf; use effectcraft_project::ItemId; use effectcraft_raster::Image; @@ -126,6 +128,7 @@ impl std::error::Error for FrameError {} /// The GPU compositor (cheap to clone; one device shared by all clones). #[derive(Clone)] pub struct Gpu { + scoped_observations: Arc, ctx: Arc, /// Backend::Auto's per-comp CPU / GPU timings. auto: Arc, @@ -162,7 +165,7 @@ impl Gpu { } pub fn from_context(ctx: GpuContext) -> Gpu { - Gpu { ctx: Arc::new(ctx), auto: Default::default(), declined: Default::default() } + Gpu { ctx: Arc::new(ctx), auto: Default::default(), declined: Default::default(), scoped_observations: Default::default() } } /// A device of its own without blocking, with deferred readbacks (a browser worker, see @@ -173,6 +176,13 @@ impl Gpu { Ok(Gpu::from_context(ctx)) } + /// Install a host's fixed-category observer before rendering. It is called at most once + /// per category across all clones, outside device/renderer locks. Returns false if already + /// registered. The observer must remain brief and must not perform GPU work. + pub fn set_scoped_error_observer(&self, observer: ScopedErrorObserver) -> bool { + self.scoped_observations.install(observer) + } + /// [`Gpu::request_deferred`] blocking (tests of the browser worker's path, natively). #[cfg(not(target_arch = "wasm32"))] pub fn headless_deferred() -> Option { @@ -316,6 +326,9 @@ impl Gpu { let validation = pollster::block_on(validation.pop()); let internal = pollster::block_on(internal.pop()); let oom = pollster::block_on(oom.pop()); + // Preserve the typed scope observations before FrameError::Memory combines OOM + // and internal errors. An internal error alone is not evidence of actual OOM. + self.scoped_observations.report_scopes(oom.is_some(), internal.is_some(), validation.is_some()); let memory = oom.is_some() || internal.is_some(); // The allocation failure first: a validation error is often its consequence. let errors: Vec = [oom, internal, validation].into_iter().flatten().map(|e| e.to_string()).collect(); diff --git a/crates/host/tests/it/main.rs b/crates/host/tests/it/main.rs index 168ec127a..ecc1abb0f 100644 --- a/crates/host/tests/it/main.rs +++ b/crates/host/tests/it/main.rs @@ -15,3 +15,5 @@ mod timeline_interop; mod vector_import; mod wav_markers; mod webm_metadata; + +mod repros; diff --git a/crates/host/tests/it/repros.rs b/crates/host/tests/it/repros.rs new file mode 100644 index 000000000..1b343603c --- /dev/null +++ b/crates/host/tests/it/repros.rs @@ -0,0 +1,31 @@ +//! Regression tests as data: every `tests/repros/*.jsonl` (repository root) is a replay script +//! (`effectcraft_engine::replay`, docs/agents.md "Repro scripts") run against a fully wired +//! headless session, like `effectcraft-cli replay`. A crash fix's regression test is a `.jsonl` +//! dropped there: it must replay without a panic and meet its expectations. + +use std::path::{Path, PathBuf}; + +use effectcraft_engine::replay; + +fn repros_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../tests/repros") +} + +#[test] +fn every_repro_replays_cleanly() { + let mut files: Vec = + std::fs::read_dir(repros_dir()).unwrap().map(|e| e.unwrap().path()).filter(|p| p.extension().is_some_and(|x| x == "jsonl")).collect(); + files.sort(); + assert!(!files.is_empty(), "no repros in {}", repros_dir().display()); + let mut failures = vec![]; + for f in &files { + let script = std::fs::read_to_string(f).unwrap(); + let mut s = effectcraft_host::session(); + let opts = replay::Options { keep_going: false, base_dir: f.parent().map(Path::to_path_buf) }; + let r = replay::run(&mut s, &script, &opts); + if !r.ok { + failures.push(format!("{}: {}", f.display(), serde_json::to_string(&r.first_failure).unwrap())); + } + } + assert!(failures.is_empty(), "{} of {} repros failed:\n{}", failures.len(), files.len(), failures.join("\n")); +} diff --git a/crates/ui-egui/src/app/gpu.rs b/crates/ui-egui/src/app/gpu.rs index c9849e921..92ea7665c 100644 --- a/crates/ui-egui/src/app/gpu.rs +++ b/crates/ui-egui/src/app/gpu.rs @@ -44,6 +44,9 @@ impl EffectcraftApp { pub fn set_gpu_failure_bridge(&mut self, bridge: gpu_failure::GpuFailureBridge) { if let Some(gpu) = &self.gpu { bridge.forward_to(gpu.context().device_failure_notifier()); + if let Some(observer) = bridge.scoped_error_observer() { + gpu.set_scoped_error_observer(observer); + } } self.gpu_failures = Some(bridge); } @@ -257,6 +260,9 @@ impl EffectcraftApp { Ok(Ok(g)) => { if let Some(bridge) = &self.gpu_failures { bridge.forward_to(g.context().device_failure_notifier()); + if let Some(observer) = bridge.scoped_error_observer() { + g.set_scoped_error_observer(observer); + } if bridge.failure().is_some() { return; } diff --git a/crates/ui-egui/src/app/playback.rs b/crates/ui-egui/src/app/playback.rs index d40bc69ff..b68e58f27 100644 --- a/crates/ui-egui/src/app/playback.rs +++ b/crates/ui-egui/src/app/playback.rs @@ -40,6 +40,7 @@ impl EffectcraftApp { restore_max, audio_frame: (!plan.video).then_some(plan.first), audio_held: false, + silent_project: None, starting: false, starting_since: now, first_cached: None, @@ -56,12 +57,10 @@ impl EffectcraftApp { } } - /// The preview includes audio at the comp's frame rate and the comp has something audible. - fn audio_plays(&self) -> bool { + /// The preview includes audio and the comp has something audible. + pub(super) fn audio_plays(&self) -> bool { let (Some(pl), Some(cid)) = (self.playback.plan, self.session.active_comp_id()) else { return false }; - let Some(c) = self.session.project.comp(cid) else { return false }; - // A preview at another frame rate plays slower or faster than real time: silent. - pl.audio && (pl.fps - c.frame_rate.as_f64()).abs() <= 0.01 && effectcraft_engine::render::audio::comp_has_audio(&self.session.project, cid) + pl.audio && effectcraft_engine::render::audio::comp_has_audio(&self.session.project, cid) } /// Start audio preview from comp time `t` when [`Self::audio_plays`] and an output device @@ -75,7 +74,17 @@ impl EffectcraftApp { let Some(dev) = self.hooks.audio_device.as_ref().and_then(|f| f(&out)) else { return }; let mix = self.session.prefs.audio.preview_sample_rate; let fr = c.frame_rate; - match audio::AudioPlayback::start(dev, self.render_source(), cid, t, fr.tick_of(pl.start), fr.tick_of(pl.end + 1), pl.looping, mix) { + match audio::AudioPlayback::start_at_speed( + dev, + self.render_source(), + cid, + t, + fr.tick_of(pl.start), + fr.tick_of(pl.end + 1), + pl.looping, + mix, + pl.fps / fr.as_f64(), + ) { Ok(a) => self.audio = Some(a), Err(e) => log::warn!("audio preview: {e}"), } diff --git a/crates/ui-egui/src/app/prefetch.rs b/crates/ui-egui/src/app/prefetch.rs index 50638b708..4287ea8f6 100644 --- a/crates/ui-egui/src/app/prefetch.rs +++ b/crates/ui-egui/src/app/prefetch.rs @@ -1,4 +1,4 @@ -use super::{EffectcraftApp, FrameKey, FrameWindow, ItemId, PlaybackPass, audio, frames}; +use super::{EffectcraftApp, FrameKey, FrameWindow, ItemId, PlaybackPass, frames}; /// Seconds without a project change before paused prefetch resumes (see /// [`EffectcraftApp::editing`]). @@ -82,6 +82,7 @@ impl EffectcraftApp { let step = plan.map_or(1, |p| p.step); let fit_ahead = self.frames_that_fit(&c, scale) - 1; self.prefetch_playback(&PlaybackPass { cur: lead, ..pass }, fit_ahead); + self.wake_silent_preview(); self.pump_meters(ctx, now); if !self.playback.playing { self.cache_when_idle(ctx, &c, &series, now, input_active, (wa, wb)); @@ -201,6 +202,10 @@ impl EffectcraftApp { /// one second. With no source but an active meter, drive the meter to silence and request a repaint. fn pump_meters(&mut self, ctx: &egui::Context, now: f64) { if let Some(a) = &mut self.audio { + // An edit while playing (mute, unmute, solo, levels): the sound follows it (#2104). + if a.project().is_some_and(|p| !std::sync::Arc::ptr_eq(&p, &self.session.project)) { + a.follow_project(self.session.project.clone()); + } a.pump(); self.meter.update(a.feed.take_peaks(), now); } else if let Some(s) = &mut self.scrub { @@ -259,6 +264,29 @@ impl EffectcraftApp { false } + /// A silent preview whose project was edited so that it has sound now (a layer unmuted while + /// playing, #2104): video previews wait for cached frames; audio-only previews start from + /// their moving audio frame, not the stationary timeline. Checked once per project edit. + fn wake_silent_preview(&mut self) { + let pb = &self.playback; + if self.audio.is_some() || !pb.playing || pb.caching || pb.starting || pb.audio_held { + return; + } + if pb.silent_project.as_ref().is_some_and(|p| std::sync::Arc::ptr_eq(p, &self.session.project)) { + return; + } + self.playback.silent_project = Some(self.session.project.clone()); + if !self.audio_plays() { + return; + } + if self.playback.plan.is_some_and(|p| p.video) { + self.playback.audio_held = true; + } else if let Some(c) = self.session.active_comp() { + let frame = self.playback.audio_frame.unwrap_or(self.playback.start_frame); + self.start_audio(c.frame_rate.tick_of(frame)); + } + } + /// Held sound starts once the frames ahead are cached (see `Playback::audio_held`): the /// prefetch's `ahead` when the silent preview keeps real time, else the rest of the preview. fn start_held_audio(&mut self, pass: &PlaybackPass<'_>, fit: i64) { @@ -293,7 +321,7 @@ impl EffectcraftApp { return true; } let snap = |f: i64| wa + (f - wa).div_euclid(step) * step; - let target = snap(audio::frame_of_sample(a.clock(), a.rate, fr).clamp(wa, wb)); + let target = snap(fr.frame_at(a.clock_time()).clamp(wa, wb)); if !video { self.playback.audio_frame = Some(target); } else if self.frames.is_cached(&FrameKey { frame: target, ..series }) { @@ -322,7 +350,7 @@ impl EffectcraftApp { fn sound_frame(&self, fr: effectcraft_engine::time::FrameRate, wa: i64, wb: i64, step: i64) -> Option { let a = self.audio.as_ref()?; let step = step.max(1); - Some(wa + (audio::frame_of_sample(a.clock(), a.rate, fr).clamp(wa, wb) - wa).div_euclid(step) * step) + Some(wa + (fr.frame_at(a.clock_time()).clamp(wa, wb) - wa).div_euclid(step) * step) } /// The last cached frame after the current one and before `target`, in play order: what a diff --git a/crates/ui-egui/src/audio.rs b/crates/ui-egui/src/audio.rs index fbfe4b8af..e0d6b4d1b 100644 --- a/crates/ui-egui/src/audio.rs +++ b/crates/ui-egui/src/audio.rs @@ -10,7 +10,7 @@ //! **A/V sync:** while audio plays, the audio clock drives playback. The clock is the number of //! sample frames the device has actually consumed (underruns do not advance it) minus the //! device's output latency, mapped through the work-area loop ([`clock_sample`]); the viewer -//! shows the comp frame at that sample ([`frame_of_sample`]), dropping video frames rather +//! shows the comp frame at that time ([`AudioPlayback::clock_time`]), dropping video frames rather //! than drifting. //! //! The feed also tracks per-channel peaks for the Audio panel's VU meters ([`Meter`]). @@ -18,16 +18,19 @@ //! **Scrubbing:** Ctrl/Cmd-dragging the current-time indicator plays a short snippet of the mix //! at each new frame ([`AudioScrub`], `playback.scrubAudio`), as in After Effects. -use std::collections::VecDeque; +use std::collections::{HashSet, VecDeque}; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::JoinHandle; -use effectcraft_engine::project::ItemId; +use effectcraft_engine::project::{GroupKind, ItemId, ItemKind, Layer, LayerSource, Project, PropGroup}; use effectcraft_engine::time::{FrameRate, Tick}; use crate::frames::RenderSource; +mod preview_rate; +use preview_rate::PreviewRate; + /// An audio output (implemented by the native app with cpal). pub trait AudioDevice: Send { /// Device sample rate in Hz. @@ -88,9 +91,11 @@ impl AudioFeed { pk[i & 1] = pk[i & 1].max(s.abs()); } real = n; + // Queue removal and the clock advance are one handoff: an edit must not restart + // behind samples already handed to the device. + self.consumed.fetch_add((real / 2) as u64, Ordering::Relaxed); } out[real..].iter_mut().for_each(|o| *o = 0.0); - self.consumed.fetch_add((real / 2) as u64, Ordering::Relaxed); for (c, v) in pk.into_iter().enumerate() { // Rust 1.99 renamed `fetch_update` to `try_update`; keep the old name while the // workspace supports Rust 1.95 (`rust-version`). @@ -106,6 +111,14 @@ impl AudioFeed { } } + /// Discard an old mix and snapshot the first unconsumed frame under the same queue lock + /// as [`Self::pull`], so a concurrent device callback cannot leave the cursor behind. + fn discard_pending(&self) -> Option { + let mut q = self.queue.lock().ok()?; + q.clear(); + Some(self.consumed()) + } + pub fn push(&self, samples: &[f32]) { if let Ok(mut q) = self.queue.lock() { q.extend(samples.iter().copied()); @@ -131,7 +144,7 @@ impl AudioFeed { } } -/// What a preview plays, in comp sample indices at the device rate. +/// What a preview plays, in output sample coordinates on its playback timeline. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct PlaySpan { pub start: i64, @@ -149,15 +162,23 @@ impl PlaySpan { } } -/// The comp sample index audible now: `consumed` frames played by the device minus its +/// The output sample coordinate audible now: `consumed` frames played by the device minus its /// `latency`, from the span start, wrapped into the work area when looping (clamped at its end /// otherwise). pub fn clock_sample(span: &PlaySpan, consumed: u64, latency: u64) -> i64 { - let s = span.start + consumed.saturating_sub(latency) as i64; - if s < span.wb { - return s; + // Slow previews can span more output samples than fit in a signed difference. Keep the + // elapsed count and loop width wide until the result is back inside the span. + let s = i128::from(span.start) + i128::from(consumed.saturating_sub(latency)); + if s < i128::from(span.wb) { + return s as i64; + } + if span.looping { + let width = (i128::from(span.wb) - i128::from(span.wa)).max(1); + let wrapped = i128::from(span.wa) + (s - i128::from(span.wb)).rem_euclid(width); + wrapped.clamp(i64::MIN as i128, i64::MAX as i128) as i64 + } else { + span.wb.saturating_sub(1) } - if span.looping { span.wa + (s - span.wb).rem_euclid((span.wb - span.wa).max(1)) } else { span.wb - 1 } } /// Comp time of sample index `s` at `rate` Hz. @@ -179,10 +200,23 @@ pub struct AudioPlayback { device: Box, stop: Arc, thread: Option>, - /// wasm32 (no threads): what the feeder thread would own, run by [`AudioPlayback::pump`]. - feeder: Option<(RenderSource, ItemId, i64, u32)>, + /// What the mixer reads and advances: the feeder thread, or [`AudioPlayback::pump`] on wasm32 + /// (no threads). + state: Arc>, + comp: ItemId, + mix: u32, pub rate: u32, pub span: PlaySpan, + timeline: PreviewRate, +} + +/// The mixer's position and its view of the project. +struct Feeder { + src: RenderSource, + /// The next output sample index to mix (mapped to composition time by the preview rate). + cursor: i64, + /// Bumped when the project changes: a block mixed from an older project is dropped. + generation: u64, } /// Audio kept queued ahead of the device: ~200 ms (a feeder thread tops it up continuously); @@ -197,7 +231,7 @@ impl AudioPlayback { /// at (resampled to the device's rate when they differ; 0 = the device's rate). #[allow(clippy::too_many_arguments)] pub fn start( - mut device: Box, + device: Box, src: RenderSource, comp: ItemId, start: Tick, @@ -205,15 +239,34 @@ impl AudioPlayback { wb: Tick, looping: bool, mix_rate: u32, + ) -> Result { + Self::start_at_speed(device, src, comp, start, wa, wb, looping, mix_rate, 1.0) + } + + /// Start at a preview speed relative to composition time. Both the sound (including pitch) + /// and its composition clock follow this speed; the physical device rate stays unchanged. + #[allow(clippy::too_many_arguments)] + pub fn start_at_speed( + mut device: Box, + src: RenderSource, + comp: ItemId, + start: Tick, + wa: Tick, + wb: Tick, + looping: bool, + mix_rate: u32, + speed: f64, ) -> Result { let rate = device.sample_rate().max(8000); let mix = if mix_rate == 0 { rate } else { mix_rate.clamp(8000, 192_000) }; - let span = PlaySpan::new(start, wa, wb, looping, rate); + let timeline = PreviewRate::new(rate, speed, start)?; + let span = timeline.span(start, wa, wb, looping); let feed = Arc::new(AudioFeed::default()); let stop = Arc::new(AtomicBool::new(false)); + let state = Arc::new(Mutex::new(Feeder { src, cursor: span.start, generation: 0 })); if cfg!(target_arch = "wasm32") { // The device takes samples only in `pump`, which fills the queue first. - let mut p = AudioPlayback { feed: feed.clone(), device, stop, thread: None, feeder: Some((src, comp, span.start, mix)), rate, span }; + let mut p = AudioPlayback { feed: feed.clone(), device, stop, thread: None, state, comp, mix, rate, span, timeline }; p.device.start(feed)?; p.pump(); return Ok(p); @@ -223,15 +276,14 @@ impl AudioPlayback { // as that took (#408). The device plays silence until the first block arrives, and the // clock doesn't count it. let thread = { - let (feed, stop) = (feed.clone(), stop.clone()); + let (feed, stop, state) = (feed.clone(), stop.clone(), state.clone()); std::thread::Builder::new() .name("ec-audio-feed".into()) .spawn(move || { - let mut cursor = span.start; let target = queue_target(rate); while !stop.load(Ordering::Relaxed) { if feed.queued_frames() < target && !feed.finished.load(Ordering::Relaxed) { - cursor = feed_block(&src, comp, &feed, &span, cursor, rate, mix); + feed_block(&state, comp, &feed, &span, timeline, mix); } else { std::thread::sleep(std::time::Duration::from_millis(4)); } @@ -240,24 +292,63 @@ impl AudioPlayback { .map_err(|e| e.to_string())? }; device.start(feed.clone())?; - Ok(AudioPlayback { feed, device, stop, thread: Some(thread), feeder: None, rate, span }) + Ok(AudioPlayback { feed, device, stop, thread: Some(thread), state, comp, mix, rate, span, timeline }) } /// Every UI frame while playing: on wasm32 mix ahead into the feed (what the feeder thread /// does elsewhere), then let the device take what it needs. pub fn pump(&mut self) { - if let Some((src, comp, cursor, mix)) = &mut self.feeder { + if cfg!(target_arch = "wasm32") { let target = queue_target(self.rate); while self.feed.queued_frames() < target && !self.feed.finished.load(Ordering::Relaxed) { - *cursor = feed_block(src, *comp, &self.feed, &self.span, *cursor, self.rate, *mix); + feed_block(&self.state, self.comp, &self.feed, &self.span, self.timeline, self.mix); } } self.device.pump(); } - /// The comp sample index audible now. + /// The project the sound is mixed from. + pub fn project(&self) -> Option> { + self.state.lock().ok().map(|s| s.src.project.clone()) + } + + /// Mix from `project` from now on (the project was edited while playing, e.g. a layer's Audio + /// switch, #2104): what is queued came from the old project, so it is dropped and mixing + /// resumes at the first sample the device has not taken yet (also when the feeder had + /// already mixed up to the end of a non-looping span). + pub fn set_project(&mut self, project: Arc) { + let Ok(mut s) = self.state.lock() else { return }; + let Some(consumed) = self.feed.discard_pending() else { return }; + s.src.project = project; + s.generation = s.generation.wrapping_add(1); + self.feed.finished.store(false, Ordering::Relaxed); + // The display clock clamps to the last *audible* sample. A finished non-looping + // feeder must instead stay at the exclusive end, never replay that last sample. + let next = i128::from(self.span.start) + i128::from(consumed); + s.cursor = if !self.span.looping && next >= i128::from(self.span.wb) { self.span.wb } else { clock_sample(&self.span, consumed, 0) }; + } + + /// Follow an edited `project`: mixing continues from it, and the queued sound is dropped only + /// when the edit changed what is heard ([`audio_changed`]); a transform or colour edit leaves + /// the queue alone, so playback does not drop out. + pub fn follow_project(&mut self, project: Arc) { + let Some(old) = self.project() else { return }; + if audio_changed(&old, &project, self.comp) { + self.set_project(project); + } else if let Ok(mut s) = self.state.lock() { + s.src.project = project; + } + } + + /// The comp sample index audible now, expressed at the device's rate. pub fn clock(&self) -> i64 { - clock_sample(&self.span, self.feed.consumed(), self.device.latency_frames()) + let sample = clock_sample(&self.span, self.feed.consumed(), self.device.latency_frames()); + if self.timeline.normal() { sample } else { self.timeline.time(sample).to_units_floor(i64::from(self.rate)) } + } + + /// Composition time under the audible output sample, including preview speed and latency. + pub fn clock_time(&self) -> Tick { + self.timeline.time(clock_sample(&self.span, self.feed.consumed(), self.device.latency_frames())) } pub fn finished(&self) -> bool { @@ -279,20 +370,101 @@ impl Drop for AudioPlayback { } } -/// Mix one block at `cursor` into the feed; returns the next cursor (wrapping or finishing at -/// the work-area end). -fn feed_block(src: &RenderSource, comp: ItemId, feed: &AudioFeed, span: &PlaySpan, cursor: i64, rate: u32, mix: u32) -> i64 { - let mut cursor = cursor; - if cursor >= span.wb { - if !span.looping { - feed.finished.store(true, Ordering::Relaxed); - return cursor; +/// Whether `new` sounds different from `old` for `comp`: a layer's timing, source, Audio / Solo / +/// Effects switch, Audio Levels or effects, or the same for a nested comp. Other edits (video +/// properties, names, labels) do not change the mix unless an audio expression can read them. +fn audio_changed(old: &Project, new: &Project, comp: ItemId) -> bool { + audio_changed_inner(old, new, comp, &mut HashSet::new()) +} + +/// Each shared precomp is compared once per depth. A depth limit alone still visits an +/// exponentially large number of paths in a small graph of reused precomps, blocking the UI. +fn audio_changed_inner(old: &Project, new: &Project, comp: ItemId, seen: &mut HashSet<(ItemId, usize)>) -> bool { + /// Compare one comp and its reachable audio sources, without revisiting a shared path depth. + fn comps_differ(old: &Project, new: &Project, comp: ItemId, depth: usize, seen: &mut HashSet<(ItemId, usize)>) -> bool { + if depth >= 16 || !seen.insert((comp, depth)) { + return false; } + let (Some(a), Some(b)) = (old.comp(comp), new.comp(comp)) else { return old.comp(comp).is_some() != new.comp(comp).is_some() }; + a.layers.len() != b.layers.len() + || a.layers.iter().zip(&b.layers).any(|(x, y)| { + x.id != y.id + || x.source != y.source + || x.start_time != y.start_time + || x.in_point != y.in_point + || x.out_point != y.out_point + || x.stretch != y.stretch + || x.switches.audio != y.switches.audio + || x.switches.solo != y.switches.solo + || x.switches.effects != y.switches.effects + || x.props.sub("audio") != y.props.sub("audio") + || audio_effects(x) != audio_effects(y) + || audio_has_expression(x) + || audio_has_expression(y) + || match &x.source { + LayerSource::Footage { item } => footage_differs(old, new, *item), + LayerSource::Comp { item } => comps_differ(old, new, *item, depth + 1, seen), + _ => false, + } + }) + } + /// The layer's audio effects (Effect > Audio), the only effects the mixer applies; a video effect + /// edit leaves them as they were. + fn audio_effects(l: &Layer) -> Vec<&PropGroup> { + let is_audio = |g: &&PropGroup| matches!(&g.kind, GroupKind::Effect { effect } if effectcraft_engine::effects::audio_fx::is_audio_effect(effect)); + l.effects().into_iter().flat_map(|fx| fx.groups()).filter(is_audio).collect() + } + /// Audio expressions can read any property in any comp (including video-only effects and + /// names). Without a complete dependency graph, keeping the old mix would be stale. + fn audio_has_expression(l: &Layer) -> bool { + l.switches.audio + && (l.props.sub("audio").and_then(|g| g.get("levels")).is_some_and(|p| p.has_expression()) + || (l.switches.effects && audio_effects(l).iter().any(|g| g.enabled && g.props().any(|p| p.has_expression())))) + } + /// The footage item's audio availability changed (relinked, missing, no audio). + fn footage_differs(old: &Project, new: &Project, item: ItemId) -> bool { + let f = |p: &Project| match p.item(item).map(|i| &i.kind) { + Some(ItemKind::Footage(f)) => Some((f.has_audio, f.missing, f.path.clone())), + _ => None, + }; + f(old) != f(new) + } + comps_differ(old, new, comp, 0, seen) +} + +/// Mix one block at the feeder's cursor into the feed and move the cursor on (wrapping or +/// finishing at the work-area end). The block is mixed without holding the lock, so a project +/// change never waits for a slow mix; a block mixed from the old project is dropped. +fn feed_block(state: &Mutex, comp: ItemId, feed: &AudioFeed, span: &PlaySpan, timeline: PreviewRate, mix: u32) { + let Ok((src, mut cursor, generation)) = state.lock().map(|s| (s.src.clone(), s.cursor, s.generation)) else { + // Poisoned: nothing more can be mixed. + feed.finished.store(true, Ordering::Relaxed); + return; + }; + let mut block = None; + if cursor >= span.wb && span.looping { cursor = span.wa; } - let n = ((span.wb - cursor) as usize).min(BLOCK); - feed.push(&mix_block(src, comp, sample_time(cursor, rate), n, rate, mix)); - cursor + n as i64 + if cursor < span.wb { + let n = span.wb.saturating_sub(cursor).clamp(0, BLOCK as i64) as usize; + block = Some(if timeline.normal() { + mix_block(&src, comp, timeline.time(cursor), n, timeline.rate(), mix) + } else { + timeline.mix(cursor, n, mix, |t, frames| { + effectcraft_engine::render::audio::mix_comp(&src.project, src.footage.as_ref(), src.expr.as_deref(), comp, t, frames, mix) + }) + }); + cursor += n as i64; + } + let Ok(mut s) = state.lock() else { return }; + if s.generation != generation { + return; + } + match block { + Some(b) => feed.push(&b), + None => feed.finished.store(true, Ordering::Relaxed), + } + s.cursor = cursor; } /// `n` stereo frames of `comp`'s mix from comp time `t` at the device `rate`, mixed at `mix` Hz @@ -457,6 +629,13 @@ mod tests { assert_eq!(clock_sample(&once, 100_000, 0), 47999); } + #[test] + fn slow_preview_clocks_do_not_overflow_a_wide_output_span() { + let span = PlaySpan { start: 0, wa: i64::MIN, wb: i64::MAX, looping: true }; + assert_eq!(clock_sample(&span, u64::MAX, 0), 0); + assert_eq!(clock_sample(&PlaySpan { looping: false, ..span }, u64::MAX, 0), i64::MAX - 1); + } + #[test] fn frame_mapping_matches_comp_frames() { let fps = FrameRate::new(30, 1); @@ -559,6 +738,145 @@ mod tests { playback.stop(); } + /// #2104: muting a layer while the preview plays silences it: the queued sound is dropped + /// and everything mixed afterwards comes from the edited project. + #[test] + fn muting_a_layer_while_playing_silences_the_preview() { + use effectcraft_engine::color::Label; + use effectcraft_engine::project::{Comp, Footage, FootageKind, ItemKind, LayerSource, build}; + use effectcraft_engine::render::FootageSource; + use effectcraft_engine::render::cache::LayerCache; + use std::time::{Duration, Instant}; + + /// Footage audio that is a constant 0.5. + struct Steady; + impl FootageSource for Steady { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + None + } + fn audio(&self, _: ItemId, _: &Footage, _: Tick, frames: usize, _: u32) -> Option> { + Some(vec![0.5; frames * 2]) + } + } + struct Device; + impl AudioDevice for Device { + fn sample_rate(&self) -> u32 { + 48000 + } + fn start(&mut self, _: Arc) -> Result<(), String> { + Ok(()) + } + fn stop(&mut self) {} + } + + let mut p = Project::default(); + let footage = + Footage { path: "tone.flac".into(), kind: FootageKind::Audio, duration: Tick::from_seconds_f64(60.0), has_audio: true, ..Default::default() }; + let item = p.add_item("tone.flac", Label::SeaFoam, None, ItemKind::Footage(footage)); + let mut comp = Comp::new(64, 36, FrameRate::new(30, 1), Tick::from_seconds_f64(10.0)); + let layer = build::layer(&mut p, &comp, "tone.flac", LayerSource::Footage { item }, (0, 0), None); + comp.layers = vec![layer].into(); + let cid = p.add_item("Comp", Label::Sandstone, None, ItemKind::Comp(comp.into())); + let mut project = Arc::new(p); + let src = RenderSource { + project: project.clone(), + footage: Arc::new(Steady), + expr: None, + layer_cache: Arc::new(LayerCache::default()), + gpu: None, + gpu_display: false, + disk: None, + }; + let mut playback = + AudioPlayback::start(Box::new(Device), src, cid, Tick::ZERO, Tick::ZERO, Tick::from_seconds_f64(10.0), true, 0).expect("the preview starts"); + let t = Instant::now(); + while playback.feed.queued_frames() == 0 { + assert!(t.elapsed() < Duration::from_secs(30), "the feeder never delivered"); + std::thread::sleep(Duration::from_millis(10)); + } + // Mute the layer while the sound plays. + if let Some(c) = Arc::make_mut(&mut project).comp_mut(cid) { + c.layers[0].switches.audio = false; + } + playback.set_project(project.clone()); + assert!(playback.project().is_some_and(|p| Arc::ptr_eq(&p, &project))); + // The feeder tops the queue up again, now from the muted project. + while playback.feed.queued_frames() == 0 { + assert!(t.elapsed() < Duration::from_secs(30), "the feeder never delivered"); + std::thread::sleep(Duration::from_millis(10)); + } + let mut out = vec![1.0f32; playback.feed.queued_frames() * 2]; + playback.feed.pull(&mut out); + assert!(out.iter().all(|v| *v == 0.0), "the muted layer still plays"); + playback.stop(); + } + + /// #2104: only edits that change the mix drop the queued sound. + #[test] + fn only_audible_edits_change_the_audio() { + use effectcraft_engine::color::Label; + use effectcraft_engine::project::{Comp, Footage, FootageKind, build}; + + let mut p = Project::default(); + let footage = + Footage { path: "tone.flac".into(), kind: FootageKind::Audio, duration: Tick::from_seconds_f64(60.0), has_audio: true, ..Default::default() }; + let item = p.add_item("tone.flac", Label::SeaFoam, None, ItemKind::Footage(footage)); + let mut comp = Comp::new(64, 36, FrameRate::new(30, 1), Tick::from_seconds_f64(10.0)); + let layer = build::layer(&mut p, &comp, "tone.flac", LayerSource::Footage { item }, (0, 0), None); + comp.layers = vec![layer].into(); + let cid = p.add_item("Comp", Label::Sandstone, None, ItemKind::Comp(comp.into())); + let old = p; + // A video-only edit: the layer is hidden, its name and label change. + let mut video = old.clone(); + if let Some(l) = video.comp_mut(cid).and_then(|c| c.layers.first_mut()) { + l.switches.video = false; + l.name = "renamed".into(); + } + assert!(!audio_changed(&old, &video, cid), "a video edit must not drop the sound"); + // Muting, solo and a moved in point do change it. + for edit in 0..3 { + let mut edited = old.clone(); + if let Some(l) = edited.comp_mut(cid).and_then(|c| c.layers.first_mut()) { + match edit { + 0 => l.switches.audio = false, + 1 => l.switches.solo = true, + _ => l.in_point = Tick::from_seconds_f64(1.0), + } + } + assert!(audio_changed(&old, &edited, cid), "audible edit {edit} must be followed"); + } + } + + /// #2104: a video effect edit leaves the mix alone, an audio effect changes it. + #[test] + fn only_audio_effects_change_the_audio() { + use effectcraft_engine::color::Label; + use effectcraft_engine::project::build::{self, Ids}; + use effectcraft_engine::project::{Comp, Footage, FootageKind}; + + let mut p = Project::default(); + let footage = + Footage { path: "tone.flac".into(), kind: FootageKind::Audio, duration: Tick::from_seconds_f64(60.0), has_audio: true, ..Default::default() }; + let item = p.add_item("tone.flac", Label::SeaFoam, None, ItemKind::Footage(footage)); + let mut comp = Comp::new(64, 36, FrameRate::new(30, 1), Tick::from_seconds_f64(10.0)); + let layer = build::layer(&mut p, &comp, "tone.flac", LayerSource::Footage { item }, (0, 0), None); + comp.layers = vec![layer].into(); + let cid = p.add_item("Comp", Label::Sandstone, None, ItemKind::Comp(comp.into())); + let with_effect = |id: &str| { + let mut edited = p.clone(); + let spec = effectcraft_engine::effects::find(id).expect("the effect exists"); + let mut next = edited.next_id; + let g = effectcraft_engine::effects::instantiate(spec, &mut Ids(&mut next), spec.name, [64.0, 36.0]); + edited.next_id = next; + if let Some(fx) = edited.comp_mut(cid).and_then(|c| c.layers.first_mut()).and_then(|l| l.props.sub_mut("effects")) { + fx.children.push(g.into()); + } + edited + }; + assert!(!audio_changed(&p, &with_effect("ec.blur.gaussian"), cid), "a video effect must not drop the sound"); + assert!(audio_changed(&p, &with_effect("ec.audio.backwards"), cid), "an audio effect must be followed"); + } + #[test] fn meter_decays_and_holds_peaks() { let mut m = Meter::default(); @@ -573,3 +891,7 @@ mod tests { assert!(m.peak_db[0] < -10.0, "{}", m.peak_db[0]); } } + +#[cfg(test)] +#[path = "audio/live_tests.rs"] +mod live_tests; diff --git a/crates/ui-egui/src/audio/live_tests.rs b/crates/ui-egui/src/audio/live_tests.rs new file mode 100644 index 000000000..a7b845c3b --- /dev/null +++ b/crates/ui-egui/src/audio/live_tests.rs @@ -0,0 +1,217 @@ +//! #2104: exercise the real mixer and queue without relying on thread timing or an audio device. + +use super::{AudioDevice, AudioFeed, AudioPlayback, Feeder, PlaySpan, PreviewRate, feed_block}; +use crate::frames::RenderSource; +use effectcraft_engine::color::Label; +use effectcraft_engine::keyframe::Value; +use effectcraft_engine::project::{Comp, Expression, Footage, FootageKind, ItemId, ItemKind, Layer, LayerSource, Project, Property, build}; +use effectcraft_engine::render::{EvalCtx, ExprHost, FootageSource, LayerCache}; +use effectcraft_engine::time::{FrameRate, Tick}; +use std::sync::atomic::AtomicBool; +use std::sync::{Arc, Mutex, mpsc}; +use std::time::Duration; + +struct Device; +impl AudioDevice for Device { + fn sample_rate(&self) -> u32 { + 48000 + } + fn start(&mut self, _: Arc) -> Result<(), String> { + Ok(()) + } + fn stop(&mut self) {} +} + +/// A ramp identifies the exact source sample, exposing skips and repeats after an edit. +struct Ramp; +impl FootageSource for Ramp { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + None + } + fn audio(&self, _: ItemId, _: &Footage, t: Tick, frames: usize, rate: u32) -> Option> { + let first = t.to_units_floor(i64::from(rate)); + Some((0..frames).flat_map(|i| [(first + i as i64) as f32 / 128.0; 2]).collect()) + } +} + +/// Construct a paused feeder whose short nonzero span makes every source sample observable. +fn playback(footage: Arc) -> AudioPlayback { + let mut project = Project::default(); + let f = Footage { path: "ramp.wav".into(), kind: FootageKind::Audio, has_audio: true, ..Default::default() }; + let item = project.add_item("Ramp", Label::SeaFoam, None, ItemKind::Footage(f)); + let mut comp = Comp::new(64, 36, FrameRate::new(30, 1), Tick::from_seconds_f64(1.0)); + let layer = build::layer(&mut project, &comp, "Ramp", LayerSource::Footage { item }, (0, 0), None); + comp.layers = vec![layer].into(); + let cid = project.add_item("Comp", Label::Sandstone, None, ItemKind::Comp(comp.into())); + let src = RenderSource { + project: Arc::new(project), + footage, + expr: None, + layer_cache: Arc::new(LayerCache::default()), + gpu: None, + gpu_display: false, + disk: None, + }; + AudioPlayback { + feed: Arc::new(AudioFeed::default()), + device: Box::new(Device), + stop: Arc::new(AtomicBool::new(false)), + thread: None, + state: Arc::new(Mutex::new(Feeder { src, cursor: 24, generation: 0 })), + comp: cid, + mix: 48000, + rate: 48000, + span: PlaySpan { start: 24, wa: 16, wb: 64, looping: false }, + timeline: PreviewRate::new(48000, 1.0, Tick::ZERO).unwrap(), + } +} + +/// Run exactly one production feeder block, without a background refill racing assertions. +fn mix(p: &AudioPlayback) { + feed_block(&p.state, p.comp, &p.feed, &p.span, p.timeline, p.mix); +} + +/// Copy the active project and apply a layer edit through the same snapshot boundary as the UI. +fn edit(p: &AudioPlayback, update: impl FnOnce(&mut Layer)) -> Arc { + let mut project = p.project().unwrap(); + update(&mut Arc::make_mut(&mut project).comp_mut(p.comp).unwrap().layers[0]); + project +} + +/// Levels edits retain sample position across the loop boundary and the exclusive non-looping end. +#[test] +fn project_replacement_resumes_at_the_first_unconsumed_sample() { + for looping in [false, true] { + let mut p = playback(Arc::new(Ramp)); + p.span.looping = looping; + mix(&p); + let mut out = [0.0; 64]; + p.feed.pull(&mut out); + assert_eq!(out[0], 24.0 / 128.0); + assert_eq!(p.feed.consumed(), 32); + let updated = edit(&p, |l| l.props.sub_mut("audio").unwrap().get_mut("levels").unwrap().value = Value::Vec2([-6.020599913279624; 2])); + p.follow_project(updated); + assert_eq!(p.feed.queued_frames(), 0); + assert_eq!(p.state.lock().unwrap().cursor, 56); + mix(&p); + let mut tail = [0.0; 16]; + p.feed.pull(&mut tail); + assert!((tail[0] - 56.0 / 256.0).abs() < 1e-6); + assert!((tail[14] - 63.0 / 256.0).abs() < 1e-6); + p.set_project(p.project().unwrap()); + assert_eq!(p.state.lock().unwrap().cursor, if looping { 16 } else { 64 }); + mix(&p); + if looping { + let mut first = [0.0; 2]; + p.feed.pull(&mut first); + assert!((first[0] - 16.0 / 256.0).abs() < 1e-6, "resume at the work-area start"); + } else { + assert!(p.finished(), "an edit after the last sample must not replay it"); + assert_eq!(p.feed.queued_frames(), 0); + } + } +} + +struct Gated { + entered: mpsc::Sender<()>, + resume: Mutex>, +} +impl FootageSource for Gated { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + None + } + fn audio(&self, _: ItemId, _: &Footage, _: Tick, frames: usize, _: u32) -> Option> { + self.entered.send(()).unwrap(); + self.resume.lock().unwrap().recv_timeout(Duration::from_secs(10)).unwrap(); + Some(vec![0.5; frames * 2]) + } +} + +/// A slow mix must not block edits or publish samples from the superseded project generation. +#[test] +fn a_block_in_flight_cannot_publish_after_a_mute() { + let (entered, waiting) = mpsc::channel(); + let (resume, gate) = mpsc::channel(); + let mut p = playback(Arc::new(Gated { entered, resume: Mutex::new(gate) })); + let (state, feed, span, comp, rate) = (p.state.clone(), p.feed.clone(), p.span, p.comp, p.rate); + let worker = std::thread::spawn(move || feed_block(&state, comp, &feed, &span, PreviewRate::new(rate, 1.0, Tick::ZERO).unwrap(), rate)); + waiting.recv_timeout(Duration::from_secs(10)).unwrap(); + // The edit completes while the mixer is blocked, without waiting on its work. + p.follow_project(edit(&p, |l| l.switches.audio = false)); + resume.send(()).unwrap(); + worker.join().unwrap(); + assert_eq!(p.feed.queued_frames(), 0, "the old generation cannot publish"); + assert_eq!(p.state.lock().unwrap().cursor, 24, "nor advance the replacement cursor"); + mix(&p); + let mut out = [1.0; 16]; + p.feed.pull(&mut out); + assert_eq!(out, [0.0; 16]); +} + +/// Models a levels expression that reads a video property through the renderer's expression API. +struct OpacityLevels; +impl ExprHost for OpacityLevels { + fn eval(&self, _: &EvalCtx, layer: &Layer, _: &Property, _: &Value) -> Result { + let opacity = layer.transform().unwrap().get("opacity").unwrap().value.as_f64(); + Ok(Value::Vec2([if opacity == 0.0 { -96.0 } else { 0.0 }; 2])) + } +} + +/// Video properties are irrelevant to a plain mix, but may be dependencies of audio expressions. +#[test] +fn video_edits_keep_the_queue_unless_audio_expressions_can_read_them() { + for expression in [false, true] { + let mut p = playback(Arc::new(Ramp)); + if expression { + let updated = edit(&p, |l| { + l.props.sub_mut("audio").unwrap().get_mut("levels").unwrap().expr = + Some(Expression { text: "[transform.opacity, transform.opacity]".into(), enabled: true }); + }); + let mut state = p.state.lock().unwrap(); + state.src.project = updated; + state.src.expr = Some(Arc::new(OpacityLevels)); + } + mix(&p); + assert_eq!(p.feed.queued_frames(), 40); + let updated = edit(&p, |l| l.props.sub_mut("transform").unwrap().get_mut("opacity").unwrap().value = Value::Scalar(0.0)); + p.follow_project(updated); + if expression { + assert_eq!(p.feed.queued_frames(), 0, "the expression can read the edited video property"); + mix(&p); + let mut out = [1.0; 16]; + p.feed.pull(&mut out); + assert_eq!(out, [0.0; 16]); + } else { + assert_eq!(p.feed.queued_frames(), 40, "an unrelated edit cannot interrupt audio"); + } + } +} + +/// Reused nested comps cost linear visits while edits to their descendants remain detectable. +#[test] +fn shared_nested_comps_are_compared_once_per_depth() { + let mut project = Project::default(); + let mut child = None; + for _ in 0..10 { + let mut comp = Comp::new(64, 36, FrameRate::new(30, 1), Tick::from_seconds_f64(1.0)); + if let Some(item) = child { + for _ in 0..10 { + let mut layer = build::layer(&mut project, &comp, "Nested", LayerSource::Comp { item }, (64, 36), None); + layer.switches.audio = false; + comp.layers.push(layer); + } + } + child = Some(project.add_item("Nested", Label::Sandstone, None, ItemKind::Comp(comp.into()))); + } + let root = child.unwrap(); + let mut edited = project.clone(); + edited.comp_mut(root).unwrap().layers[0].name = "Renamed".into(); + let mut seen = std::collections::HashSet::new(); + assert!(!super::audio_changed_inner(&project, &edited, root, &mut seen)); + assert_eq!(seen.len(), 10, "compare the ten comps, not a billion paths through them"); + let leaf = *seen.iter().find(|(_, depth)| *depth == 9).map(|(item, _)| item).unwrap(); + let mut layer = project.comp(root).unwrap().layers[0].clone(); + layer.source = LayerSource::Null; + edited.comp_mut(leaf).unwrap().layers.push(layer); + assert!(super::audio_changed(&project, &edited, root), "memoization must still find a nested edit"); +} diff --git a/crates/ui-egui/src/audio/preview_rate.rs b/crates/ui-egui/src/audio/preview_rate.rs new file mode 100644 index 000000000..06124b4c9 --- /dev/null +++ b/crates/ui-egui/src/audio/preview_rate.rs @@ -0,0 +1,199 @@ +//! Preview speed changes composition time, not the output device's sample rate. +//! Keep the clock and the resampler on the same rational output-sample grid. + +use effectcraft_engine::time::{FrameRate, TICKS_PER_SECOND, Tick}; + +use super::PlaySpan; + +/// At most this many composition samples are mixed at once. At very high preview speeds, +/// skip samples that no output frame needs instead of allocating the entire source interval. +const SOURCE_PAGE: usize = 4096; + +#[derive(Clone, Copy)] +pub(super) struct PreviewRate { + rate: u32, + speed: FrameRate, + origin: Tick, +} + +impl PreviewRate { + pub(super) fn new(rate: u32, speed: f64, start: Tick) -> Result { + // A billionth keeps the ratio of decimal preview rates and rational comp rates without + // rounding it to an integer sample rate. Reject unrepresentable values, never mute them. + let num = (speed * 1_000_000_000.0).round(); + if rate == 0 || !speed.is_finite() || speed <= 0.0 || num < 1.0 || num >= i64::MAX as f64 { + return Err("The preview playback rate must be finite, positive and representable".into()); + } + let speed = FrameRate::new(num as i64, 1_000_000_000); + // Keep the existing 1x sample grid. At another speed, anchor the grid to the exact + // requested frame rather than rounding a nonzero start backwards by an output sample. + let origin = if speed.num == speed.den { Tick::ZERO } else { start }; + Ok(Self { rate, speed, origin }) + } + + pub(super) fn rate(self) -> u32 { + self.rate + } + + pub(super) fn normal(self) -> bool { + self.speed.num == self.speed.den + } + + pub(super) fn time(self, sample: i64) -> Tick { + if self.normal() { + // Preserve the legacy grid, including negative samples at unusual device rates. + return super::sample_time(sample, self.rate); + } + self.origin + Tick::from_rational(sample, self.speed.num, i64::from(self.rate) * self.speed.den) + } + + /// First output sample at or after a composition time. Ceiling keeps loop endpoints inside + /// the work area. A fractional loop boundary is quantized by less than one output sample. + fn sample_ceil(self, time: Tick) -> i64 { + let n = i128::from((time - self.origin).0) * i128::from(self.rate) * i128::from(self.speed.den); + let d = i128::from(TICKS_PER_SECOND) * i128::from(self.speed.num); + let sample = n.div_euclid(d) + i128::from(n.rem_euclid(d) != 0); + sample.clamp(i64::MIN as i128, i64::MAX as i128) as i64 + } + + pub(super) fn span(self, start: Tick, wa: Tick, wb: Tick, looping: bool) -> PlaySpan { + if self.normal() { + return PlaySpan::new(start, wa, wb, looping, self.rate); + } + let wa = self.sample_ceil(wa); + PlaySpan { start: 0, wa, wb: self.sample_ceil(wb).max(wa.saturating_add(1)), looping } + } + + /// Source sample and interpolation phase at an absolute output cursor. Computing this + /// from the timeline (not by accumulating block lengths) makes partitioning immaterial. + fn position(self, sample: i64, mix: u32) -> (i64, f32) { + let n = i128::from(self.time(sample).0) * i128::from(mix); + let d = i128::from(TICKS_PER_SECOND); + (n.div_euclid(d) as i64, n.rem_euclid(d) as f32 / d as f32) + } + + pub(super) fn mix(self, cursor: i64, n: usize, mix: u32, mut render: impl FnMut(Tick, usize) -> Vec) -> Vec { + let mut out = Vec::with_capacity(n.saturating_mul(2)); + let mut page = Vec::new(); + let mut first = 0; + for i in 0..n { + let sample = cursor.saturating_add(i as i64); + let (at, fraction) = self.position(sample, mix); + let offset = usize::try_from(at.saturating_sub(first)).ok(); + if page.is_empty() || offset.is_none_or(|i| i >= (page.len() / 2).saturating_sub(1)) { + first = at; + let last = self.position(cursor.saturating_add(n.saturating_sub(1) as i64), mix).0; + let next = self.position(sample.saturating_add(1), mix).0; + let frames = if next.saturating_sub(first) >= SOURCE_PAGE as i64 - 1 { + // No following output sample can reuse this page. Read only this sample + // and its interpolation neighbor, rather than mixing an unused gap. + 2 + } else { + last.saturating_sub(first).saturating_add(2).clamp(2, SOURCE_PAGE as i64) as usize + }; + // mix_comp floors its starting sample. Add the fractional tick back so exotic + // source rates start on `first`, not the preceding composition sample. + let mut t = Tick::from_units(first, i64::from(mix)); + if t.to_units_floor(i64::from(mix)) < first { + t.0 = t.0.saturating_add(1); + } + page = render(t, frames); + } + let index = usize::try_from(at.saturating_sub(first)).unwrap_or(usize::MAX).saturating_mul(2); + for channel in 0..2 { + let a = page.get(index.saturating_add(channel)).copied().unwrap_or(0.0); + let b = page.get(index.saturating_add(2 + channel)).copied().unwrap_or(a); + out.push(a + (b - a) * fraction); + } + } + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::audio::clock_sample; + + #[test] + fn normal_speed_preserves_the_legacy_sample_grid() { + for rate in [44_100, 48_000, 48_001] { + let timeline = PreviewRate::new(rate, 1.0, Tick::from_seconds_f64(-0.3)).unwrap(); + for sample in [-48_001, -1, 0, 1, 48_001] { + assert_eq!(timeline.time(sample), crate::audio::sample_time(sample, rate)); + } + } + } + + #[test] + fn speed_clock_matches_slow_fast_and_fractional_previews() { + for (comp, preview) in [(FrameRate::FPS_60, 30.0), (FrameRate::FPS_30, 60.0), (FrameRate::FPS_59_94, 29.97)] { + let speed = preview / comp.as_f64(); + let start = comp.tick_of(17); + let rate = PreviewRate::new(48_000, speed, start).unwrap(); + assert_eq!(rate.time(0), start, "the first frame stays exact"); + let time = rate.time(48_000); + assert!(((time - start).seconds() - speed).abs() < 1e-9); + assert_eq!(comp.frame_at(time), comp.frame_at(start + Tick::from_seconds_f64(speed))); + } + } + + #[test] + fn speed_clock_subtracts_output_latency_and_wraps_inside_the_work_area() { + for speed in [0.5, 2.0, 0.73] { + let start = FrameRate::FPS_29_97.tick_of(19); + let wa = FrameRate::FPS_29_97.tick_of(13); + let wb = FrameRate::FPS_29_97.tick_of(43); + let rate = PreviewRate::new(48_000, speed, start).unwrap(); + let span = rate.span(start, wa, wb, true); + assert_eq!(rate.time(clock_sample(&span, 512, 1024)), start); + assert!(rate.time(span.wb - 1) < wb); + let wrapped = rate.time(clock_sample(&span, span.wb as u64 + 1024, 1024)); + assert!(wrapped >= wa && (wrapped - wa).seconds() < speed / 48_000.0); + let once = PlaySpan { looping: false, ..span }; + assert!(rate.time(clock_sample(&once, u32::MAX as u64, 0)) < wb); + } + } + + fn ramp(t: Tick, n: usize, mix: u32) -> Vec { + let first = t.to_units_floor(i64::from(mix)); + (0..n).flat_map(|i| [(first + i as i64) as f32 / 100_000.0, -(first + i as i64) as f32 / 100_000.0]).collect() + } + + #[test] + fn speed_mix_keeps_fractional_phase_across_uneven_blocks() { + for speed in [0.5, 2.0, 0.73, 29.97 / FrameRate::FPS_59_94.as_f64()] { + let start = FrameRate::FPS_29_97.tick_of(17); + let rate = PreviewRate::new(48_000, speed, start).unwrap(); + for mix in [44_100, 48_000, 96_000] { + let whole = rate.mix(0, 2048, mix, |t, n| ramp(t, n, mix)); + let split: Vec<_> = [(0, 321), (321, 107), (428, 1620)].into_iter().flat_map(|(at, n)| rate.mix(at, n, mix, |t, n| ramp(t, n, mix))).collect(); + assert_eq!(whole, split, "speed {speed}, source rate {mix}"); + for (i, frame) in whole.as_chunks::<2>().0.iter().enumerate() { + let expected = (start.seconds() + i as f64 * speed / 48_000.0) * mix as f64 / 100_000.0; + assert!((f64::from(frame[0]) - expected).abs() < 1e-6); + assert_eq!(frame[1], -frame[0]); + } + } + } + } + + #[test] + fn extreme_speeds_keep_source_reads_bounded_without_changing_speed() { + for speed in [0.00001, 99_900.0] { + let rate = PreviewRate::new(8_000, speed, Tick::ZERO).unwrap(); + let mut rendered = 0; + let out = rate.mix(0, 1024, 192_000, |_, n| { + assert!(n <= SOURCE_PAGE); + rendered += n; + vec![0.25; n * 2] + }); + assert!(rendered <= SOURCE_PAGE, "unused source intervals were mixed: {rendered} samples"); + assert_eq!(out, vec![0.25; 2048]); + assert!((rate.time(8000).seconds() - speed).abs() < 1e-9); + } + for speed in [0.0, -1.0, f64::NAN, f64::INFINITY, f64::MAX] { + assert!(PreviewRate::new(48_000, speed, Tick::ZERO).is_err()); + } + } +} diff --git a/crates/ui-egui/src/gpu_failure.rs b/crates/ui-egui/src/gpu_failure.rs index 275112912..a0840b790 100644 --- a/crates/ui-egui/src/gpu_failure.rs +++ b/crates/ui-egui/src/gpu_failure.rs @@ -2,7 +2,9 @@ //! embedding host, never to a library borrowing its device. This bridge does no device work, //! document mutation, file I/O or renderer locking inside a notification. -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, OnceLock}; + +pub use effectcraft_gpu::{ScopedError, ScopedErrorObserver}; type Notifier = Arc; @@ -25,11 +27,22 @@ struct State { pub struct GpuFailureBridge { state: Arc>, ctx: egui::Context, + scoped_observer: Arc>, } impl GpuFailureBridge { pub fn new(ctx: &egui::Context) -> Self { - Self { state: Arc::default(), ctx: ctx.clone() } + Self { state: Arc::default(), ctx: ctx.clone(), scoped_observer: Arc::default() } + } + + /// Register the host observer before handing this bridge to the app. Scoped errors do + /// not retire the device: they remain recoverable per-frame observations. + pub fn set_scoped_error_observer(&self, observer: impl Fn(ScopedError) + Send + Sync + 'static) -> bool { + self.scoped_observer.set(Arc::new(observer)).is_ok() + } + + pub(crate) fn scoped_error_observer(&self) -> Option { + self.scoped_observer.get().cloned() } pub fn failure(&self) -> Option { @@ -90,6 +103,18 @@ mod tests { use super::*; use std::sync::atomic::{AtomicUsize, Ordering}; + #[test] + fn scoped_observer_is_shared_without_retiring_the_device() { + let bridge = GpuFailureBridge::new(&egui::Context::default()); + let seen = Arc::new(Mutex::new(Vec::new())); + let copy = seen.clone(); + assert!(bridge.set_scoped_error_observer(move |error| copy.lock().unwrap().push(error))); + assert!(!bridge.set_scoped_error_observer(|_| panic!("must not replace observer"))); + bridge.clone().scoped_error_observer().unwrap()(ScopedError::Internal); + assert_eq!(*seen.lock().unwrap(), [ScopedError::Internal]); + assert!(bridge.failure().is_none()); + } + #[test] fn a_construction_failure_is_replayed_after_notifier_registration() { let bridge = GpuFailureBridge::new(&egui::Context::default()); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index be14cd8df..7a8805ec4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -203,6 +203,8 @@ pub struct Playback { /// sound plays it goes on through frames that aren't cached, and the picture drops frames to /// follow it (#2473). pub audio_held: bool, + /// The project last checked for a silent preview that became audible (#2104). + pub silent_project: Option>, /// Starting a preview with sound (#947): the playhead stays on the first frame while the /// frames ahead render (the prefetch window, at most what fits in the RAM preview), then /// sound and picture start together from it. Without this the picture played on silently diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index be0ef05d5..4de3233e8 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -588,15 +588,27 @@ pub fn snap_time(candidates: &[f64], t: f64, tolerance: f64) -> f64 { /// Times the current-time indicator snaps to (comp seconds). fn snap_candidates(comp: &Comp, rows: &[Row]) -> Vec { + snap_targets(comp, rows, &[], false) +} + +/// [`snap_candidates`] without the `edited` layers' In / Out points; when `moving` them, without +/// their markers and keys either (those move along). +fn snap_targets(comp: &Comp, rows: &[Row], edited: &[u64], moving: bool) -> Vec { + let skip = |l: LayerId, contents: bool| edited.contains(&l.0) && (moving || !contents); let mut v = vec![0.0, comp.work_area.0.seconds(), comp.work_area.1.seconds(), comp.duration.seconds()]; v.extend(comp.markers.iter().map(|m| m.time.seconds())); for l in &comp.layers { - v.push(l.in_point.seconds()); - v.push(l.out_point.seconds()); - v.extend(l.markers.iter().map(|m| l.comp_time(m.time).seconds())); + if !skip(l.id, false) { + v.push(l.in_point.seconds()); + v.push(l.out_point.seconds()); + } + if !skip(l.id, true) { + v.extend(l.markers.iter().map(|m| l.comp_time(m.time).seconds())); + } } for r in rows { if let RowKind::Prop { uid } = r.kind + && !skip(r.layer, true) && let Some(l) = comp.layer(r.layer) && let Some(p) = l.props.find(uid) { @@ -3473,44 +3485,85 @@ fn layer_quick_offset_begin(ui: &mut egui::Ui, g: &Rw, rc: &RowCtx<'_>, body: &e ctx.data(|d| d.get_temp::(quick_bar_id())).is_some_and(|q| q.layer == layer.id) } -/// Moving and trimming a layer bar by whole frames. +/// One layer bar drag: the layers it edits, the pointer's travel (px), the grabbed layer's moving +/// ends and the snap targets when it started (comp seconds), and the whole frames sent so far. +#[derive(Clone)] +struct BarDrag { + edited: Vec, + px: f64, + ends: Vec, + targets: Arc<[f64]>, + sent: i64, +} + +/// The whole frames a bar drag of `px` moves its ends by: truncated as the pointer goes, or with +/// `shift`, the move that puts one of `ends` on the nearest target within 8 px of the pointer +/// (#1828). Measured from the pointer, never the snapped result, so dragging on lets go. The +/// move stays whole frames, so an end off the frame grid lands within half a frame of its target. +fn bar_drag_frames(px: f64, pps: f64, fd: f64, ends: &[f64], targets: &[f64], shift: bool) -> i64 { + let secs = px / pps; + let snapped = shift + .then(|| { + ends.iter() + .flat_map(|e| targets.iter().map(move |t| t - e)) + .filter(|d| ((d - secs) * pps).abs() <= 8.0) + .min_by(|a, b| (a - secs).abs().total_cmp(&(b - secs).abs())) + }) + .flatten(); + // `as` saturates (NaN → 0), so a zero frame duration or zoom can't panic. + snapped.map_or((secs / fd).trunc() as i64, |d| (d / fd).round() as i64) +} + +/// Moving and trimming a layer bar by whole frames; Shift snaps its ends to the current time, +/// markers, keys, the work area and other layers' In / Out points, as in After Effects (#1828). fn layer_bar_drag(g: &mut Rw, rc: &RowCtx<'_>, bars: &BarResps, quick: bool) { let BarResps { body, lin, lout } = bars; - let pps = g.tl.pps; - let fd = g.tl.fd; let ctx = g.tl.ctx.clone(); - let actions = &mut g.tl.actions; - let selected = &g.selected; - let ui_actions = &mut g.ui_actions; let layer = rc.layer; let drag_key = format!("bar-{}", layer.id.0); for (resp, kind) in [(&body, "move"), (&lin, "in"), (&lout, "out")] { if kind == "move" && quick { continue; } + let id = egui::Id::unique(("bar-drag", layer.id.0, kind)); if resp.dragged() { - let acc_id = egui::Id::unique(("bar-acc", layer.id.0, kind)); - let mut acc: f32 = ctx.data(|d| d.get_temp(acc_id).unwrap_or(0.0)); - acc += resp.drag_delta().x; - let frames = ((acc as f64 / pps) / fd).trunc() as i64; - if frames != 0 { - acc -= (frames as f64 * fd * pps) as f32; - let d = frames as f64 * fd; - // Moving or trimming a selected layer acts on the whole - // selection; each layer trims from its own in/out point. - let targets = json!(edited_layers(layer.id, selected)); - let params = match kind { - "move" => json!({"layers": targets, "delta": d, "merge": drag_key}), - "in" => json!({"layers": targets, "inDelta": d, "merge": drag_key}), - _ => json!({"layers": targets, "outDelta": d, "merge": drag_key}), + let shift = ctx.input(|i| i.modifiers.shift); + // Moving or trimming a selected layer acts on the whole selection as it is when the + // drag starts; each layer trims from its own in/out point. A Shift-drag on an + // unselected bar adds it to the selection (`layer_bar_hits`) and moves them all. + let start = || { + let mut edited = edited_layers(layer.id, &g.selected); + if kind == "move" && shift && !g.selected.contains(&layer.id) { + edited.extend(g.selected.iter().map(|l| l.0)); + } + let (i, o) = (layer.in_point.seconds(), layer.out_point.seconds()); + let ends = match kind { + "move" => vec![i, o], + "in" => vec![i], + _ => vec![o], }; - actions.push(("layer.timing".into(), params)); + let mut targets = snap_targets(&g.tl.comp, &g.tl.rows, &edited, kind == "move"); + targets.push(g.tl.time.seconds()); + BarDrag { edited, px: 0.0, ends, targets: targets.into(), sent: 0 } + }; + let mut drag = ctx.data(|d| d.get_temp::(id)).filter(|_| !resp.drag_started()).unwrap_or_else(start); + drag.px += f64::from(resp.drag_delta().x); + let frames = bar_drag_frames(drag.px, g.tl.pps, g.tl.fd, &drag.ends, &drag.targets, shift); + if frames != drag.sent { + let d = frames.saturating_sub(drag.sent) as f64 * g.tl.fd; + drag.sent = frames; + let params = match kind { + "move" => json!({"layers": drag.edited, "delta": d, "merge": drag_key}), + "in" => json!({"layers": drag.edited, "inDelta": d, "merge": drag_key}), + _ => json!({"layers": drag.edited, "outDelta": d, "merge": drag_key}), + }; + g.tl.actions.push(("layer.timing".into(), params)); } - ctx.data_mut(|d| d.insert_temp(acc_id, acc)); + ctx.data_mut(|d| d.insert_temp(id, drag)); } if resp.drag_stopped() { - ctx.data_mut(|d| d.remove::(egui::Id::unique(("bar-acc", layer.id.0, kind)))); - ui_actions.push(UiAct::EndMerge); + ctx.data_mut(|d| d.remove::(id)); + g.ui_actions.push(UiAct::EndMerge); } } } diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index e42923e4a..1cd5e3423 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -434,6 +434,34 @@ fn cti_snaps_to_the_nearest_candidate() { assert!(c.contains(&comp.duration.seconds()) && c.contains(&0.0)); } +/// A dragged layer's own In / Out points are no snap targets; moved, neither are its markers +/// (#1828). +#[test] +fn bar_drags_dont_snap_to_their_own_layer() { + let mut app = app(); + let bx = app.session.active_comp().unwrap().layers.iter().find(|l| l.name == "Box").unwrap().id.0; + app.session.execute("layer.addMarker", json!({"layers": [bx], "time": 1.0})).unwrap(); + let comp = app.session.active_comp().unwrap().clone(); + let rows = build_rows(&app, &comp); + let all = snap_targets(&comp, &rows, &[], false); + let trimmed = snap_targets(&comp, &rows, &[bx], false); + let moved = snap_targets(&comp, &rows, &[bx], true); + assert_eq!(trimmed.len(), all.len() - 2, "trimming drops only the layer's In and Out"); + assert!(trimmed.contains(&1.0) && !moved.contains(&1.0), "moving drops the layer's marker: {moved:?}"); +} + +/// Shift snaps a bar drag's nearest end to a target within 8 px of the pointer; farther away, or +/// without Shift, the bar moves by the whole frames the pointer covered (#1828). +#[test] +fn bar_drags_snap_their_ends_with_shift() { + let (pps, fd, ends) = (100.0, 0.1, [2.0, 6.0]); + assert_eq!(bar_drag_frames(95.0, pps, fd, &ends, &[3.0], true), 10, "5 px short of 2 → 3 s snaps"); + assert_eq!(bar_drag_frames(95.0, pps, fd, &ends, &[3.0], false), 9, "no snap without Shift"); + assert_eq!(bar_drag_frames(88.0, pps, fd, &ends, &[3.0], true), 8, "12 px away lets go"); + assert_eq!(bar_drag_frames(104.0, pps, fd, &ends, &[3.0, 7.06], true), 11, "the target nearest the pointer wins"); + assert_eq!(bar_drag_frames(5.0, 0.0, 0.0, &ends, &[0.0], true), i64::MAX, "zero zoom saturates, no panic"); +} + /// Every string drawn by one frame of `draw`. fn drawn_text(mut draw: impl FnMut(&mut egui::Ui)) -> Vec { fn collect(shape: &egui::Shape, out: &mut Vec) { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index cd0006c0f..b8fb4bb2b 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -838,9 +838,12 @@ fn phase_menu(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: &m // A right-click an overlay took (the Cryptomatte eyedropper's) opens the menu too. let forwarded = ui.data_mut(|d| d.remove_temp::(egui::Id::unique(FORWARDED_SECONDARY))); if let Some(pos) = resp.interact_pointer_pos().filter(|_| resp.secondary_clicked()).or(forwarded) { - // Picking an already selected layer preserves the group; an unselected hit - // becomes the target, using the same visible/unlocked hit test as left-click. - pick(app, ectx, map.to_comp(pos), false); + // The layer a left click there would select: the topmost one, keeping the group when it + // is already selected (#1535), or the selected layer whose shown path is under the + // pointer. The menu's list reaches the layers under it. + if ov::segment_at(&v.paths, &map, pos, 6.0).is_none() { + click_pick(app, ectx, map.to_comp(pos), false); + } let point = map.to_comp(pos); let hits: Vec<(u64, String)> = layers_at(ectx, point).map(|l| (l.id.0, l.name.clone())).collect(); ui.data_mut(|d| d.insert_temp(menu_hits_id, hits)); @@ -1845,15 +1848,19 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response } // The Roto Brush paints in the Layer panel only: select as usual and say so. rt if rt.roto_kind(false).is_some() => { - if pick(app, ectx, cpt, mods.shift).is_none() { + if click_pick(app, ectx, cpt, mods.shift).is_none() { let _ = app.session.execute("edit.deselectAll", json!({})); } app.ui.status = crate::i18n::tr(ROTO_HINT).to_string(); } _ => { - if pick(app, ectx, cpt, mods.shift).is_none() { + let on_path = ov::segment_at(&paths, &map, pos, 6.0).is_some(); + if on_path && !mods.shift { + // A click on a path of a selected layer stays on that layer, even where a + // layer in front covers it, so a double-click there free-transforms the path. + } else if click_pick(app, ectx, cpt, mods.shift).is_none() { let _ = app.session.execute("edit.deselectAll", json!({})); - } else if !mods.shift && !app.session.state.selected_vertices.is_empty() && ov::segment_at(&paths, &map, pos, 6.0).is_none() { + } else if !mods.shift && !app.session.state.selected_vertices.is_empty() && !on_path { // A click away from the paths, even on their layer, deselects the mask // points (the layer stays selected), as in After Effects (#513). let _ = app.session.execute("mask.selectVertices", json!({"vertices": []})); @@ -1904,7 +1911,8 @@ fn phase_dblclick(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Respons { let cpt = map.to_comp(pos); let roto = app.ui.tool.roto_kind(false).is_some(); - if let Some(l) = pick(app, ectx, cpt, false).and_then(|l| comp.layer(l)) { + // The topmost layer, as the double-click's first click selected it (#1535). + if let Some(l) = click_pick(app, ectx, cpt, false).and_then(|l| comp.layer(l)) { match &l.source { effectcraft_engine::project::LayerSource::Comp { item } => app.session.open_comp(*item), // With a Roto Brush tool, double-clicking a layer opens it in the Layer panel, @@ -2002,8 +2010,8 @@ use geometry::{ use geometry::{box_corners, project_clipped}; pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad}; use gesture::{Gesture, HANDLE, ROTO_HINT, pen_id, roi_drag, roi_handles, vertex_grab_radius}; +pub(crate) use shapes::{click_pick, layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; use shapes::{create_shape, draw_shape_ghost, shape_drag}; -pub(crate) use shapes::{layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; #[cfg(test)] use tools::constrain_45; use tools::{add_motion_key, axis_at, axis_drag, drag_axes, draw_rigs, parent_world_inverse, pen_cursor, pen_cursor_icon, pen_press, select_motion_key}; diff --git a/crates/ui-egui/src/panels/viewer/shapes.rs b/crates/ui-egui/src/panels/viewer/shapes.rs index 76892cbaf..ce9b256cd 100644 --- a/crates/ui-egui/src/panels/viewer/shapes.rs +++ b/crates/ui-egui/src/panels/viewer/shapes.rs @@ -39,10 +39,24 @@ fn quad_area(q: &[[f64; 2]; 4]) -> f64 { (0..4).map(|i| q[i][0] * q[(i + 1) % 4][1] - q[(i + 1) % 4][0] * q[i][1]).sum::().abs() / 2.0 } -/// The layer a press at a comp point acts on, selected (Shift toggles the topmost layer). As in +/// The layer a drag from a comp point acts on, selected (Shift toggles the topmost layer). As in /// After Effects, a selected layer under the point comes before the layers in front of it, so it -/// can be dragged where they cover it; otherwise the topmost layer (#230). +/// can be dragged where they cover it; otherwise the topmost layer (#230). Clicks use +/// [`click_pick`], except a Puppet tool click while no layer has pins, which uses this so a pin +/// can go on a selected layer behind others. pub(crate) fn pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool) -> Option { + pick_layer(app, ectx, cpt, toggle, true) +} + +/// The layer a click (press and release without a drag) at a comp point acts on: always the +/// topmost layer, selected unless it already is (Shift toggles it). Only a drag goes to a selected +/// layer behind others ([`pick`]), so a click can still select a layer in front of it (#1535). +/// The viewer doesn't call it for a click on a shown path, which stays on the path's layer. +pub(crate) fn click_pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool) -> Option { + pick_layer(app, ectx, cpt, toggle, false) +} + +fn pick_layer(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool, prefer_selected: bool) -> Option { if toggle { let hit = layer_at(ectx, cpt)?.id; let _ = app.session.execute("layer.select", json!({"layers": [hit.0], "toggle": true})); @@ -50,7 +64,8 @@ pub(crate) fn pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], togg } let selected = &app.session.state.selected_layers; let hits: Vec = layers_at(ectx, cpt).map(|l| l.id).collect(); - let hit = *hits.iter().find(|id| selected.contains(id)).or(hits.first())?; + let preferred = if prefer_selected { hits.iter().find(|id| selected.contains(id)) } else { None }; + let hit = *preferred.or(hits.first())?; if !selected.contains(&hit) { let _ = app.session.execute("layer.select", json!({"layers": [hit.0]})); } diff --git a/crates/ui-egui/tests/it/ui_audio_scrub.rs b/crates/ui-egui/tests/it/ui_audio_scrub.rs index 80936042a..f6baa0443 100644 --- a/crates/ui-egui/tests/it/ui_audio_scrub.rs +++ b/crates/ui-egui/tests/it/ui_audio_scrub.rs @@ -739,3 +739,206 @@ fn adding_markers_does_not_stop_the_sound() { } assert_eq!(h.state().session.project.comp(cid).unwrap().markers.len(), 6); } + +/// #2104: unmuting a preview that started silent opens sound at the moving preview position, +/// both with video and with Video off (where the timeline deliberately stays still). +#[test] +fn unmuting_a_silent_preview_starts_audio_at_its_current_frame() { + for video in [false, true] { + let fake = Fake::default(); + let (mut app, cid) = setup(true, &fake); + let layer = app.session.project.comp(cid).unwrap().layers[0].id.0; + app.session.execute("layer.setSwitch", json!({"layers": [layer], "switch": "audio", "value": false})).unwrap(); + let gate = GatedFrames::default(); + app.frames.set_remote(Some(Arc::new(gate.clone()))); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 30.0).build_eframe(|_| app); + h.state_mut().use_manual_clock(); + h.run_steps(2); + gate.render(&mut h, cid, &(0..60).collect::>()); + let now = h.state().now(); + let shortcut = if video { h.state().session.prefs.preview.current } else { effectcraft_engine::preview::PreviewShortcut::AltNumpad0 }; + h.state_mut().play_with(now, shortcut); + for _ in 0..6 { + step(&mut h); + } + assert!(h.state().audio.is_none(), "muted playback has no output"); + let current = if video { frame(&h, cid) } else { h.state().playback.audio_frame.unwrap() }; + assert!(current > 0, "the silent preview advanced"); + h.state_mut().session.execute("layer.setSwitch", json!({"layers": [layer], "switch": "audio", "value": true})).unwrap(); + gate.render(&mut h, cid, &(0..60).collect::>()); + h.step(); + assert_eq!(audio_frame(&h, cid), Some(current), "unmute starts at the moving preview frame, video={video}"); + if !video { + assert_eq!(frame(&h, cid), 0, "audio-only preview leaves the timeline stationary"); + } + let feed = fake.0.lock().unwrap().0.clone().expect("unmute opened the output"); + assert!(pull(&feed, 256).iter().any(|v| v.abs() > 0.01), "the unmuted tone reaches the output"); + // Mute and unmute again without stopping: edits reach the existing feed too. + h.state_mut().session.execute("layer.setSwitch", json!({"layers": [layer], "switch": "audio", "value": false})).unwrap(); + h.step(); + assert!(pull(&feed, 256).iter().all(|v| *v == 0.0), "mute silences the current output"); + h.state_mut().session.execute("layer.setSwitch", json!({"layers": [layer], "switch": "audio", "value": true})).unwrap(); + h.step(); + assert!(pull(&feed, 256).iter().any(|v| v.abs() > 0.01), "a second unmute reaches the same output"); + h.state_mut().stop(); + } +} + +/// Pull only queued frames: underruns are deliberately excluded so the test measures the +/// feeder's composition mapping, not how quickly a loaded test host schedules its thread. +fn take_preview_samples(app: &EffectcraftApp, frames: usize) -> Vec { + let audio = app.audio.as_ref().expect("preview audio opened"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60); + let mut samples = Vec::with_capacity(frames * 2); + while samples.len() < frames * 2 { + let n = audio.feed.queued_frames().min(frames - samples.len() / 2); + if n == 0 { + assert!(!audio.finished(), "the preview ended before the requested samples"); + assert!(std::time::Instant::now() < deadline, "audio feeder timed out"); + std::thread::sleep(std::time::Duration::from_millis(1)); + continue; + } + let mut block = vec![0.0; n * 2]; + audio.feed.pull(&mut block); + samples.extend(block); + } + samples +} + +#[test] +fn preview_rate_audio_sounds_at_the_selected_speed_and_finishes_at_the_end() { + use effectcraft_engine::preview::{PlayFrom, PreviewRange}; + for (comp_fps, preview_fps, duration_frames) in [(60.0, 30.0, 96_000), (30.0, 60.0, 24_000)] { + let fake = Fake::default(); + let (mut app, cid) = setup_long(true, &fake, 1.0); + app.session.execute("comp.settings", json!({"comp": cid.0, "frameRate": comp_fps})).unwrap(); + let p = &mut app.session.prefs.preview.spacebar; + p.include_video = false; + p.frame_rate = Some(preview_fps); + p.loop_ = false; + p.range = PreviewRange::EntireDuration; + p.play_from = PlayFrom::RangeStart; + app.play(0.0); + assert!(app.audio.is_some(), "a frame-rate override must not mute the preview"); + let samples = take_preview_samples(&app, 12_000); + assert!(samples.iter().any(|v| v.abs() > 0.01)); + let audio = app.audio.as_ref().unwrap(); + assert_eq!(audio.rate, 48_000, "the physical output rate is unchanged"); + assert!((audio.clock_time().seconds() - 0.25 * preview_fps / comp_fps).abs() < 1e-9); + take_preview_samples(&app, duration_frames - 12_000); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60); + while !app.audio.as_ref().unwrap().finished() { + assert!(std::time::Instant::now() < deadline, "one-shot audio did not finish"); + std::thread::sleep(std::time::Duration::from_millis(1)); + } + assert!(app.audio.as_ref().unwrap().clock_time().seconds() < 1.0); + app.stop(); + } +} + +#[test] +fn preview_rate_audio_resamples_the_sound_instead_of_only_changing_the_clock() { + use effectcraft_engine::preview::{PlayFrom, PreviewRange}; + let render = |preview_fps, frames| { + let fake = Fake::default(); + let (mut app, _) = setup(true, &fake); + let p = &mut app.session.prefs.preview.spacebar; + p.include_video = false; + p.frame_rate = Some(preview_fps); + p.range = PreviewRange::EntireDuration; + p.play_from = PlayFrom::RangeStart; + app.session.prefs.audio.preview_sample_rate = 48_000; + app.play(0.0); + let samples = take_preview_samples(&app, frames); + app.stop(); + samples + }; + let normal = render(30.0, 2048); + let slow = render(15.0, 4096); + for (original, slowed) in normal.as_chunks::<2>().0.iter().zip(slow.as_chunks::<4>().0) { + assert!((original[0] - slowed[0]).abs() < 1e-5); + assert!((original[1] - slowed[1]).abs() < 1e-5); + } +} + +#[test] +fn preview_rate_audio_drives_video_at_half_speed_and_loops() { + let fake = Fake::default(); + let (mut app, cid) = setup_long(true, &fake, 1.0); + app.session.execute("comp.settings", json!({"comp": cid.0, "frameRate": 60})).unwrap(); + app.session.prefs.preview.spacebar.frame_rate = Some(30.0); + app.session.prefs.preview.spacebar.loop_ = true; + let gate = GatedFrames::default(); + app.frames.set_remote(Some(Arc::new(gate.clone()))); + let ahead = (app.frames.parallelism() * 2).max(4) as i64; + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_| app); + h.state_mut().use_manual_clock(); + h.run_steps(2); + gate.discard(&mut h); + h.state_mut().play(0.0); + assert!(h.state().playback.starting, "mismatched-rate audio waits with the first picture"); + let frames: Vec = (0..ahead.min(60)).collect(); + gate.render(&mut h, cid, &frames); + step(&mut h); + assert!(h.state().audio.is_some()); + take_preview_samples(h.state(), 4800); + step(&mut h); + assert_eq!(frame(&h, cid), 3, "0.1 output seconds is 0.05 composition seconds"); + take_preview_samples(h.state(), 96_000 - 4800); + step(&mut h); + assert_eq!(frame(&h, cid), 0, "the audio and picture wrap together"); + assert!(!h.state().audio.as_ref().unwrap().finished()); + h.state_mut().stop(); +} + +#[test] +fn preview_rate_audio_keeps_the_include_audio_switch_and_skip_independent() { + let fake = Fake::default(); + let (mut app, _) = setup(true, &fake); + app.session.prefs.preview.spacebar.include_audio = false; + app.session.prefs.preview.spacebar.frame_rate = Some(60.0); + app.play(0.0); + assert!(app.audio.is_none() && !app.playback.audio_held && !app.playback.starting); + app.stop(); + let p = &mut app.session.prefs.preview.spacebar; + p.include_audio = true; + p.include_video = false; + p.skip = 2; + app.play(0.0); + take_preview_samples(&app, 12_000); + assert_eq!(app.audio.as_ref().unwrap().clock_time(), Tick::from_seconds_f64(0.5), "Skip drops pictures, not audio or time"); + app.stop(); +} + +/// Live edits keep the consumed output cursor at either speed, including a nonzero start. +#[test] +fn preview_rate_audio_keeps_its_position_through_live_mute_and_unmute() { + use effectcraft_engine::preview::{PlayFrom, PreviewRange}; + for preview_fps in [15.0, 60.0] { + let fake = Fake::default(); + let (mut app, cid) = setup(true, &fake); + let comp = app.session.project.comp(cid).unwrap(); + let layer = comp.layers[0].id.0; + let start = comp.frame_rate.tick_of(17); + app.session.set_time(start); + let p = &mut app.session.prefs.preview.spacebar; + p.include_video = false; + p.frame_rate = Some(preview_fps); + p.range = PreviewRange::EntireDuration; + p.play_from = PlayFrom::CurrentTime; + app.play(0.0); + assert_eq!(app.audio.as_ref().unwrap().clock_time(), start); + for (edit, audible) in [(None, true), (Some(false), false), (Some(true), true)] { + if let Some(value) = edit { + app.session.execute("layer.setSwitch", json!({"layers": [layer], "switch": "audio", "value": value})).unwrap(); + app.audio.as_mut().unwrap().follow_project(app.session.project.clone()); + } + let before = app.audio.as_ref().unwrap().clock_time(); + let samples = take_preview_samples(&app, 2400); + assert_eq!(samples.iter().any(|v| v.abs() > 0.01), audible); + let elapsed = (app.audio.as_ref().unwrap().clock_time() - before).seconds(); + assert!((elapsed - 0.05 * preview_fps / 30.0).abs() < 1e-9, "live edit changed the preview cursor"); + } + app.stop(); + } +} diff --git a/crates/ui-egui/tests/it/ui_timeline_trim.rs b/crates/ui-egui/tests/it/ui_timeline_trim.rs index 6f95f744f..9aaf1f500 100644 --- a/crates/ui-egui/tests/it/ui_timeline_trim.rs +++ b/crates/ui-egui/tests/it/ui_timeline_trim.rs @@ -45,18 +45,46 @@ fn timing(h: &Harness<'_, EffectcraftApp>, layer: u64) -> (f64, f64, f64) { } fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) { + drag_with(h, from, to, Modifiers::NONE); +} + +fn drag_with(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2, modifiers: Modifiers) { h.event(Event::PointerMoved(from)); + h.event(Event::ModifiersChanged(modifiers)); h.step(); - h.event(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.event(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers }); h.step(); for k in 1..=10 { h.event(Event::PointerMoved(from + (to - from) * (k as f32 / 10.0))); h.step(); } - h.event(Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.event(Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.event(Event::ModifiersChanged(Modifiers::NONE)); h.run_steps(3); } +/// Shift-dragging a bar end or the bar snaps it to another layer's In / Out point a few pixels +/// away; without Shift it stops on the frame the pointer reached (#1828). +#[test] +fn shift_dragging_a_bar_snaps_to_other_layers() { + for shift in [false, true] { + let (mut h, solid, pre) = harness(); + let mods = if shift { Modifiers::SHIFT } else { Modifiers::NONE }; + let (b, p) = (bar(&h, solid), bar(&h, pre)); + let y = b.center().y; + // The solid's Out end (6 s), grabbed 2 px inside, to 6 px short of the precomp's Out (7 s). + drag_with(&mut h, pos2(b.max.x - 2.0, y), pos2(p.max.x - 8.0, y), mods); + let (i, o, _) = timing(&h, solid); + assert!((i - 2.0).abs() < 1e-6, "shift {shift}: in stays at 2 s, got {i}"); + assert_eq!((o - 7.0).abs() < 1e-6, shift, "shift {shift}: out {o} snaps to 7 s only with Shift"); + // The whole bar, so its In (2 s) lands 5 px short of the precomp's In (3 s). + let b = bar(&h, solid); + drag_with(&mut h, pos2(b.center().x, y), pos2(b.center().x + p.min.x - b.min.x - 5.0, y), mods); + let (i, _, _) = timing(&h, solid); + assert_eq!((i - 3.0).abs() < 1e-6, shift, "shift {shift}: in {i} snaps to 3 s only with Shift"); + } +} + /// Both ends of every bar show the trim cursor, just inside and just outside the end; the middle /// doesn't. The same holds for a selected layer, a twirled-open layer and a short bar. fn assert_trim_cursor_at_ends(h: &mut Harness<'_, EffectcraftApp>, layer: u64, case: &str) { @@ -120,6 +148,19 @@ fn dragging_bar_ends_trims_in_and_out() { assert!((i - 3.0).abs() < 1e-6 && (o - 8.0).abs() < 1.5 / 30.0 && s.abs() < 1e-6, "out extended to ~8 s, not slipped: in {i} out {o} start {s}"); } +/// A Shift-drag on an unselected bar adds it to the selection and moves the whole selection +/// together, so the other selected layers keep their offset (#1828). +#[test] +fn shift_dragging_an_unselected_bar_moves_the_selection() { + let (mut h, solid, pre) = harness(); + h.state_mut().session.execute("layer.select", json!({"layers": [pre]})).unwrap(); + h.run_steps(2); + let b = bar(&h, solid); + drag_with(&mut h, b.center(), b.center() + vec2(b.width() / 4.0 * 2.5, 0.0), Modifiers::SHIFT); + let ((si, ..), (pi, ..)) = (timing(&h, solid), timing(&h, pre)); + assert!(si > 4.0 && (pi - si - 1.0).abs() < 1e-6, "both moved by the same amount: solid in {si}, precomp in {pi}"); +} + /// With Frames and Frame Count Start at 1, In moves to 00061 but the Duration column still counts /// the layer's 120 frames (#1807). #[test] diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index e01608e16..7934168cd 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -198,11 +198,10 @@ fn viewer_click_picks_nothing_while_the_solo_layer_is_inactive() { assert!(h.state().session.state.selected_layers.is_empty(), "{back:?} stays excluded by solo"); } -/// A selected layer under the pointer takes the press before the layers in front of it, as in -/// After Effects: it stays selected and a drag moves it. With none selected there, the topmost -/// layer does (#230). +/// A click, left or right, selects the topmost layer under the pointer, even over a selected +/// layer behind it (#1535); a drag still moves a selected layer behind others (#230). #[test] -fn viewer_press_prefers_a_selected_layer_under_the_pointer() { +fn viewer_click_selects_the_topmost_layer_and_a_drag_moves_the_selected_one() { let mut h = selection_harness(); let comp = h.state().session.active_comp().unwrap(); let (front, back) = (comp.layers[0].id, comp.layers[1].id); @@ -210,7 +209,20 @@ fn viewer_press_prefers_a_selected_layer_under_the_pointer() { h.run_steps(3); let at = screen(&h, [320.0, 180.0]); click(&mut h, at); - assert_eq!(h.state().session.state.selected_layers, vec![back], "a click keeps the selected layer behind"); + assert_eq!(h.state().session.state.selected_layers, vec![front], "a click selects the layer in front of the selected one (#1535)"); + // A right-click selects what a left click would, so the menu acts on the layer in front, + // here where both layers cover the pointer and the layer behind is selected. + h.state_mut().session.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + h.run_steps(3); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); + assert_eq!(h.state().session.state.selected_layers, vec![front], "a right-click selects the layer in front"); + h.key_press(egui::Key::Escape); + h.run_steps(3); + h.state_mut().session.execute("layer.select", json!({"layers": [back.0]})).unwrap(); h.run_steps(40); let (front0, back0) = (position_and_anchor(&h, front).0, position_and_anchor(&h, back).0); drag(&mut h, at, at + vec2(60.0, 0.0)); @@ -224,6 +236,71 @@ fn viewer_press_prefers_a_selected_layer_under_the_pointer() { assert_eq!(h.state().session.state.selected_layers, vec![front], "nothing selected: the topmost layer"); } +/// A click, left or right, on a path of the selected layer, where a layer in front covers it, stays +/// on that path's layer rather than selecting the layer in front, so a double-click there +/// free-transforms the path (#1535 keeps this from `pick`). +#[test] +fn viewer_double_click_on_a_covered_path_of_the_selected_layer_free_transforms_it() { + let mut h = selection_harness(); + let comp = h.state().session.active_comp().unwrap(); + let back = comp.layers[1].id; + let s = &mut h.state_mut().session; + s.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + // In Back's own space (80×80): its top edge runs through comp (320, 150), under Front. + s.execute("mask.new", json!({"layer": back.0, "vertices": [[10, 10], [70, 10], [70, 70], [10, 70]], "closed": true})).unwrap(); + h.run_steps(40); + let at = screen(&h, [320.0, 150.0]); + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + // A right-click there acts on the path's layer too, as a left click does. + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); + assert_eq!(h.state().session.state.selected_layers, vec![back], "a right-click on the path keeps its layer"); + h.key_press(egui::Key::Escape); + h.run_steps(40); + // Two clicks a frame apart: the first selects as a click does, the second double-clicks. + for _ in 0..2 { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + h.step(); + } + h.run_steps(3); + assert_eq!(h.state().session.state.selected_layers, vec![back], "the path's layer stays selected"); + assert_eq!(h.state().session.state.selected_vertices.len(), 4, "all the path's points are selected"); + assert!(h.state().auto.find("viewer.freeTransform.handle.0").is_some(), "the free-transform box is up"); +} + +/// A double-click where a precomp layer covers the selected layer opens the precomp: like a single +/// click, it acts on the topmost layer, which its first click selects (#1535). +#[test] +fn viewer_double_click_opens_the_topmost_layer_over_a_selected_one() { + let mut s = Session::default(); + let inner = s.execute("comp.new", json!({"name": "Inner", "width": 80, "height": 80, "duration": 4})).unwrap()["comp"].as_u64().unwrap(); + s.execute("comp.new", json!({"name": "Main", "width": 640, "height": 360, "duration": 4})).unwrap(); + s.execute("layer.newSolid", json!({"name": "Back", "color": "#406080", "width": 200, "height": 200})).unwrap(); + s.execute("layer.addItem", json!({"item": "Inner"})).unwrap(); + let back = s.active_comp().unwrap().layers[1].id; + s.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(40); + let at = screen(&h, [320.0, 180.0]); + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + // Two clicks a frame apart: the first selects as a click does, the second double-clicks. + for _ in 0..2 { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + h.step(); + } + h.run_steps(3); + assert_eq!(h.state().session.active_comp_id().map(|c| c.0), Some(inner), "the precomp in front opens"); +} + /// Each composition keeps its Resolution, saved with the project as in After Effects: chosen for /// comp A in the viewer's popup, it is still A's after showing comp B, costs no undo step, keeps /// A's cached frames and is read back from the saved file (#417). diff --git a/docs/agents.md b/docs/agents.md index ae9b1a6f6..06f9bedae 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -865,6 +865,75 @@ not read or written. `` is an id or name, or `-` for the active comp. `` is JSON (`50`, `[960,540]`, `"#ff0000"`) or a bare string. +## Repro scripts + +Every journaled command the app runs (menus, panels, shortcuts, agents, scripts) goes into the session's +command journal. A **repro script** is that journal as JSON lines, so a bug or crash becomes +something an agent can replay and a regression test becomes a data file. + +```text +// comments: lines starting with // or #; blank lines are skipped +{"start": "new" | "demo" | {"project": "rel/or/abs.ecproj"}, "allow_errors": false} optional, first line +{"command": "comp.new", "params": {"name": "Main", "frameRate": 30}} +{"command": "comp.settings", "params": {"frameRate": 0}, "expect_error": "frameRate"} +{"expect": {"query": "comp.info", "pointer": "/frameRate", "equals": 30}} +{"expect_no_error": true} +``` + +- `start`: the project to begin with (default `new`). A relative `project` path is relative to the + script. Other header keys (`version`, `app`, `os`, `crash`, `note`) are for people. +- `command`: run like the app runs it. It must succeed, unless it has `expect_error` (`true`, or a + text the error must contain) or the header says `allow_errors` (errors are reported but + tolerated). A panic always fails the replay. +- `expect`: run a command, normally a query (`prop.get`, `comp.info`, `layer.tree`, `project.summary`), + and compare the value at the JSON `pointer` of its result with `equals` (numbers within 1e-6). + Without `equals` it only checks that the query succeeds. +- `expect_no_error`: the previous command succeeded. + +Where scripts come from: + +- `journal.export {path?}` returns `{script, steps}` (or writes `path`): the commands since the + project was last replaced (`file.newProject`, `file.open`, `file.openDemoProject`). Queries and + saves are not in it; commands a script or `file.runScript` ran are covered by that one step. + A successful Revert starts a new replay boundary at the saved file it reopened. +- `help.exportRepro {path?}` saves the same for a bug report, by default to `Repros/` in the + settings folder (macOS `~/Library/Application Support/EffectCraft/Repros`, next to `Logs/`). + Confined automation must give an explicit path inside a write root; `journal.export {}` + remains available because it only returns the script in memory. +- **Crash repros**: when the last-resort guard catches a panic in a command, the app writes + `Repros/crash--.jsonl` there automatically (the newest 20 are kept): the + journal, then the command that panicked, with `"allow_errors": true` and the error in the header. + The error the user or agent sees names the file. Headless sessions without a settings folder + (the CLI, tests) don't write one. + +Privacy: a repro holds command ids and parameters only, never project, image or media file +contents. Parameters can include file paths, layer and item names and text typed into text +layers, so read a repro before sharing it. + +Replay one headless (a fully wired session, like the CLI's other subcommands): + +```sh +effectcraft-cli replay crash.jsonl # per-step ok/FAIL lines, then the first failure +effectcraft-cli replay crash.jsonl --json # {ok, panicked, steps, first_failure, skipped} +effectcraft-cli replay crash.jsonl --keep-going +``` + +Replaying a repro runs its commands with your privileges and file access, including commands +that write files or run scripts. Read repros from untrusted sources before running them, or use +a disposable environment. + +It exits with status 1 when a step fails, an expectation is not met or a command panics, and +stops at the first failure unless `--keep-going`. Steps that work on the user's files (`file.open` +of their project, imports) need those files; replace them with a `start` project or generated +content when you turn a report into a test. A repro only sees commands: UI state changed +without one (a hover, a half-finished drag) is not in it. + +**Regression tests are data files.** A crash fix's regression test is the repro, trimmed to the +steps that matter, with an `expect_error` or `expect` line where the fix changed the outcome, +dropped into [`tests/repros/`](../tests/repros). `crates/host/tests/it/repros.rs` replays every +`*.jsonl` there and fails on any panic or unmet expectation +(`cargo nextest run -p effectcraft-host --test it -E 'test(/^repros::/)'`). + ## Seeing the UI To work on the UI, start the app with `cargo run -p effectcraft -- --control 9877` (add `--demo` to diff --git a/docs/testing.md b/docs/testing.md index e6cd8df68..4c5bfddcb 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -3,6 +3,7 @@ ```sh cargo test --workspace # everything cargo xtask ci # fmt, clippy -D warnings, tests, pdf-deps, lock-patches, layers, assets, wasm +cargo xtask ci --affected # the same gates, scoped to crates your change can affect ``` The After Effects test projects of `crates/aep` are not in the repository; without them the @@ -32,6 +33,47 @@ Each crate with several integration-test files has one test binary, `tests/it/ma modules are the files in `tests/it/`. Run one file with `cargo test -p --test it ::` or `cargo nextest run -p -E 'binary(it) and test(/^::/)'`. +## Affected-only CI + +Use `cargo xtask ci --affected` to run the gates for the crates a change can affect. The full +`cargo xtask ci` remains the required check before a release and when affected mode falls back +because a workspace-wide input changed. + +```sh +cargo xtask ci --affected # base: the merge-base of HEAD with main +cargo xtask ci --affected --base HEAD~3 # any git revision +cargo xtask ci --affected --dry-run # print the plan only +``` + +It collects files that differ from the base (committed, staged, unstaged and untracked), preserving +Unicode and whitespace in their names. Each file maps to the workspace package whose directory +holds it and every package whose sources point at it with a `../` literal, such as +`include_bytes!("../../../assets/fonts/Inter-Regular.ttf")` or +`concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/preferences.md")`. Deleted and renamed inputs +still select their readers, including when the input's entire directory no longer exists. + +The affected set is those packages plus everything that depends on them (transitively through +normal and build dependencies) and every package that uses one of them as a dev-dependency. +Clippy and tests use the same dev profile as full CI. Both modes use the same test runner: +nextest plus doctests when installed, otherwise cargo test plus doctests. GPU tests run in one +libtest process, and the translation report reruns with visible output when ui-egui is affected. +The audio-backend probe runs before building affected crates. + +Formatting, PDF dependency policy, lock-patch and layer checks always run. Assets are checked +when an asset, an `.attribution` sidecar or `ATTRIBUTION.md` changes. The wasm check selects +affected L0–L4 and web crates. Every step runs even when an earlier step fails; the final result +names failed steps and includes timings. + +Changes to the root `Cargo.toml`, `Cargo.lock`, `.cargo/`, `xtask/`, `rust-toolchain(.toml)`, +`clippy.toml` or `.config/nextest.toml` can change every crate's build or test policy, so they run +the full CI instead and say why. Changes no crate reads (most of `docs/`, `README.md`, `.github/`, +`packaging/`) only get the cheap checks. + +Limits: a crate that reads a repo file at run time through a path that is not a `../` literal in its +sources is not detected, and building a subset of packages can unify features differently from the +whole workspace. Non-UTF-8 Git paths stop the command rather than silently omitting checks. +Run the full `cargo xtask ci` before a release. + ## What the tests cover - **Engine commands** (`crates/engine/src/tests*.rs`): every command family, including undo and @@ -123,16 +165,45 @@ A teardown panic leaves reporting armed. An abort or other unexpected process te closes the pipe without that marker. The helper saves a local report under the session log folder's `Crash Reports` directory and shows a native error dialog. Control-server launches save the report without a modal dialog. -Reports contain version/platform and at most the last 64 KiB of the matching session's log. -Logging filters and the session file size cap can omit later events from that tail. +Reports contain version/platform, UTC detection and observer-start times, observed duration, +GUI uptime at the last target-present sample, and at most the last 64 KiB of the matching +session's log. A separate ring retains the last 32 timestamped, fixed-vocabulary backend and +GPU observations even if startup messages have fallen out of the log tail. `WGPU_BACKEND` +values are normalized to known backend names; arbitrary environment text is not copied. +Logging filters and the session file size cap can omit later events from the log tail. They are never uploaded automatically. Review logs for private project paths before sharing. +On Windows the helper acquires a process handle before acknowledging readiness and retains +it until the GUI exits. It records the real OS exit status in unsigned hexadecimal, with a +small lookup of known status names. These bits are **not a verified exception record**: +software can explicitly set the same code, and an access violation does not identify the +faulting module or prove a driver crash. Other platforms report exit status as unavailable. +A lost pipe while the GUI is still present, or a failed/absent process refresh, is not reported +as confirmed termination. + +The helper samples only the target process and system RAM at roughly one-second intervals, +retaining a count, last-attempt time, sampled resident-memory maximum, Windows private-commit +maximum (virtual memory on other platforms), and system available-RAM minimum. This is a +fixed-size summary, not a memory dump. OS query failure can leave stale or zero values, and +sampling misses short spikes. System commit limits/usage and GPU VRAM budgets are not captured. +No threshold is treated as proof of memory exhaustion. Typed wgpu OOM, internal/validation +errors and device-loss notifications are observations; device destruction is distinguished +from unknown loss, and neither establishes why the process later exited. + Run `cargo test -p effectcraft crash_report::tests` for bounded protocol, normal shutdown, actual subprocess abort, unrelated child handles, concurrent sessions, missing log/helper, -log rotation and report-write failure coverage. The three ignored tests are subprocess entry +log rotation, bounded retained events, live-GUI pipe loss, and report-write failure coverage. +Windows also tests a real subprocess exit with explicit `0xC0000017` bits, without exhausting +memory or pretending that the exit was an actual OOM exception. The three ignored tests are subprocess entry points launched by these tests, not manual tests. -This is diagnosis, not memory recovery: a pipe closing does not prove an out-of-memory failure -or provide the process's exit code. System-wide exhaustion, killing the whole process tree, +These are observations, not memory recovery or a causal diagnosis: a pipe closing does not +prove an out-of-memory failure. System-wide exhaustion, killing the whole process tree, or an unavailable native dialog service can also prevent the helper from reporting. Existing auto-save recovery remains responsible for restoring previously saved work. + +Native exception records, faulting-module attribution and driver/TDR confirmation require +separately collected OS event evidence or a native dump. They are deliberately not gathered +here: dumps can contain document contents and credentials. Any future dump workflow must be +explicitly opt-in, bounded, local-first and separately reviewed before sharing. This reporter +must remain useful when those sources are unavailable. diff --git a/tests/repros/comp-zero-frame-rate.jsonl b/tests/repros/comp-zero-frame-rate.jsonl new file mode 100644 index 000000000..5ce9dd873 --- /dev/null +++ b/tests/repros/comp-zero-frame-rate.jsonl @@ -0,0 +1,13 @@ +// comp.new / comp.settings took a frameRate of 0 (or below 0.0005) as a 0/1 rate: the next +// frame-duration computation divided by zero and killed the process with the unsaved project +// (fixed in 780419c, "Compositions: reject zero and negative frame rates instead of panicking"). +{"start": "new"} +{"command": "comp.new", "params": {"name": "Zero", "frameRate": 0}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Tiny", "frameRate": 0.0001}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Negative", "frameRate": -24}, "expect_error": "frameRate"} +{"command": "comp.new", "params": {"name": "Main", "width": 320, "height": 180, "frameRate": 30, "duration": 1}} +{"command": "comp.settings", "params": {"frameRate": 0}, "expect_error": "frameRate"} +{"expect": {"query": "comp.info", "pointer": "/frameRate", "equals": 30}} +{"command": "layer.newSolid", "params": {"color": "#ff0000"}} +{"command": "time.set", "params": {"time": 0.5}} +{"expect_no_error": true} diff --git a/tests/repros/layer-settings-empty-selection.jsonl b/tests/repros/layer-settings-empty-selection.jsonl new file mode 100644 index 000000000..71a0216ad --- /dev/null +++ b/tests/repros/layer-settings-empty-selection.jsonl @@ -0,0 +1,8 @@ +// Layer Settings' enablement read the first selected layer with `selected_layers[0]`: with layers +// in the comp but none selected it indexed an empty list and panicked (fixed in b21e04b, M4.12). +{"start": "demo"} +{"command": "edit.deselectAll"} +{"command": "layer.settings", "expect_error": "select a layer"} +{"command": "layer.newSolid", "params": {"color": "#3366ff", "name": "Blue"}} +{"command": "layer.settings", "params": {"name": "Renamed"}} +{"expect_no_error": true} diff --git a/tests/repros/new-comp-from-selection-hostile-overlap.jsonl b/tests/repros/new-comp-from-selection-hostile-overlap.jsonl new file mode 100644 index 000000000..07e96afc6 --- /dev/null +++ b/tests/repros/new-comp-from-selection-hostile-overlap.jsonl @@ -0,0 +1,17 @@ +// File > New Comp from Selection with Sequence Layers: a huge `overlapDuration` times the layer +// count overflowed the duration arithmetic (a panic with overflow checks, a wrapped, wrong comp +// length without them), and `dimensionsFrom` past the selection indexed out of bounds +// (fixed in b21e04b, M4.12). +{"start": "new"} +{"command": "comp.new", "params": {"name": "Holder", "width": 64, "height": 64, "frameRate": 30, "duration": 2}} +{"command": "layer.newSolid", "params": {"color": "#ff0000", "name": "a"}} +{"command": "layer.newSolid", "params": {"color": "#00ff00", "name": "b"}} +{"command": "layer.newSolid", "params": {"color": "#0000ff", "name": "c"}} +{"command": "project.select", "params": {"items": ["a", "b", "c"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "sequence": true, "overlap": true, "overlapDuration": 1e30, "duration": 2}} +{"expect": {"query": "comp.info", "pointer": "/width", "equals": 64}} +{"command": "project.select", "params": {"items": ["a", "b", "c"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "sequence": true, "overlap": true, "overlapDuration": -5, "duration": 2}} +{"expect": {"query": "comp.info", "pointer": "/duration", "equals": 4}} +{"command": "project.select", "params": {"items": ["a", "b"]}} +{"command": "file.newCompFromSelection", "params": {"single": true, "dimensionsFrom": 99}, "expect_error": "dimensionsFrom"} diff --git a/xtask/src/affected.rs b/xtask/src/affected.rs new file mode 100644 index 000000000..af56ad0cb --- /dev/null +++ b/xtask/src/affected.rs @@ -0,0 +1,766 @@ +//! `cargo xtask ci --affected [--base REV] [--dry-run]`: the CI gates, run only for the crates a +//! change can affect (docs/testing.md). +//! +//! 1. Changed files: everything that differs from the base (default: the merge-base with `main`), +//! committed, staged, unstaged or untracked. +//! 2. Files map to the workspace package whose directory holds them, and to every package whose +//! sources point at them with a `../` path (`include_bytes!("../../../assets/fonts/…")`, +//! `env!("CARGO_MANIFEST_DIR")` + `"/../../docs/effects.md"`, …). +//! 3. Affected = those packages, every package that depends on them (transitively, through +//! normal and build dependencies) and every package that has one of them as a dev-dependency. +//! 4. Workspace-wide inputs (root `Cargo.toml`, `Cargo.lock`, `.cargo/`, `xtask/`, the toolchain +//! file, `clippy.toml`, `.config/nextest.toml`) fall back to the full `cargo xtask ci`. + +use std::collections::BTreeSet; +use std::path::Path; +use std::process::Command; +use std::time::{Duration, Instant}; + +use serde_json::Value; + +/// A workspace package, as far as change mapping cares. +#[derive(Debug, Clone, Default)] +pub struct Pkg { + pub name: String, + /// Package directory relative to the workspace root, `/`-separated, no trailing slash. + pub dir: String, + /// Workspace (path) dependencies: (package name, is a dev-dependency). + pub deps: Vec<(String, bool)>, + /// Has a library target (`--lib` is valid even when doctests are disabled). + pub has_lib: bool, + /// Has a library target with doctests (`cargo test --doc` runs for it). + pub has_doctests: bool, + /// Repo paths outside `dir` that the sources read (files or directories). + pub inputs: Vec, +} + +/// Changes to these force the full CI: they can change every crate's build or test policy. +pub fn full_ci_reason(path: &str) -> Option<&'static str> { + match path { + "Cargo.toml" => Some("workspace Cargo.toml changed"), + "Cargo.lock" => Some("Cargo.lock changed"), + "rust-toolchain" | "rust-toolchain.toml" => Some("toolchain changed"), + "clippy.toml" => Some("clippy.toml changed"), + // Test selection and required runner versions apply to the whole workspace. + ".config/nextest.toml" => Some("nextest configuration changed"), + _ if path.starts_with(".cargo/") => Some(".cargo/ changed"), + _ if path.starts_with("xtask/") => Some("xtask changed"), + _ => None, + } +} + +/// Files the `assets` check looks at (see `assets()` in main.rs). +pub fn is_asset_related(path: &str) -> bool { + path == "ATTRIBUTION.md" || path.ends_with(".attribution") || crate::is_asset(path) +} + +fn under(path: &str, dir: &str) -> bool { + dir.is_empty() || path == dir || path.strip_prefix(dir).is_some_and(|r| r.starts_with('/')) +} + +/// The package whose directory holds `path` (the innermost one). +pub fn owner<'a>(path: &str, pkgs: &'a [Pkg]) -> Option<&'a Pkg> { + pkgs.iter().filter(|p| !p.dir.is_empty() && under(path, &p.dir)).max_by_key(|p| p.dir.len()) +} + +/// Packages whose sources read `path` from outside their own directory. +pub fn readers<'a>(path: &'a str, pkgs: &'a [Pkg]) -> impl Iterator + 'a { + pkgs.iter().filter(move |p| p.inputs.iter().any(|i| under(path, i))) +} + +/// `direct` plus everything that depends on it: transitively through normal/build edges, then +/// one hop through dev-dependency edges (a dev-dependent's tests rerun, its dependents don't). +pub fn expand(direct: &BTreeSet, pkgs: &[Pkg]) -> BTreeSet { + let mut out = direct.clone(); + let mut stack: Vec = direct.iter().cloned().collect(); + while let Some(n) = stack.pop() { + for p in pkgs { + if !out.contains(&p.name) && p.deps.iter().any(|(d, dev)| !dev && *d == n) { + out.insert(p.name.clone()); + stack.push(p.name.clone()); + } + } + } + let dev: Vec = pkgs.iter().filter(|p| p.deps.iter().any(|(d, dev)| *dev && out.contains(d))).map(|p| p.name.clone()).collect(); + out.extend(dev); + out +} + +/// What `ci --affected` will do for a set of changed files. +#[derive(Debug, Default, PartialEq)] +pub struct Plan { + /// Set when the full CI must run instead. + pub full: Option, + /// Packages that hold or read a changed file. + pub direct: BTreeSet, + /// `direct` and its dependents: clippy and tests run for these. + pub affected: BTreeSet, + /// Whether the assets check runs. + pub assets: bool, + /// Changed files no package reads (docs, CI configs, packaging…). + pub unmapped: Vec, +} + +pub fn plan(files: &[String], pkgs: &[Pkg]) -> Plan { + let mut plan = Plan::default(); + for f in files { + if let Some(why) = full_ci_reason(f) { + plan.full = Some(format!("{why} ({f})")); + return plan; + } + plan.assets |= is_asset_related(f); + let mut hit = false; + for p in owner(f, pkgs).into_iter().chain(readers(f, pkgs)) { + plan.direct.insert(p.name.clone()); + hit = true; + } + if !hit { + plan.unmapped.push(f.clone()); + } + } + plan.affected = expand(&plan.direct, pkgs); + plan +} + +/// Normalise `base/rel` (both `/`-separated, relative to the repo root). None if it leaves the repo. +pub fn join_rel(base: &str, rel: &str) -> Option { + let mut parts: Vec<&str> = base.split('/').filter(|s| !s.is_empty() && *s != ".").collect(); + for c in rel.split('/') { + match c { + "" | "." => {} + ".." => { + parts.pop()?; + } + c => parts.push(c), + } + } + Some(parts.join("/")) +} + +/// The `../…` paths in a line of Rust: the part of each string literal from its first `../`, +/// cut back to a directory when a format placeholder follows (`"{}/../model/fixtures/{n}"`). +pub fn rel_paths(line: &str) -> Vec { + let mut out = vec![]; + for lit in line.split('"').skip(1).step_by(2) { + let Some(i) = lit.find("../") else { continue }; + let Some(rest) = lit.get(i..) else { continue }; + let p = match rest.find('{') { + Some(j) => rest.get(..j).and_then(|s| s.rfind('/').and_then(|k| s.get(..k))).unwrap_or(""), + None => rest, + }; + let p = p.trim_end_matches('/'); + if !p.is_empty() { + out.push(p.to_string()); + } + } + out +} + +/// Repo paths outside `pkg_dir` that a source file at `file` (both repo-relative) points at. +/// A literal may be relative to the file (`include_bytes!`) or to the manifest dir +/// (`CARGO_MANIFEST_DIR`), so both are tried. Keep existing paths and paths containing a change, +/// including deleted files and directories that no longer exist in the working tree. +pub fn inputs_of(file: &str, pkg_dir: &str, text: &str, known_input: &dyn Fn(&str) -> bool) -> BTreeSet { + let file_dir = file.rsplit_once('/').map(|(d, _)| d).unwrap_or(""); + let mut out = BTreeSet::new(); + for line in text.lines().filter(|l| l.contains("../")) { + let t = line.trim_start(); + if t.starts_with("//") && !t.starts_with("///") && !t.starts_with("//!") { + continue; + } + for rel in rel_paths(line) { + for base in [file_dir, pkg_dir] { + let Some(p) = join_rel(base, &rel) else { continue }; + if p.is_empty() || p == "target" || p.starts_with("target/") || under(&p, pkg_dir) || !known_input(&p) { + continue; + } + out.insert(p); + } + } + } + out +} + +fn rs_files(dir: &Path, out: &mut Vec, depth: u32) { + if depth > 32 { + return; + } + for e in std::fs::read_dir(dir).into_iter().flatten().flatten() { + let p = e.path(); + let name = e.file_name(); + if p.is_dir() { + if name != "target" && name != ".git" { + rs_files(&p, out, depth + 1); + } + } else if p.extension().is_some_and(|x| x == "rs") { + out.push(p); + } + } +} + +fn rel_to(root: &Path, p: &Path) -> String { + p.strip_prefix(root).unwrap_or(p).components().map(|c| c.as_os_str().to_string_lossy().into_owned()).collect::>().join("/") +} + +/// Workspace packages (xtask excluded) with their path dependencies and outside inputs. +pub fn packages(root: &Path, md: &Value, changed: &[String]) -> Vec { + let ws = md["workspace_root"].as_str().map(Path::new).unwrap_or(root); + let names: BTreeSet<&str> = md["packages"].as_array().into_iter().flatten().filter_map(|p| p["name"].as_str()).collect(); + let mut pkgs = vec![]; + for p in md["packages"].as_array().into_iter().flatten() { + let Some(name) = p["name"].as_str().filter(|n| *n != "xtask") else { continue }; + let Some(dir) = p["manifest_path"].as_str().and_then(|m| Path::new(m).parent()) else { continue }; + let deps = p["dependencies"] + .as_array() + .into_iter() + .flatten() + .filter(|d| !d["path"].is_null()) + .filter_map(|d| d["name"].as_str().filter(|n| names.contains(n)).map(|n| (n.to_string(), d["kind"] == "dev"))) + .collect(); + let lib_kind = |k: &Value| ["lib", "rlib", "dylib", "cdylib", "staticlib", "proc-macro"].iter().any(|l| k == *l); + let libs: Vec<&Value> = p["targets"].as_array().into_iter().flatten().filter(|t| t["kind"].as_array().into_iter().flatten().any(lib_kind)).collect(); + let has_lib = !libs.is_empty(); + let has_doctests = libs.iter().any(|t| t["doctest"] != false); + let rel = rel_to(ws, dir); + let mut files = vec![]; + rs_files(dir, &mut files, 0); + let known_input = |p: &str| ws.join(p).exists() || changed.iter().any(|f| under(f, p)); + let mut inputs = BTreeSet::new(); + for f in files { + if let Ok(text) = std::fs::read_to_string(&f) { + inputs.extend(inputs_of(&rel_to(ws, &f), &rel, &text, &known_input)); + } + } + pkgs.push(Pkg { name: name.to_string(), dir: rel, deps, has_lib, has_doctests, inputs: inputs.into_iter().collect() }); + } + pkgs +} + +fn git(root: &Path, args: &[&str]) -> Result { + let out = Command::new("git").arg("-C").arg(root).args(args).output().map_err(|e| format!("git: {e}"))?; + if !out.status.success() { + return Err(format!("git {}: {}", args.join(" "), String::from_utf8_lossy(&out.stderr).trim())); + } + String::from_utf8(out.stdout).map_err(|e| format!("git {}: output is not UTF-8; cannot safely map changed paths: {e}", args.join(" "))) +} + +/// The base commit: `--base REV` or the merge-base of HEAD with `main` (or `origin/main`). +fn base(root: &Path, rev: Option<&str>) -> Result<(String, String), String> { + if let Some(r) = rev { + let sha = git(root, &["rev-parse", "--verify", &format!("{r}^{{commit}}")])?; + return Ok((sha.trim().to_string(), r.to_string())); + } + for m in ["main", "origin/main"] { + if let Ok(sha) = git(root, &["merge-base", "HEAD", m]) { + return Ok((sha.trim().to_string(), format!("merge-base with {m}"))); + } + } + Err("no merge-base with main or origin/main; pass --base REV".into()) +} + +/// Committed since `base`, staged, unstaged and untracked files (repo-relative, sorted). +fn changed_files(root: &Path, base: &str) -> Result, String> { + let diff = git(root, &["diff", "--name-only", "--no-renames", "-z", base, "--"])?; + let untracked = git(root, &["ls-files", "--others", "--exclude-standard", "-z"])?; + // Git otherwise C-quotes Unicode, tabs and newlines. Whitespace is part of the filename. + let set: BTreeSet = diff.split('\0').chain(untracked.split('\0')).filter(|p| !p.is_empty()).map(str::to_string).collect(); + Ok(set.into_iter().collect()) +} + +fn short_list(names: &BTreeSet) -> String { + if names.is_empty() { "none".into() } else { names.iter().map(|n| crate::short(n)).collect::>().join(", ") } +} + +fn first_few(files: &[String]) -> String { + let mut s = files.iter().take(3).cloned().collect::>().join(", "); + if files.len() > 3 { + s.push_str(&format!(", … (+{})", files.len() - 3)); + } + s +} + +/// One CI step: label and the cargo invocation (None = an in-process check). +struct Step { + label: String, + cmd: Option, + check: Option Result<(), String>>>, +} + +fn cargo(args: &[&str], pkgs: &[&String], tail: &[&str]) -> Command { + let mut c = Command::new(env!("CARGO")); + c.args(args); + for p in pkgs { + c.args(["-p", p.as_str()]); + } + c.args(tail); + c +} + +/// Library tests and doctests are independent: a library can opt out of doctests. +fn test_targets(plan: &Plan, pkgs: &[Pkg]) -> (bool, bool) { + pkgs.iter().filter(|p| plan.affected.contains(&p.name)).fold((false, false), |(lib, docs), p| (lib || p.has_lib, docs || p.has_doctests)) +} + +fn steps(plan: &Plan, pkgs: &[Pkg], nextest: bool) -> Vec { + let mut v = vec![Step { label: "fmt".into(), cmd: Some(cargo(&["fmt", "--check"], &[], &[])), check: None }]; + let aff: Vec<&String> = plan.affected.iter().collect(); + if !aff.is_empty() { + v.push(Step { label: "audio backend".into(), cmd: None, check: Some(Box::new(crate::audio_backend)) }); + let n = aff.len(); + v.push(Step { label: format!("clippy ({n})"), cmd: Some(cargo(&["clippy", "--all-targets"], &aff, &["--", "-D", "warnings"])), check: None }); + let (has_lib, doctests) = test_targets(plan, pkgs); + let selected: Vec = aff.iter().map(|p| p.to_string()).collect(); + v.push(Step { label: format!("tests ({n})"), cmd: None, check: Some(Box::new(move || crate::tests_for(&selected, nextest, has_lib, doctests))) }); + } + v.push(Step { label: "pdf-deps".into(), cmd: None, check: Some(Box::new(|| crate::pdf_deps::run(&crate::root()))) }); + v.push(Step { label: "lock-patches".into(), cmd: None, check: Some(Box::new(|| crate::lock_patches::run(&crate::root()))) }); + v.push(Step { label: "layers".into(), cmd: None, check: Some(Box::new(crate::layers)) }); + if plan.assets { + v.push(Step { label: "assets".into(), cmd: None, check: Some(Box::new(crate::assets)) }); + } + let wasm: Vec<&String> = aff.iter().copied().filter(|a| crate::layer_of(a).is_some_and(|l| l <= 4) || crate::WEB_CRATES.contains(&a.as_str())).collect(); + if !wasm.is_empty() { + v.push(Step { label: format!("wasm ({})", wasm.len()), cmd: Some(cargo(&["check", "--target", "wasm32-unknown-unknown"], &wasm, &[])), check: None }); + } + v +} + +fn fmt_dur(d: Duration) -> String { + let s = d.as_secs(); + if s >= 60 { format!("{}m{:02}s", s / 60, s % 60) } else { format!("{:.1}s", d.as_secs_f64()) } +} + +fn summary(times: &[(String, Duration, bool)]) { + let total: Duration = times.iter().map(|t| t.1).sum(); + eprintln!("\nci --affected timing:"); + for (label, d, ok) in times { + eprintln!(" {:<16} {:>8} {}", label, fmt_dur(*d), if *ok { "ok" } else { "FAILED" }); + } + eprintln!(" {:<16} {:>8}", "total", fmt_dur(total)); +} + +pub fn run(root: &Path, args: &[String]) -> Result<(), String> { + let dry = args.iter().any(|a| a == "--dry-run"); + let rev = match args.iter().position(|a| a == "--base") { + Some(i) => Some(args.get(i + 1).map(String::as_str).ok_or("--base needs a revision")?), + None => None, + }; + let (sha, why) = base(root, rev)?; + let files = changed_files(root, &sha)?; + let md = crate::metadata()?; + let pkgs = packages(root, &md, &files); + let plan = plan(&files, &pkgs); + let short_sha = sha.get(..9).unwrap_or(&sha); + println!("ci --affected: base {short_sha} ({why}), {} changed file(s)", files.len()); + if let Some(reason) = &plan.full { + println!(" full CI: {reason}"); + if dry { + return Ok(()); + } + let t = Instant::now(); + let r = crate::ci(); + summary(&[("full ci".into(), t.elapsed(), r.is_ok())]); + return r; + } + println!(" changed: {}", short_list(&plan.direct)); + println!(" affected: {} ({} of {} crates)", short_list(&plan.affected), plan.affected.len(), pkgs.len()); + if !plan.unmapped.is_empty() { + println!(" read by no crate: {}", first_few(&plan.unmapped)); + } + let nextest = Command::new(env!("CARGO")).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); + let steps = steps(&plan, &pkgs, nextest); + let labels: Vec<&str> = steps.iter().map(|s| s.label.as_str()).collect(); + println!(" steps: {}{}", labels.join(", "), if plan.assets { "" } else { " (assets skipped: no asset changes)" }); + if dry { + return Ok(()); + } + let mut times = vec![]; + let mut failed = vec![]; + for mut s in steps { + let t = Instant::now(); + let r = match (s.cmd.as_mut(), s.check) { + (Some(c), _) => crate::run(c), + (None, Some(f)) => f(), + (None, None) => Ok(()), + }; + if let Err(e) = &r { + eprintln!("ci --affected: {} failed: {e}", s.label); + failed.push(s.label.clone()); + } + times.push((s.label, t.elapsed(), r.is_ok())); + } + summary(×); + if failed.is_empty() { Ok(()) } else { Err(format!("ci --affected failed: {}", failed.join(", "))) } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn pkg(name: &str, dir: &str, deps: &[(&str, bool)], inputs: &[&str]) -> Pkg { + Pkg { + name: name.into(), + dir: dir.into(), + deps: deps.iter().map(|(d, dev)| (d.to_string(), *dev)).collect(), + has_lib: true, + has_doctests: true, + inputs: inputs.iter().map(|s| s.to_string()).collect(), + } + } + + /// time ← render ← engine ← ui ← app; host has engine as a dev-dependency; text reads fonts. + fn ws() -> Vec { + vec![ + pkg("effectcraft-time", "crates/time", &[], &[]), + pkg("effectcraft-text", "crates/text", &[], &["assets/fonts"]), + pkg("effectcraft-render", "crates/render", &[("effectcraft-time", false), ("effectcraft-text", false)], &[]), + pkg("effectcraft-engine", "crates/engine", &[("effectcraft-render", false)], &["docs/preferences.md"]), + pkg("effectcraft-host", "crates/host", &[("effectcraft-time", false), ("effectcraft-engine", true)], &["crates/svg/tests/fixtures"]), + pkg("effectcraft-ui-egui", "crates/ui-egui", &[("effectcraft-engine", false)], &[]), + pkg("effectcraft", "apps/effectcraft", &[("effectcraft-ui-egui", false)], &[]), + pkg("effectcraft-cli", "apps/effectcraft-cli", &[("effectcraft", false)], &[]), + pkg("effectcraft-svg", "crates/svg", &[], &[]), + ] + } + + fn set(v: &[&str]) -> BTreeSet { + v.iter().map(|s| s.to_string()).collect() + } + + fn files(v: &[&str]) -> Vec { + v.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn file_maps_to_innermost_package() { + let mut p = ws(); + p.push(pkg("nested", "crates/time/nested", &[], &[])); + assert_eq!(owner("crates/time/src/lib.rs", &p).map(|p| p.name.as_str()), Some("effectcraft-time")); + assert_eq!(owner("crates/time/nested/src/lib.rs", &p).map(|p| p.name.as_str()), Some("nested")); + // a prefix of the dir name is not a match + assert_eq!(owner("crates/timeline/src/lib.rs", &p).map(|p| p.name.as_str()), None); + assert_eq!(owner("apps/effectcraft-cli/src/main.rs", &p).map(|p| p.name.as_str()), Some("effectcraft-cli")); + assert_eq!(owner("apps/effectcraft/src/main.rs", &p).map(|p| p.name.as_str()), Some("effectcraft")); + assert!(owner("docs/testing.md", &p).is_none()); + } + + #[test] + fn reverse_deps_follow_normal_edges_and_one_dev_hop() { + let p = ws(); + // time: render, engine, ui, app, cli via normal edges; host directly; host is not + // a dependency of anything. + let a = expand(&set(&["effectcraft-time"]), &p); + assert_eq!( + a, + set(&["effectcraft-time", "effectcraft-render", "effectcraft-engine", "effectcraft-host", "effectcraft-ui-egui", "effectcraft", "effectcraft-cli"]) + ); + // ui-egui: only its dependents + assert_eq!(expand(&set(&["effectcraft-ui-egui"]), &p), set(&["effectcraft-ui-egui", "effectcraft", "effectcraft-cli"])); + // engine: host only through its dev-dependency, and nothing beyond host + let e = expand(&set(&["effectcraft-engine"]), &p); + assert!(e.contains("effectcraft-host")); + assert!(!e.contains("effectcraft-time") && !e.contains("effectcraft-render")); + assert_eq!(expand(&set(&["effectcraft-cli"]), &p), set(&["effectcraft-cli"])); + assert!(expand(&BTreeSet::new(), &p).is_empty()); + } + + #[test] + fn workspace_inputs_fall_back_to_full_ci() { + let p = ws(); + for f in ["Cargo.toml", "Cargo.lock", ".cargo/config.toml", "xtask/src/main.rs", "rust-toolchain.toml", "clippy.toml"] { + let pl = plan(&files(&["crates/time/src/lib.rs", f]), &p); + assert!(pl.full.is_some(), "{f}"); + } + // a crate manifest is not workspace-wide + assert_eq!(plan(&files(&["crates/time/Cargo.toml"]), &p).full, None); + assert_eq!(full_ci_reason("docs/xtask.md"), None); + } + + #[test] + fn nextest_policy_alone_requires_full_ci() { + let pl = plan(&files(&[".config/nextest.toml"]), &ws()); + assert_eq!(pl.full.as_deref(), Some("nextest configuration changed (.config/nextest.toml)")); + // Only the actual shared test input triggers this fallback, not every config file. + assert_eq!(full_ci_reason(".config/editor.toml"), None); + assert_eq!(full_ci_reason("docs/nextest.toml"), None); + } + + #[test] + fn docs_only_changes_affect_nothing() { + let pl = plan(&files(&["docs/gaps.md", "README.md", ".github/workflows/release.yml"]), &ws()); + assert_eq!(pl.full, None); + assert!(pl.affected.is_empty()); + assert!(!pl.assets); + assert_eq!(pl.unmapped.len(), 3); + } + + #[test] + fn files_read_from_outside_map_to_their_readers() { + let p = ws(); + // docs/preferences.md is read by an engine test + let pl = plan(&files(&["docs/preferences.md"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-engine"])); + assert!(pl.unmapped.is_empty()); + // a font: read by text (directory input), and an asset + let pl = plan(&files(&["assets/fonts/Inter-Regular.ttf"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-text"])); + assert!(pl.assets); + assert!(pl.affected.contains("effectcraft-cli")); + // svg fixtures belong to svg and are read by host + let pl = plan(&files(&["crates/svg/tests/fixtures/basic-shapes.svg"]), &p); + assert_eq!(pl.direct, set(&["effectcraft-svg", "effectcraft-host"])); + } + + #[test] + fn asset_rules() { + assert!(is_asset_related("ATTRIBUTION.md")); + assert!(is_asset_related("crates/engine/scripts/Rename Layers.jsx.attribution")); + assert!(is_asset_related("docs/brand/artcraft-mark.png")); + assert!(!is_asset_related("docs/testing.md")); + assert!(!is_asset_related("crates/time/src/lib.rs")); + } + + #[test] + fn rel_path_extraction() { + assert_eq!(rel_paths(r#"static F: &[u8] = include_bytes!("../../../assets/fonts/Inter-Regular.ttf");"#), ["../../../assets/fonts/Inter-Regular.ttf"]); + assert_eq!(rel_paths(r#"let p = concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/effects.md");"#), ["../../docs/effects.md"]); + assert_eq!(rel_paths(r#"format!("{}/../model/tests/fixtures/{n}", env!("CARGO_MANIFEST_DIR"))"#), ["../model/tests/fixtures"]); + assert_eq!(rel_paths(r#"let a = "x"; let b = "../y/z{n}.png";"#), ["../y"]); + assert!(rel_paths(r#"let s = "no parent here";"#).is_empty()); + } + + #[test] + fn rel_path_joining() { + assert_eq!(join_rel("crates/text/src", "../../../assets/fonts/A.ttf").as_deref(), Some("assets/fonts/A.ttf")); + assert_eq!(join_rel("crates/host", "../model/./tests").as_deref(), Some("crates/model/tests")); + assert_eq!(join_rel("crates", "../../outside"), None); + } + + #[test] + fn inputs_try_file_and_manifest_dirs() { + let existing = set(&["assets/fonts/Inter-Regular.ttf", "docs/effects.md", "crates/model/tests/fixtures", "crates/svg/tests/fixtures/a.svg"]); + let exists = |p: &str| existing.contains(p); + let src = r#" +static F: &[u8] = include_bytes!("../../../assets/fonts/Inter-Regular.ttf"); +let p = concat!(env!("CARGO_MANIFEST_DIR"), "/../../docs/effects.md"); +let fx = format!("{}/../model/tests/fixtures/{n}", env!("CARGO_MANIFEST_DIR")); +let own = include_bytes!("../tests/fixtures/own.svg"); +let out = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/test-out"); +// let commented = include_bytes!("../../svg/tests/fixtures/a.svg"); +"#; + let got = inputs_of("crates/host/src/lib.rs", "crates/host", src, &exists); + assert_eq!(got, set(&["assets/fonts/Inter-Regular.ttf", "docs/effects.md", "crates/model/tests/fixtures"])); + // integration tests in tests/ resolve include_bytes! against tests/ + let got = inputs_of("crates/host/tests/v.rs", "crates/host", r#"include_bytes!("../../svg/tests/fixtures/a.svg")"#, &exists); + assert_eq!(got, set(&["crates/svg/tests/fixtures/a.svg"])); + } + + fn committed_repo() -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + git(root, &["init", "-q"]).unwrap(); + git(root, &["config", "core.quotepath", "true"]).unwrap(); + std::fs::write(root.join("README.md"), "fixture\n").unwrap(); + commit_fixture(root); + dir + } + + fn commit_fixture(root: &Path) { + git(root, &["add", "--all"]).unwrap(); + git(root, &["-c", "user.name=Test", "-c", "user.email=test@example.invalid", "-c", "commit.gpgsign=false", "commit", "--no-verify", "-qm", "fixture"]) + .unwrap(); + } + + #[test] + fn changed_files_preserve_unicode_in_tracked_and_untracked_paths() { + let dir = committed_repo(); + let root = dir.path(); + let tracked = "crates/time/src/caf\u{e9}.rs"; + let untracked = "crates/time/src/\u{65b0}\u{898f}.rs"; + std::fs::create_dir_all(root.join("crates/time/src")).unwrap(); + std::fs::write(root.join(tracked), "before").unwrap(); + commit_fixture(root); + std::fs::write(root.join(tracked), "after").unwrap(); + std::fs::write(root.join(untracked), "new").unwrap(); + let changed = changed_files(root, "HEAD").unwrap(); + assert_eq!(changed, files(&[tracked, untracked])); + assert_eq!(plan(&changed, &ws()).direct, set(&["effectcraft-time"])); + } + + #[cfg(unix)] + #[test] + fn changed_files_preserve_whitespace_quotes_and_backslashes() { + let dir = committed_repo(); + let root = dir.path(); + let tracked = " tracked\t\"\\name\n "; + let untracked = " untracked\t\"\\name\n "; + std::fs::write(root.join(tracked), "before").unwrap(); + commit_fixture(root); + std::fs::write(root.join(tracked), "after").unwrap(); + std::fs::write(root.join(untracked), "new").unwrap(); + assert_eq!(changed_files(root, "HEAD").unwrap(), files(&[tracked, untracked])); + } + + // macOS (APFS) refuses file names that aren't UTF-8 ("Illegal byte sequence"), so there the + // fixture can't be made and such a change can't happen. + #[cfg(all(unix, not(target_os = "macos")))] + #[test] + fn changed_files_reject_non_utf8_instead_of_silently_omitting_it() { + use std::os::unix::ffi::OsStringExt; + let dir = committed_repo(); + std::fs::write(dir.path().join(std::ffi::OsString::from_vec(b"invalid-\xff.rs".to_vec())), "new").unwrap(); + assert!(changed_files(dir.path(), "HEAD").unwrap_err().contains("not UTF-8")); + } + + #[test] + fn deleted_and_renamed_shared_inputs_still_select_their_readers() { + for rename in [false, true] { + let dir = committed_repo(); + let root = dir.path(); + std::fs::create_dir_all(root.join("crates/engine/src")).unwrap(); + std::fs::create_dir_all(root.join("docs/fixtures")).unwrap(); + std::fs::write(root.join("docs/preferences.md"), "preferences").unwrap(); + std::fs::write(root.join("docs/fixtures/example.json"), "{}").unwrap(); + std::fs::write( + root.join("crates/engine/src/lib.rs"), + r#" +const PREFS: &str = include_str!("../../../docs/preferences.md"); +fn fixture(n: &str) -> String { format!("{}/../../docs/fixtures/{n}", env!("CARGO_MANIFEST_DIR")) } +"#, + ) + .unwrap(); + commit_fixture(root); + if rename { + std::fs::rename(root.join("docs/preferences.md"), root.join("docs/renamed.md")).unwrap(); + std::fs::rename(root.join("docs/fixtures"), root.join("docs/moved")).unwrap(); + } else { + std::fs::remove_file(root.join("docs/preferences.md")).unwrap(); + std::fs::remove_dir_all(root.join("docs/fixtures")).unwrap(); + } + let changed = changed_files(root, "HEAD").unwrap(); + let md = serde_json::json!({ + "workspace_root": root, + "packages": [{ + "name": "effectcraft-engine", + "manifest_path": root.join("crates/engine/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": true}], + "dependencies": [] + }] + }); + let pkgs = packages(root, &md, &changed); + let engine = pkgs.first().unwrap(); + assert!(engine.inputs.contains(&"docs/preferences.md".into())); + assert!(engine.inputs.contains(&"docs/fixtures".into())); + for deleted in ["docs/preferences.md", "docs/fixtures/example.json"] { + assert!(changed.contains(&deleted.to_string())); + assert_eq!(plan(&files(&[deleted]), &pkgs).direct, set(&["effectcraft-engine"])); + } + } + } + + #[test] + fn test_runner_keeps_dev_profile_target_selection_and_gpu_isolation() { + for packages in [vec![], files(&["effectcraft-gpu", "effectcraft-ui-egui"])] { + for nextest in [false, true] { + let command = crate::test_command(&packages, nextest, true); + let args: Vec<_> = command.get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + assert!(!args.iter().any(|a| a == "--release" || a == "--profile")); + for target in ["--lib", "--bins", "--tests", "--no-fail-fast"] { + assert!(args.iter().any(|a| a == target)); + } + assert_eq!(args.iter().any(|a| a == "--workspace"), packages.is_empty()); + for package in &packages { + assert!(args.windows(2).any(|w| w[0] == "-p" && w[1] == *package)); + } + assert_eq!(args.windows(2).any(|w| w == ["-E", "not package(effectcraft-gpu)"]), nextest); + assert_eq!(args.iter().any(|a| a == "--no-tests=pass"), nextest && !packages.is_empty()); + } + } + } + + #[test] + fn full_workspace_test_commands_keep_library_tests() { + for nextest in [false, true] { + let command = crate::test_command(&[], nextest, false); + assert!(command.get_args().any(|a| a == "--workspace")); + assert!(command.get_args().any(|a| a == "--lib")); + } + } + + #[test] + fn bin_only_and_doctest_disabled_packages_select_the_right_test_targets() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let md = serde_json::json!({ + "workspace_root": root, + "packages": [ + { + "name": "effectcraft-cli", + "manifest_path": root.join("apps/effectcraft-cli/Cargo.toml"), + "targets": [{"kind": ["bin"], "doctest": false}], + "dependencies": [] + }, + { + "name": "effectcraft-time", + "manifest_path": root.join("crates/time/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": false}], + "dependencies": [] + }, + { + "name": "effectcraft-color", + "manifest_path": root.join("crates/color/Cargo.toml"), + "targets": [{"kind": ["lib"], "doctest": true}], + "dependencies": [] + } + ] + }); + let pkgs = packages(root, &md, &[]); + for (name, changed, want_lib, want_doctests) in [ + ("effectcraft-cli", "apps/effectcraft-cli/src/main.rs", false, false), + ("effectcraft-time", "crates/time/src/lib.rs", true, false), + ("effectcraft-color", "crates/color/src/lib.rs", true, true), + ] { + let plan = plan(&files(&[changed]), &pkgs); + assert_eq!(plan.affected, set(&[name])); + let (has_lib, doctests) = test_targets(&plan, &pkgs); + assert_eq!((has_lib, doctests), (want_lib, want_doctests)); + let selected = plan.affected.into_iter().collect::>(); + for nextest in [false, true] { + let command = crate::test_command(&selected, nextest, has_lib); + let args: Vec<_> = command.get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + assert_eq!(args.iter().any(|a| a == "--lib"), want_lib); + assert!(args.windows(2).any(|w| w == ["-p", name])); + for target in ["--bins", "--tests", "--no-fail-fast"] { + assert!(args.iter().any(|a| a == target)); + } + } + } + } + + fn labels(s: &[Step]) -> Vec { + s.iter().map(|s| s.label.clone()).collect() + } + + #[test] + fn steps_follow_the_plan() { + let mut p = ws(); + if let Some(cli) = p.iter_mut().find(|p| p.name == "effectcraft-cli") { + cli.has_lib = false; + cli.has_doctests = false; + } + // docs only: cheap checks + assert_eq!(labels(&steps(&plan(&files(&["docs/gaps.md"]), &p), &p, true)), ["fmt", "pdf-deps", "lock-patches", "layers"]); + // ui-egui (L5, a web crate) and its two app dependents; cli has no doctests + let s = steps(&plan(&files(&["crates/ui-egui/src/dock.rs"]), &p), &p, true); + assert_eq!(labels(&s), ["fmt", "audio backend", "clippy (3)", "tests (3)", "pdf-deps", "lock-patches", "layers", "wasm (1)"]); + let args = |i: usize| s.get(i).and_then(|s| s.cmd.as_ref()).map(|c| c.get_args().map(|a| a.to_string_lossy().into_owned()).collect::>()); + let clippy = args(2).unwrap_or_default(); + assert!(!clippy.iter().any(|a| a == "--release" || a == "--profile")); + assert!(clippy.windows(2).any(|w| w == ["-p", "effectcraft-ui-egui"])); + assert!(clippy.ends_with(&["--".into(), "-D".into(), "warnings".into()])); + assert!(args(7).unwrap_or_default().windows(2).any(|w| w == ["-p", "effectcraft-ui-egui"])); + // without nextest: one cargo test step; an asset change adds the assets check + let s = steps(&plan(&files(&["assets/fonts/A.ttf"]), &p), &p, false); + assert!(labels(&s).contains(&"tests (7)".to_string()) && labels(&s).contains(&"assets".to_string())); + } +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 06d32e51a..16e28fa07 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -13,11 +13,14 @@ //! `crates/aep/tests/fixtures/corpus.lock`, verify every file's SHA-256, install into `corpus/aep`. //! - `ico …`: pack PNGs into a Windows `.ico` (used by `packaging/icons.sh`). //! - `ci`: fmt check, clippy -D warnings, tests (`cargo nextest run` when installed) and doctests, -//! PDF dependency policy, layers, assets, wasm. Everything builds in the dev profile. +//! PDF dependency policy, lock patches, layers, assets, wasm. Everything builds in the dev profile. +//! - `ci --affected [--base REV] [--dry-run]`: the same gates for only the crates the change since +//! REV (default: the merge-base with `main`) can affect; see `affected.rs` and docs/testing.md. //! - `bench [--bin CLI] [--quick] [--compare OLD.jsonl] …`: render benchmark of generated scenarios //! (memory, CPU, wall time) into `target/bench/results.jsonl` (`bench.rs`, docs/testing.md). mod aep_corpus; +mod affected; mod bench; mod ico; mod lock_patches; @@ -480,34 +483,73 @@ fn audio_backend() -> Result<(), String> { /// /// Each run goes ahead when an earlier one fails; the error names every run that failed. fn tests() -> Result<(), String> { + let nextest = Command::new(env!("CARGO")).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); + tests_for(&[], nextest, true, true) +} + +/// Keep the full and affected gates on the same test runner, including GPU isolation and the +/// translation report. An empty package list selects the whole workspace. +fn tests_for(packages: &[String], nextest: bool, has_lib: bool, doctests: bool) -> Result<(), String> { let cargo = env!("CARGO"); - let targets = ["--workspace", "--lib", "--bins", "--tests", "--no-fail-fast"]; - let nextest = Command::new(cargo).args(["nextest", "--version"]).output().is_ok_and(|o| o.status.success()); - let mut runs = Vec::new(); - if nextest { - runs.push(run(Command::new(cargo).args(["nextest", "run"]).args(targets).args(["-E", "not package(effectcraft-gpu)"]))); - // effectcraft-gpu's tests share one device, so they run in one libtest process: the - // binary nextest just built (`cargo test -p effectcraft-gpu` would rebuild ~2 minutes). - runs.push(lib_test_binary("effectcraft-gpu").and_then(|mut c| run(&mut c))); - } else { + let selection = test_selection(packages); + let includes = |name: &str| packages.is_empty() || packages.iter().any(|p| p == name); + if !nextest { println!("cargo-nextest not found, running cargo test (install: cargo install cargo-nextest --locked)"); - runs.push(run(Command::new(cargo).arg("test").args(targets))); + } + let mut runs = vec![run(&mut test_command(packages, nextest, has_lib))]; + if nextest && includes("effectcraft-gpu") { + // GPU tests share one device, so reuse the selected build in one libtest process. + runs.push(lib_test_binary("effectcraft-gpu", &selection).and_then(|mut c| run(&mut c))); } // The test runs above hide a passing test's output, so the always-passing report runs again // with --nocapture, from the binary already built. - runs.push(lib_test_binary("effectcraft-ui-egui").and_then(|mut c| run(c.args(["missing_translations_report", "--nocapture"])))); - runs.push(run(Command::new(cargo).args(["test", "--workspace", "--doc", "--no-fail-fast"]))); + if includes("effectcraft-ui-egui") { + runs.push(lib_test_binary("effectcraft-ui-egui", &selection).and_then(|mut c| run(c.args(["missing_translations_report", "--nocapture"])))); + } + if doctests { + runs.push(run(Command::new(cargo).arg("test").args(&selection).args(["--doc", "--no-fail-fast"]))); + } let failed: Vec = runs.into_iter().filter_map(Result::err).collect(); if failed.is_empty() { Ok(()) } else { Err(failed.join("; ")) } } +/// Select library tests only when the selected packages contain a library; bin-only package +/// sets otherwise fail before running any tests. The full workspace always has library targets. +fn test_command(packages: &[String], nextest: bool, has_lib: bool) -> Command { + let mut c = Command::new(env!("CARGO")); + if nextest { + c.args(["nextest", "run"]); + // A selected package can have no tests, or only GPU tests that run separately. + if !packages.is_empty() { + c.arg("--no-tests=pass"); + } + } else { + c.arg("test"); + } + c.args(test_selection(packages)); + if packages.is_empty() || has_lib { + c.arg("--lib"); + } + c.args(["--bins", "--tests", "--no-fail-fast"]); + if nextest { + c.args(["-E", "not package(effectcraft-gpu)"]); + } + c +} + +fn test_selection(packages: &[String]) -> Vec { + if packages.is_empty() { vec!["--workspace".into()] } else { packages.iter().flat_map(|p| ["-p".into(), p.clone()]).collect() } +} + /// A command running `package`'s lib test binary from the test build above, the way `cargo test` -/// runs it (from the package directory), found with `--no-run` and the same workspace selection +/// runs it (from the package directory), found with `--no-run` and the same package selection /// so nothing rebuilds: `cargo test -p ` resolves features for that package alone and /// recompiles much of the workspace. -fn lib_test_binary(package: &str) -> Result { +fn lib_test_binary(package: &str, selection: &[String]) -> Result { let out = Command::new(env!("CARGO")) - .args(["test", "--workspace", "--lib", "--bins", "--tests", "--no-run", "--message-format=json-render-diagnostics"]) + .arg("test") + .args(selection) + .args(["--lib", "--bins", "--tests", "--no-run", "--message-format=json-render-diagnostics"]) .stderr(std::process::Stdio::inherit()) .output() .map_err(|e| format!("cargo test --no-run: {e}"))?; @@ -560,7 +602,10 @@ fn main() -> ExitCode { "pdf-deps" => pdf_deps::run(&root()), "lock-patches" => lock_patches::run(&root()), "aep-corpus" => aep_corpus::run(&root(), &std::env::args().skip(2).collect::>()), - "ci" => ci(), + "ci" => { + let rest: Vec = std::env::args().skip(2).collect(); + if rest.iter().any(|a| a == "--affected") { affected::run(&root(), &rest) } else { ci() } + } "audio-backend" => audio_backend(), "web" => web(&std::env::args().skip(2).collect::>()), "bench" => bench::run(&root(), &std::env::args().skip(2).collect::>()), @@ -572,7 +617,7 @@ fn main() -> ExitCode { let rest: Vec = std::env::args().skip(2).collect(); version::run(&root(), &rest.iter().map(String::as_str).collect::>()) } - _ => Err("usage: cargo xtask ".into()), + _ => Err("usage: cargo xtask ".into()), }; match r { Ok(()) => ExitCode::SUCCESS, @@ -585,7 +630,7 @@ fn main() -> ExitCode { /// The workspace root (the parent of `xtask/`). pub fn root() -> std::path::PathBuf { - std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().expect("xtask has a parent dir").to_path_buf() + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().map(std::path::Path::to_path_buf).unwrap_or_else(|| std::path::PathBuf::from(".")) } #[cfg(all(test, target_os = "linux"))]