From 9d33c200d5df7bcd6df808b4ddd12e9234d3521d Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:00:46 +0000 Subject: [PATCH 1/8] #1743: protect render sources and refresh proxies safely --- .../workflows/render-source-protection.yml | 34 ++ Cargo.lock | 1 + Cargo.toml | 1 + crates/engine/Cargo.toml | 4 + crates/engine/src/commands/comp_more.rs | 7 +- crates/engine/src/commands/proxy.rs | 2 + crates/engine/src/commands/render_queue.rs | 9 + crates/engine/src/lib.rs | 3 + crates/engine/src/output_guard.rs | 435 +++++++++++++ crates/engine/src/render_queue.rs | 95 +-- crates/engine/src/tests_render_overwrite.rs | 571 ++++++++++++++++++ crates/project/src/render_queue.rs | 80 ++- docs/agents.md | 4 + docs/render-source-protection.md | 26 + 14 files changed, 1230 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/render-source-protection.yml create mode 100644 crates/engine/src/output_guard.rs create mode 100644 crates/engine/src/tests_render_overwrite.rs create mode 100644 docs/render-source-protection.md diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml new file mode 100644 index 000000000..3c040d831 --- /dev/null +++ b/.github/workflows/render-source-protection.yml @@ -0,0 +1,34 @@ +name: Render source protection + +on: + pull_request: + paths: + - .github/workflows/render-source-protection.yml + - Cargo.toml + - Cargo.lock + - crates/engine/** + - crates/project/src/render_queue.rs + workflow_dispatch: + +permissions: + contents: read + +jobs: + regression: + name: Source guard (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + env: + CARGO_PROFILE_DEV_DEBUG: 0 + CARGO_PROFILE_TEST_DEBUG: 0 + CARGO_INCREMENTAL: 0 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@1.95.0 + - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests + - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite + - run: cargo test --locked -p effectcraft-project render_queue::tests diff --git a/Cargo.lock b/Cargo.lock index f99e09a06..2f9163471 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1285,6 +1285,7 @@ dependencies = [ "kurbo", "log", "rayon", + "same-file", "serde", "serde_json", "thiserror 2.0.21", diff --git a/Cargo.toml b/Cargo.toml index a3935f047..bf3ac938f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ skrifa = { version = "0.44", default-features = false, features = ["std"] } harfrust = { version = "0.12", default-features = false, features = ["std"] } exr = "1.74" image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } +same-file = "1.0" # media codecs + containers: FilmCraft's pure-Rust crates, pinned to a revision (plan/adr/0001). # Only `crates/media`, `crates/export` and `crates/interchange` may depend on these. diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 7104bc1cc..4bb488569 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -39,3 +39,7 @@ rayon = { workspace = true } image = { workspace = true } web-time = { workspace = true } exr = { workspace = true } + +# Native only: the volume's own identity for a file, to tell that a hard link is the footage it leads to. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +same-file = { workspace = true } diff --git a/crates/engine/src/commands/comp_more.rs b/crates/engine/src/commands/comp_more.rs index d6de320ec..fca36e670 100644 --- a/crates/engine/src/commands/comp_more.rs +++ b/crates/engine/src/commands/comp_more.rs @@ -93,8 +93,11 @@ fn queue_frame(s: &mut Session, p: &Value) -> Result { it.output = s.project.render_templates.default_output_module(TemplateSlot::Still); if let Some(path) = str_p(p, "path") { it.output.output = path.to_string(); - } else if !it.output.format.is_sequence() { - it.output.output = "[compName]_frame.[fileExtension]".into(); + } else { + if !it.output.format.is_sequence() { + it.output.output = "[compName]_frame.[fileExtension]".into(); + } + s.avoid_sources(&mut it); } let id = s.edit("Save Frame As", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; diff --git a/crates/engine/src/commands/proxy.rs b/crates/engine/src/commands/proxy.rs index b7d644b94..dfa259618 100644 --- a/crates/engine/src/commands/proxy.rs +++ b/crates/engine/src/commands/proxy.rs @@ -190,6 +190,8 @@ fn create_proxy(s: &mut Session, p: &Value) -> Result { it.output.channels = Channels::Rgba; if let Some(o) = str_p(p, "path").or(str_p(p, "output")) { it.output.output = o.to_string(); + } else { + s.avoid_sources(&mut it); } it.post_render = PostRenderAction::SetProxy; let id = s.edit("Create Proxy", None, |proj, _| { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index 09a7c7d2f..d7e914b1a 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -532,6 +532,9 @@ fn add(s: &mut Session, p: &Value) -> Result { object.remove("template"); } apply_output(&mut it.output, &templates, s.state.region_of_interest, &output_params, "renderQueue.add")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } if let Some(l) = enum_p(p, "log", "renderQueue.add", RenderLog::parse, "errorsOnly|plusSettings|plusPerFrameInfo")? { it.log = l; } @@ -770,6 +773,9 @@ fn pre_render(s: &mut Session, p: &Value) -> Result { it.output = templates.default_output_module(TemplateSlot::PreRender); it.output.output = "[compName]_prerender.[fileExtension]".into(); apply_output(&mut it.output, &templates, s.state.region_of_interest, p, "render.preRender")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } it.post_render = PostRenderAction::ImportAndReplace; let id = s.edit("Pre-render", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; @@ -795,6 +801,9 @@ fn save_current_preview(s: &mut Session, p: &Value) -> Result { let mut item = RenderQueueItem::new(0, cid); item.output = OutputModule::for_format(format); item.output.output = path.clone(); + if let Some(refusal) = s.check_outputs(&[vec![(item.clone(), path.clone())]]).first() { + return Err(EngineError::Other(refusal.reason.clone())); + } let job = crate::ExportJob { project: &s.project, footage: s.footage.as_ref(), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 382534a44..3f703129b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -30,6 +30,7 @@ pub mod media_cache; pub mod menus; pub mod models; pub mod offload; +mod output_guard; pub mod perf; pub mod prefs; pub mod preview; @@ -1333,6 +1334,8 @@ mod tests_proxy; #[cfg(test)] mod tests_quick_offset; #[cfg(test)] +mod tests_render_overwrite; +#[cfg(test)] mod tests_replace_source; #[cfg(test)] mod tests_rig3d; diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs new file mode 100644 index 000000000..5d4dc289e --- /dev/null +++ b/crates/engine/src/output_guard.rs @@ -0,0 +1,435 @@ +//! Protect files the project reads from render outputs (#1743). +//! +//! This guard is independent of replacement consent and export staging: it never authorizes +//! overwriting ordinary outputs or opens a second confirmation flow. Those belong to the +//! exporter. Existing aliases are compared by native file identity, including hard links. +//! Missing sources remain protected by their normalized path. Checks happen before dispatch; +//! filesystem changes between the check and the write require exporter-side revalidation. + +use std::collections::{HashMap, HashSet}; +use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; + +use effectcraft_project::render_queue::{RenderQueueItem, log_path, numbered_template, sequence_frame, sequence_path}; +use effectcraft_project::{Footage, ItemKind}; + +use crate::Session; + +/// Volumes on Windows and macOS ignore case unless they were set up not to. +const IGNORES_CASE: bool = cfg!(any(windows, target_os = "macos")); + +/// How many frames are looked at one by one for a sequence whose folder cannot be read at once +/// (its frame number is in a folder's name). +const PROBE_LIMIT: u64 = 100_000; + +/// How far a default output name is numbered (`_1` … `_1000`) before it is left to the check at +/// render time. +const MAX_NUMBER: u32 = 1_000; + +/// What reads a file the project depends on. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Source { + /// A footage item, by name. + Footage(Arc), + /// An item's proxy, by the item's name. + Proxy(Arc), + /// The project file. + Project, +} + +impl Source { + /// Why `path`, a file this reads, cannot be a render's output. + pub(crate) fn refusal(&self, path: &str) -> String { + match self { + Source::Footage(name) => format!("{path} is the footage “{name}” of this project, and a render never replaces footage; choose another Output To"), + Source::Proxy(name) => format!("{path} is the proxy of “{name}”, and a render never replaces a proxy; choose another Output To"), + Source::Project => format!("{path} is this project's own file, and a render never replaces it; choose another Output To"), + } + } +} + +/// The files the project reads: footage (every frame of a sequence), proxies and the project file. +#[derive(Default)] +pub(crate) struct Protected { + files: Vec<(PathBuf, Source)>, + by_path: HashMap, + /// Hashes of native identities, built once. Handles are closed after indexing so long + /// sequences do not exhaust the process's file descriptors. Hash hits are verified with + /// native handle equality; a hash collision cannot make another file a protected source. + #[cfg(not(target_arch = "wasm32"))] + by_identity: std::sync::OnceLock>>, + #[cfg(all(test, not(target_arch = "wasm32")))] + identity_reads: std::sync::atomic::AtomicUsize, +} + +impl Protected { + pub(crate) fn of(s: &Session) -> Protected { + let mut found = Protected::default(); + for item in s.project.items.values() { + let name: Arc = item.name.as_str().into(); + if let ItemKind::Footage(f) = &item.kind { + found.add_footage(s, f, &Source::Footage(name.clone())); + } + if let Some(proxy) = &item.proxy { + found.add_footage(s, &proxy.footage, &Source::Proxy(name)); + } + } + if let Some(path) = s.path.as_deref().filter(|p| !p.is_empty()) { + found.add(PathBuf::from(path), Source::Project); + } + found + } + + fn add_footage(&mut self, s: &Session, f: &Footage, source: &Source) { + for stored in std::iter::once(&f.path).chain(&f.sequence).filter(|p| !p.is_empty()) { + // The file the media layer opens for the path the project stores. + let path = s.stored_path(stored).unwrap_or_else(|_| PathBuf::from(stored)); + self.add(path, source.clone()); + } + } + + fn add(&mut self, path: PathBuf, source: Source) { + self.by_path.entry(key(&path)).or_insert(self.files.len()); + self.files.push((path, source)); + } + + pub(crate) fn is_empty(&self) -> bool { + self.files.is_empty() + } + + /// The source at this normalized path or native file identity. A new output only needs + /// its path lookup and one failed filesystem probe; existing outputs build the index once. + pub(crate) fn find(&self, path: &Path) -> Option<&Source> { + self.by_path.get(&key(path)).and_then(|i| self.files.get(*i)).map(|(_, source)| source).or_else(|| self.alias_of(path)) + } + + #[cfg(not(target_arch = "wasm32"))] + fn handle(&self, path: &Path) -> Option { + #[cfg(test)] + self.identity_reads.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + // Opening a FIFO can wait for a writer. Only regular files can be protected media; + // preserve the old size-indexed lookup's exclusion of special files and directories. + if !std::fs::metadata(path).ok()?.is_file() { + return None; + } + same_file::Handle::from_path(path).ok() + } + + /// Hashes narrow the search, while native handle equality also checks any hash collisions. + #[cfg(not(target_arch = "wasm32"))] + fn alias_of(&self, path: &Path) -> Option<&Source> { + let output = self.handle(path)?; + let indexed = self.by_identity.get_or_init(|| { + let mut ids: HashMap> = HashMap::new(); + for (i, (known, _)) in self.files.iter().enumerate() { + if let Some(handle) = self.handle(known) { + ids.entry(identity_hash(&handle)).or_default().push(i); + } + } + ids + }); + indexed.get(&identity_hash(&output))?.iter().find_map(|i| { + let (known, source) = self.files.get(*i)?; + (self.handle(known)? == output).then_some(source) + }) + } + + #[cfg(target_arch = "wasm32")] + fn alias_of(&self, _: &Path) -> Option<&Source> { + None + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn identity_hash(handle: &same_file::Handle) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hash = std::collections::hash_map::DefaultHasher::new(); + handle.hash(&mut hash); + hash.finish() +} + +/// `path` made absolute, with `.` and `..` resolved by name alone (no file is looked at). +fn lexical(path: &Path) -> PathBuf { + let absolute = std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()); + let mut out = PathBuf::new(); + for part in absolute.components() { + match part { + Component::CurDir => {} + Component::ParentDir => match out.components().next_back() { + Some(Component::Normal(_)) => { + out.pop(); + } + Some(Component::RootDir | Component::Prefix(_)) => {} + _ => out.push(".."), + }, + other => out.push(other.as_os_str()), + } + } + out +} + +/// The name a path is compared by: absolute, resolved by name, without case where volumes ignore it. +fn key(path: &Path) -> String { + let parts: Vec = lexical(path).components().map(|c| c.as_os_str().to_string_lossy().into_owned()).collect(); + fold(&parts.join("/")) +} + +fn fold(name: &str) -> String { + if IGNORES_CASE { name.to_lowercase() } else { name.to_string() } +} + +#[cfg(test)] +fn same_file(a: &Path, b: &Path) -> bool { + if key(a) == key(b) { + return true; + } + #[cfg(not(target_arch = "wasm32"))] + return same_file::is_same_file(a, b).unwrap_or(false); + #[cfg(target_arch = "wasm32")] + false +} + +/// Whether a file (or a link, even one leading nowhere) is at `path`. +fn exists(path: &str) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|m| !m.is_dir()) +} + +/// The files of a numbered sequence (`count` frames from `first`) that are on disk: its folder is +/// read once, not asked about frame by frame. +fn frames_on_disk(path: &str, first: i64, count: u64) -> Result, String> { + let last = first.saturating_add(i64::try_from(count).unwrap_or(i64::MAX)); + let p = Path::new(path); + let dir = p.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); + let name = p.file_name().map(|n| n.to_string_lossy().into_owned()); + let Some(name) = name.filter(|_| !dir.to_string_lossy().contains('#')) else { + // The frame number is in a folder's name: every frame has a folder of its own. + if count > PROBE_LIMIT { + return Err(format!( + "cannot safely check {count} output frames with a number in a folder name (limit {PROBE_LIMIT}); choose a fixed output folder or a shorter range" + )); + } + return Ok((0..count) + .map(|k| sequence_path(path, first.saturating_add(i64::try_from(k).unwrap_or(i64::MAX)))) + .filter(|frame| exists(frame)) + .map(PathBuf::from) + .collect()); + }; + let pattern = fold(&name); + let entries = match std::fs::read_dir(dir) { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(format!("cannot check render output folder {}: {e}", dir.display())), + }; + Ok(entries + .filter_map(Result::ok) + .filter(|e| sequence_frame(&pattern, &fold(&e.file_name().to_string_lossy())).is_some_and(|frame| (first..last).contains(&frame))) + .filter(|e| !e.file_type().is_ok_and(|t| t.is_dir())) + .map(|e| e.path()) + .collect()) +} + +/// A queue item refused before any of its output modules are dispatched. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct Refused { + pub item: u64, + pub reason: String, +} + +impl Session { + /// The files the project reads. + pub(crate) fn protected_files(&self) -> Protected { + Protected::of(self) + } + + /// Reject protected outputs and ranges that cannot be checked completely. Ordinary-file + /// replacement consent is left to the exporter, independently of this source guard. + pub(crate) fn check_outputs(&self, items: &[Vec<(RenderQueueItem, String)>]) -> Vec { + if cfg!(target_arch = "wasm32") { + return Vec::new(); + } + let protected = self.protected_files(); + let mut seen = HashSet::new(); + let mut refused = Vec::new(); + for (item, path) in items.iter().flatten() { + if seen.contains(&item.id) { + continue; + } + let reason = match self.check_output(&protected, item, path) { + Ok(reason) => reason, + Err(reason) => Some(reason), + }; + if let Some(reason) = reason { + seen.insert(item.id); + refused.push(Refused { item: item.id, reason }); + } + } + refused + } + + fn check_output(&self, protected: &Protected, item: &RenderQueueItem, path: &str) -> Result, String> { + if protected.is_empty() { + return Ok(None); + } + let sequence = item.output.format.is_sequence(); + // Skip Existing Files leaves existing frames untouched, but not the render log. + let frames = if sequence && !item.settings.skip_existing { + match self.project.comp(item.comp) { + Some(comp) => frames_on_disk(path, item.settings.first_frame(comp), item.settings.frame_count(comp))?, + None => Vec::new(), + } + } else { + Vec::new() + }; + let log = log_path(path); + let movie = (!sequence).then_some(Path::new(path)); + Ok(frames + .iter() + .map(PathBuf::as_path) + .chain(movie) + .chain(Some(Path::new(&log))) + .find_map(|path| protected.find(path).map(|source| source.refusal(&path.to_string_lossy())))) + } + + /// A new item's default output must not be a file the project reads: `[compName]_1`, + /// `[compName]_2`… take its place (and avoid the queue's other outputs), as After Effects + /// numbers a second item of the same composition. An output nothing is in the way of keeps + /// the composition's own name, and one the caller chose is not touched: rendering it is + /// refused instead. + pub(crate) fn avoid_sources(&self, item: &mut RenderQueueItem) { + if cfg!(target_arch = "wasm32") { + return; + } + let protected = self.protected_files(); + if protected.is_empty() { + return; + } + let template = item.output.output.clone(); + let mut taken: Option> = None; + 'candidate: for n in 0..=MAX_NUMBER { + item.output.output = if n == 0 { template.clone() } else { numbered_template(&template, n) }; + let Ok(outputs) = self.outputs_of(item, None) else { break }; + for (module, path) in &outputs { + match self.check_output(&protected, module, path) { + Ok(Some(_)) => continue 'candidate, + Err(_) => break 'candidate, + Ok(None) => {} + } + } + // Only a name that had to move is kept clear of the other items. + let others = if n == 0 { None } else { Some(taken.get_or_insert_with(|| self.queue_output_keys())) }; + if others.is_none_or(|others| !outputs.iter().any(|(_, path)| others.contains(&key(Path::new(path))))) { + return; + } + } + item.output.output = template; + } + + /// [`key`] of every output of the queue's items. + fn queue_output_keys(&self) -> HashSet { + self.project.render_queue.iter().filter_map(|it| self.outputs_of(it, None).ok()).flatten().map(|(_, path)| key(Path::new(&path))).collect() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn unrelated_same_size_frames_are_indexed_once() { + let dir = std::env::temp_dir().join(format!("effectcraft-identity-index-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut protected = Protected::default(); + for i in 0..128 { + let source = dir.join(format!("source{i}")).join("plate.png"); + let output = dir.join(format!("output{i}")).join("plate.png"); + for path in [&source, &output] { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, b"same-size frame").unwrap(); + } + protected.add(source, Source::Footage(format!("plate{i}").into())); + } + for i in 0..128 { + assert!(protected.find(&dir.join(format!("output{i}")).join("plate.png")).is_none()); + } + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 256, "one source-index pass plus one lookup per output"); + let alias = dir.join("renamed.png"); + std::fs::hard_link(dir.join("source64").join("plate.png"), &alias).unwrap(); + assert_eq!(protected.find(&alias), Some(&Source::Footage("plate64".into()))); + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 258, "native equality verifies a matching identity"); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn folder_numbered_sequences_are_never_partially_checked() { + let pattern = std::env::temp_dir().join("frames[######]").join("plate.png").to_string_lossy().into_owned(); + let error = frames_on_disk(&pattern, 0, PROBE_LIMIT + 1).unwrap_err(); + assert!(error.contains("cannot safely check") && error.contains("100000"), "{error}"); + assert!(frames_on_disk(&pattern, 0, u64::MAX).is_err()); + } + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn non_file_outputs_do_not_build_the_identity_index() { + let protected = Protected::default(); + assert!(protected.find(&std::env::temp_dir()).is_none()); + assert!(protected.by_identity.get().is_none()); + } + + #[test] + fn paths_compare_by_name_once_resolved() { + let here = std::env::current_dir().unwrap(); + assert_eq!(key(Path::new("a/b/../c.mp4")), key(Path::new("a/c.mp4"))); + assert_eq!(key(Path::new("./a/c.mp4")), key(&here.join("a/c.mp4"))); + assert_ne!(key(Path::new("a/c.mp4")), key(Path::new("b/c.mp4"))); + assert_eq!(key(Path::new(&format!("{}x", "../".repeat(64)))), key(Path::new(&format!("{}x", "../".repeat(80)))), "past the root stays at the root"); + assert_eq!(IGNORES_CASE, key(Path::new("A/C.MP4")) == key(Path::new("a/c.mp4"))); + } + + #[test] + fn a_file_is_the_same_file_under_every_spelling_the_volume_allows() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-guard-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + std::fs::write(dir.join("clip.mov"), b"x").unwrap(); + std::fs::write(dir.join("other.mov"), b"y").unwrap(); + assert!(same_file(&dir.join("clip.mov"), &dir.join("sub").join("..").join("clip.mov"))); + assert!(!same_file(&dir.join("clip.mov"), &dir.join("other.mov"))); + // Nothing there yet: only the name decides. + assert!(same_file(&dir.join("gone.mov"), &dir.join("sub").join("..").join("gone.mov"))); + assert!(!same_file(&dir.join("gone.mov"), &dir.join("clip.mov"))); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_sequences_folder_is_read_once_for_its_frames() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-frames-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("a_00003.png")).unwrap(); // a folder is not a frame + for name in ["a_00001.png", "a_00002.png", "a_0004.png", "a_00004.jpg", "b_00002.png", "a_00009.png"] { + std::fs::write(dir.join(name), b"x").unwrap(); + } + let pattern = dir.join("a_[#####].png").to_string_lossy().into_owned(); + let names = |frames: Vec| { + let mut names: Vec = frames.iter().filter_map(|f| f.file_name()).map(|n| n.to_string_lossy().into_owned()).collect(); + names.sort(); + names + }; + assert_eq!( + names(frames_on_disk(&pattern, 0, 5).unwrap()), + ["a_00001.png", "a_00002.png"], + "frames 0-4: not the 4-digit one, the jpg, the folder or frame 9" + ); + assert_eq!(names(frames_on_disk(&pattern, 2, 1).unwrap()), ["a_00002.png"]); + assert_eq!(names(frames_on_disk(&pattern, 9, 100).unwrap()), ["a_00009.png"]); + assert_eq!(names(frames_on_disk(&pattern, 0, u64::MAX).unwrap()), ["a_00001.png", "a_00002.png", "a_00009.png"], "a hostile frame count is not walked"); + // A frame number in a folder's name: each frame has its own folder. + std::fs::create_dir_all(dir.join("f02")).unwrap(); + std::fs::write(dir.join("f02").join("x.png"), b"x").unwrap(); + let in_folder = dir.join("f[##]").join("x.png").to_string_lossy().into_owned(); + assert_eq!(names(frames_on_disk(&in_folder, 0, 4).unwrap()), ["x.png"]); + // A folder that is not there holds nothing. + assert!(frames_on_disk(&dir.join("none").join("a_[#####].png").to_string_lossy(), 0, 5).unwrap().is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 74b333151..f36b500cc 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -432,10 +432,46 @@ impl Session { self.render_job.as_ref().map(|j| j.shared.snapshot()) } - /// Start rendering every queued item. `wait`: block until done (otherwise a background thread - /// renders, or the [`Session::offload`] worker; call [`Session::poll_render`] regularly). - /// Returns the ids being rendered. Without threads or an offload (wasm32) the render always - /// runs to completion before this returns. + /// The (item, resolved output path) of each output of `it`: one per output module and, with + /// Segment Sequences / Segment Movie Files, per segment. `formats`: the ones that can be + /// written (`None`: all). Fails with the status the item gets instead. + pub(crate) fn outputs_of(&self, it: &RenderQueueItem, formats: Option<&[OutputFormat]>) -> Result, RenderStatus> { + let mut modules = vec![]; + for om in it.output_modules() { + let mut m = it.clone(); + m.output = om.clone(); + match self.resolve_output(&m) { + Some(p) if formats.is_none_or(|f| f.contains(&om.format)) => { + // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / + // Movie Files. + let bits = output_bits(om.format, self.project.settings.bit_depth); + let p = self.prefs.output_name(&p, bits); + modules.extend(self.segments(&m, &p)); + } + Some(_) => return Err(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), + None if self.project.comp(it.comp).is_none() => return Err(RenderStatus::Failed("composition missing".into())), + None => return Err(RenderStatus::NeedsOutput), + } + } + Ok(modules) + } + + /// What the next Render would write: for each queued item its [`Session::outputs_of`], and + /// the status of each queued item that has none. + pub(crate) fn queued_outputs(&self, formats: &[OutputFormat]) -> (Vec>, Vec<(u64, RenderStatus)>) { + let (mut items, mut problems) = (Vec::new(), Vec::new()); + for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { + match self.outputs_of(it, Some(formats)) { + Ok(modules) => items.push(modules), + Err(status) => problems.push((it.id, status)), + } + } + (items, problems) + } + + /// Start rendering every queued item, refusing outputs that replace footage, proxies or + /// the project file. Other existing outputs keep the exporter's usual replacement policy. + /// `wait`: block until done, otherwise render in the background and poll regularly. pub fn start_render(&mut self, wait: bool) -> Result, String> { let offload = self.offload.clone().filter(|_| !wait); let wait = wait || (cfg!(target_arch = "wasm32") && offload.is_none()); @@ -445,46 +481,33 @@ impl Session { self.poll_render(); let exporter = self.exporter.clone().ok_or("export is not available in this build")?; let formats = exporter.formats(); - let mut items = Vec::new(); - let mut needs_output = Vec::new(); - for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { - let mut modules = vec![]; - let mut problem = None; - for om in it.output_modules() { - let mut m = it.clone(); - m.output = om.clone(); - match self.resolve_output(&m) { - Some(p) if formats.contains(&om.format) => { - // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / - // Movie Files. - let bits = output_bits(om.format, self.project.settings.bit_depth); - let p = self.prefs.output_name(&p, bits); - modules.extend(self.segments(&m, &p)); - } - Some(_) => problem = Some(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), - None if self.project.comp(it.comp).is_none() => problem = Some(RenderStatus::Failed("composition missing".into())), - None => problem = Some(RenderStatus::NeedsOutput), - } - if problem.is_some() { - break; - } - } - match problem { - Some(st) => needs_output.push((it.id, st)), - None => items.push(modules), - } - } - if !needs_output.is_empty() { + let (mut items, mut problems) = self.queued_outputs(&formats); + let refused = self.check_outputs(&items); + items.retain(|modules| modules.first().is_none_or(|(it, _)| !refused.iter().any(|r| r.item == it.id))); + problems.extend(refused.iter().map(|r| (r.item, RenderStatus::Failed(r.reason.clone())))); + if !problems.is_empty() { let p = Arc::make_mut(&mut self.project); - for (id, st) in needs_output { + for (id, st) in problems { if let Some(i) = p.render_queue.iter_mut().find(|i| i.id == id) { i.status = st; } } self.bump(); } + if let Some(first) = refused.first() { + let message = if refused.len() == 1 { + format!("Not rendered: {}", first.reason) + } else { + format!("{} items were not rendered; first: {}", refused.len(), first.reason) + }; + self.events.push(Event::Toast { message, error: true }); + } if items.is_empty() { - return Err("nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into()); + return Err(match refused.as_slice() { + [] => "nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into(), + [only] => only.reason.clone(), + [first, ..] => format!("{} items would replace files the project reads; first: {}", refused.len(), first.reason), + }); } let ids: Vec = items.iter().filter_map(|m| m.first().map(|(i, _)| i.id)).collect(); if let Some(off) = offload { diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs new file mode 100644 index 000000000..a2473c6e8 --- /dev/null +++ b/crates/engine/src/tests_render_overwrite.rs @@ -0,0 +1,571 @@ +//! Rendering never replaces a file the project reads (#1743). Ordinary-file replacement +//! behavior stays with the exporter and its confirmation API. + +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +use effectcraft_color::Label; +use effectcraft_project::render_queue::{OutputFormat, RenderStatus, sequence_path}; +use effectcraft_project::{Footage, FootageKind, ItemId, ItemKind}; +use effectcraft_time::Tick; +use serde_json::{Value, json}; + +use crate::{ExportJob, ExportResult, Exporter, Session}; + +const SOURCE: &[u8] = b"SOURCE FOOTAGE"; +const EARLIER: &[u8] = b"AN EARLIER RENDER"; +const RENDERED: &[u8] = b"RENDERED"; + +/// An exporter that writes like the real one: `RENDERED` at the job's path, or at the path of +/// every frame for a sequence (leaving the frames that exist alone when the item skips them). +#[derive(Default)] +struct Writer { + jobs: Mutex>, +} + +impl Exporter for Writer { + fn formats(&self) -> Vec { + OutputFormat::ALL.to_vec() + } + fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + self.jobs.lock().unwrap().push(job.path.to_string()); + let comp = job.project.comp(job.item.comp).ok_or("no comp")?; + let paths: Vec = if job.item.output.format.is_sequence() { + let first = job.item.settings.first_frame(comp); + (0..job.item.settings.frame_count(comp)).map(|k| sequence_path(job.path, first + k as i64)).collect() + } else { + vec![job.path.to_string()] + }; + for path in &paths { + if job.item.settings.skip_existing && Path::new(path).exists() { + continue; + } + if let Some(dir) = Path::new(path).parent() { + std::fs::create_dir_all(dir).map_err(|e| e.to_string())?; + } + std::fs::write(path, RENDERED).map_err(|e| e.to_string())?; + } + Ok(ExportResult { path: paths.first().cloned().unwrap_or_default(), ..Default::default() }) + } +} + +/// A session whose project is saved in a fresh folder (the footage goes there too), with an +/// exporter that writes real files. +struct Fixture { + s: Session, + writer: Arc, + dir: PathBuf, +} + +impl Fixture { + fn new(name: &str) -> Fixture { + let dir = std::env::temp_dir().join("effectcraft-render-overwrite").join(format!("{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut s = Session::default(); + let writer = Arc::new(Writer::default()); + s.exporter = Some(writer.clone()); + s.execute("file.saveAs", json!({"path": dir.join("Project.ecproj").to_string_lossy()})).unwrap(); + Fixture { s, writer, dir } + } + + fn path(&self, file: &str) -> PathBuf { + self.dir.join(file) + } + + fn text(&self, file: &str) -> String { + self.path(file).to_string_lossy().into_owned() + } + + fn read(&self, file: &str) -> Vec { + std::fs::read(self.path(file)).unwrap() + } + + fn write(&self, file: &str, bytes: &[u8]) { + std::fs::write(self.path(file), bytes).unwrap(); + } + + /// A video file in the project's folder, imported. + fn footage(&mut self, file: &str) -> ItemId { + self.write(file, SOURCE); + let f = Footage { + path: self.text(file), + kind: FootageKind::Video, + width: 64, + height: 36, + has_video: true, + duration: Tick::from_seconds_f64(1.0), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(file, Label::None, None, ItemKind::Footage(f)) + } + + /// Numbered stills in the project's folder, imported as one image sequence. + fn sequence(&mut self, name: &str, frames: std::ops::Range) -> ItemId { + let files: Vec = frames + .map(|n| { + let file = format!("{name}_{n:05}.png"); + self.write(&file, SOURCE); + self.text(&file) + }) + .collect(); + let f = Footage { + path: files[0].clone(), + kind: FootageKind::Sequence, + sequence: files, + width: 16, + height: 16, + has_video: true, + duration: Tick::from_seconds_f64(0.5), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(&format!("{name}_[00000-00004].png"), Label::None, None, ItemKind::Footage(f)) + } + + /// A five-frame composition. + fn comp(&mut self, name: &str) { + self.s.execute("comp.new", json!({"name": name, "width": 16, "height": 16, "frameRate": 10, "duration": 0.5})).unwrap(); + } + + /// The output path of queue item `index` (0-based). + fn output(&self, index: usize) -> PathBuf { + PathBuf::from(self.s.resolve_output(&self.s.project.render_queue[index]).unwrap()) + } + + /// The paths the exporter was started for. + fn exported(&self) -> Vec { + self.writer.jobs.lock().unwrap().clone() + } + + fn render(&mut self, params: Value) -> crate::Result { + self.s.execute("renderQueue.render", params) + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +fn link(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_file(target, link); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// A folder that leads to `target`: a symbolic link, or on Windows, where that takes a privilege +/// most accounts lack, a junction. +fn link_dir(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_dir(target, link).or_else(|_| { + let made = std::process::Command::new("cmd").args(["/C", "mklink", "/J"]).arg(link).arg(target).output()?; + if made.status.success() { Ok(()) } else { Err(std::io::Error::other(String::from_utf8_lossy(&made.stdout).into_owned())) } + }); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// The report: save a new project beside `footage.mp4`, import it, make a composition from it +/// (it is named after the file), add that to the Render Queue and click Render. The composition's +/// default output is `/footage.mp4`, the footage itself. +#[test] +fn a_comp_made_from_footage_does_not_render_over_it() { + let mut f = Fixture::new("comp-from-footage"); + let footage = f.footage("footage.mp4"); + f.s.state.project_selection = vec![footage]; + f.s.execute("file.newCompFromSelection", json!({"addToRenderQueue": true})).unwrap(); + assert_eq!(f.s.project.render_queue.len(), 1); + f.render(json!({})).unwrap(); + assert_eq!(f.read("footage.mp4"), SOURCE, "the footage is still the footage"); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); + // The default name stepped aside: the render sits next to it. + assert_eq!(f.output(0), f.path("footage_1.mp4")); + assert_eq!(f.read("footage_1.mp4"), RENDERED); + // Another item for the same composition takes the next free number. + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(1), f.path("footage_2.mp4")); +} + +#[test] +fn an_output_that_is_the_footage_is_refused_with_or_without_overwrite() { + for (n, params) in [json!({}), json!({"overwrite": true})].into_iter().enumerate() { + let mut f = Fixture::new(&format!("explicit-footage-{n}")); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let e = f.render(params).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty(), "the exporter was not started"); + let RenderStatus::Failed(why) = &f.s.project.render_queue[0].status else { panic!("{:?}", f.s.project.render_queue[0].status) }; + assert!(why.contains("clip.mov"), "{why}"); + } +} + +/// An item that would replace the footage fails on its own; the rest of the queue renders. +#[test] +fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { + let mut f = Fixture::new("one-refused"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let ok = f.text("main.mp4"); + f.s.execute("renderQueue.add", json!({"output": ok})).unwrap(); + let r = f.render(json!({})).unwrap(); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("main.mp4"), RENDERED); + assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); + assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); +} + +#[test] +fn the_project_file_is_never_an_output() { + let mut f = Fixture::new("project-file"); + // A project saved under a name a movie could have. + let name = f.text("final.mp4"); + f.s.execute("file.saveAs", json!({"path": name})).unwrap(); + f.comp("final"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("final_1.mp4"), "the default name moves aside"); + f.s.execute("renderQueue.remove", json!({"index": 1})).unwrap(); + f.s.execute("renderQueue.add", json!({"output": name})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("final.mp4") && e.contains("project"), "{e}"); + assert!(std::fs::read_to_string(f.path("final.mp4")).unwrap().contains("\"schema\""), "the project file is still a project"); +} + +#[test] +fn a_proxy_is_not_an_output_either() { + let mut f = Fixture::new("proxy"); + f.footage("clip.mov"); + f.write("proxy.mov", SOURCE); + f.comp("Main"); + let proxy = Footage { path: f.text("proxy.mov"), kind: FootageKind::Video, has_video: true, ..Default::default() }; + let comp = f.s.active_comp_id().unwrap(); + Arc::make_mut(&mut f.s.project).item_mut(comp).unwrap().proxy = Some(Box::new(effectcraft_project::Proxy { footage: proxy, enabled: true })); + let target = f.text("proxy.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": target})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("proxy"), "{e}"); + assert_eq!(f.read("proxy.mov"), SOURCE); +} + +/// Recreating a default proxy writes a new file and attaches it, without replacing the proxy +/// the project is currently reading. Explicitly choosing that protected path is still refused. +#[test] +fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("proxy file missing".into()); + } + Ok(Footage { + path: path.into(), + kind: if path.ends_with(".png") { FootageKind::Still } else { FootageKind::Video }, + width: 16, + height: 16, + has_video: true, + ..Default::default() + }) + } + } + for kind in ["movie", "still"] { + let mut f = Fixture::new(&format!("recreate-proxy-{kind}")); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + let comp = f.s.active_comp_id().unwrap(); + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let first = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let second = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_ne!(second, first, "the replacement proxy gets its own file"); + assert_eq!(std::fs::read(&second).unwrap(), RENDERED); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); + + // For a still proxy, an explicit output is the sequence pattern, not its first + // rendered frame (a bare PNG name would have another frame number appended). + let protected_output = f.output(1); + f.s.execute("file.createProxy", json!({"kind": kind, "path": protected_output})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("never replaces a proxy"), "{error}"); + assert_eq!(f.exported().len(), 2); + } +} + +/// Exercise the engine's import and project serialization paths, rather than injecting a +/// Footage item directly. The importer reads dimensions from a real, generated PNG. +#[test] +fn imported_footage_stays_protected_after_the_project_is_reopened() { + struct PngProbe; + impl crate::Importer for PngProbe { + fn probe(&self, path: &str) -> Result { + let (width, height) = image::image_dimensions(path).map_err(|e| e.to_string())?; + Ok(Footage { path: path.into(), kind: FootageKind::Still, width, height, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("import-reopen"); + f.s.importer = Some(Arc::new(PngProbe)); + let path = f.path("plate_00002.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&path).unwrap(); + let original = std::fs::read(&path).unwrap(); + let result = f.s.execute("file.import", json!({"paths": [path]})).unwrap(); + assert_eq!(result["items"].as_array().unwrap().len(), 1); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("plate_[#####].png")})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.s.execute("file.open", json!({"path": f.text("Project.ecproj")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("plate_00002.png") && error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&path).unwrap(), original); + assert!(f.exported().is_empty()); +} + +#[test] +fn a_protected_frame_past_the_probe_limit_is_not_rendered() { + struct LastFrame(PathBuf); + impl Exporter for LastFrame { + fn formats(&self) -> Vec { + vec![OutputFormat::PngSequence] + } + fn export(&self, _: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + // A synthetic exporter writes only the vulnerable late frame, so a broken guard + // can be reproduced without creating 100,001 output directories. + std::fs::write(&self.0, RENDERED).map_err(|e| e.to_string())?; + Ok(ExportResult { path: self.0.to_string_lossy().into_owned(), ..Default::default() }) + } + } + let mut f = Fixture::new("late-frame"); + std::fs::create_dir_all(f.path("f100000")).unwrap(); + f.footage("f100000/plate.png"); + f.comp("Main"); + f.s.execute("comp.settings", json!({"duration": 10000.1})).unwrap(); + f.s.exporter = Some(Arc::new(LastFrame(f.path("f100000/plate.png")))); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("f[######]/plate.png")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("cannot safely check"), "{error}"); + assert_eq!(f.read("f100000/plate.png"), SOURCE); +} + +/// A numbered sequence is footage frame by frame: one colliding frame is enough, and nothing of +/// the item is rendered. +#[test] +fn a_sequence_never_replaces_one_of_its_footage_frames() { + let mut f = Fixture::new("sequence"); + f.sequence("shot", 0..5); + f.comp("shot"); + // The default name steps aside as a whole: shot_1_00000.png … + f.s.execute("renderQueue.add", json!({"format": "png"})).unwrap(); + assert_eq!(f.output(0), f.path("shot_1_[#####].png")); + f.render(json!({})).unwrap(); + for n in 0..5 { + assert_eq!(f.read(&format!("shot_{n:05}.png")), SOURCE, "frame {n}"); + assert_eq!(f.read(&format!("shot_1_{n:05}.png")), RENDERED, "frame {n}"); + } + + // Named by hand, one frame of five is a footage file. + let mut f = Fixture::new("sequence-one-frame"); + f.write("shot_00002.png", SOURCE); + let still = Footage { path: f.text("shot_00002.png"), kind: FootageKind::Still, width: 16, height: 16, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("shot_00002.png", Label::None, None, ItemKind::Footage(still)); + f.comp("shot"); + let pattern = f.text("shot_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("shot_00002.png"), "{e}"); + assert_eq!(f.read("shot_00002.png"), SOURCE); + assert!(!f.path("shot_00000.png").exists(), "the frames before it were not rendered either"); + assert!(f.exported().is_empty()); +} + +#[test] +fn ordinary_existing_outputs_keep_the_exporters_replacement_policy() { + let mut f = Fixture::new("earlier-render"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"output": f.text("render.mp4")})).unwrap(); + f.write("render.mp4", EARLIER); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render.mp4"), RENDERED); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); +} + +#[test] +fn a_render_with_nothing_in_the_way_needs_no_flag() { + let mut f = Fixture::new("free"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("Main.mp4"), "no footage in the way: the comp's own name"); + f.render(json!({})).unwrap(); + assert_eq!(f.read("Main.mp4"), RENDERED); +} + +/// Skip Existing Files leaves the frames that exist alone, so they need no answer. +#[test] +fn a_sequence_that_skips_existing_frames_needs_no_flag() { + let mut f = Fixture::new("skip-existing"); + f.comp("Main"); + let pattern = f.text("render_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern, "skipExisting": true})).unwrap(); + for n in 0..3 { + f.write(&format!("render_{n:05}.png"), EARLIER); + } + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), EARLIER); + assert_eq!(f.read("render_00003.png"), RENDERED); + // Ordinary files still follow the exporter's replacement policy when skipping is off. + f.s.execute("renderQueue.setRenderSettings", json!({"index": 1, "skipExisting": false})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), RENDERED); +} + +/// A link is another name for the file it points to. +#[test] +fn a_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("link"); + f.footage("clip.mov"); + f.comp("Main"); + let alias = f.path("render.mov"); + if link(&f.path("clip.mov"), &alias).is_err() { + eprintln!("this account cannot make symbolic links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(std::fs::symlink_metadata(&alias).unwrap().file_type().is_symlink(), "the link was not replaced either"); +} + +/// A hard link to the footage under another name is the same file on disk: replacing it would truncate the footage. +#[test] +fn a_hard_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("hardlink"); + f.footage("clip.mov"); + f.comp("Main"); + if std::fs::hard_link(f.path("clip.mov"), f.path("render.mov")).is_err() { + eprintln!("this volume cannot make hard links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + // not even with overwrite + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("render.mov"), SOURCE, "the other name still leads to the footage"); + assert!(f.exported().is_empty(), "nothing was rendered"); +} + +/// A copy of the footage has its size and bytes, but it is another file: an earlier render may replace it. +#[test] +fn a_copy_of_the_footage_is_not_the_footage() { + let mut f = Fixture::new("copy"); + f.footage("clip.mov"); + f.comp("Main"); + std::fs::copy(f.path("clip.mov"), f.path("copy.mov")).unwrap(); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("copy.mov")})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("copy.mov"), RENDERED); + assert_eq!(f.read("clip.mov"), SOURCE, "the footage is as it was"); +} + +/// Footage imported through a linked folder is the same file as the one in the real folder. +#[test] +fn footage_reached_through_a_linked_folder_is_still_footage() { + let mut f = Fixture::new("linked-folder"); + std::fs::create_dir_all(f.path("real")).unwrap(); + std::fs::write(f.path("real").join("clip.mov"), SOURCE).unwrap(); + if link_dir(&f.path("real"), &f.path("linked")).is_err() { + eprintln!("this account cannot link folders: skipped"); + return; + } + let via_link = + Footage { path: f.path("linked").join("clip.mov").to_string_lossy().into_owned(), kind: FootageKind::Video, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("clip.mov", Label::None, None, ItemKind::Footage(via_link)); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.path("real").join("clip.mov").to_string_lossy()})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(std::fs::read(f.path("real").join("clip.mov")).unwrap(), SOURCE); +} + +/// Where the volume ignores case, `CLIP.MOV` is `clip.mov`. +#[test] +fn another_spelling_of_the_footage_name_is_the_footage_where_the_volume_ignores_case() { + let mut f = Fixture::new("case"); + f.footage("clip.mov"); + f.comp("Main"); + if !f.path("CLIP.MOV").exists() { + eprintln!("this volume tells the two names apart: skipped"); + return; + } + let shouting = f.text("CLIP.MOV"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": shouting})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); +} + +#[test] +fn save_current_preview_never_replaces_footage() { + let mut f = Fixture::new("preview"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + let e = f.s.execute("render.saveCurrentPreview", json!({"path": clip})).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty()); + // Anywhere else it renders. + let elsewhere = f.text("preview.mp4"); + f.s.execute("render.saveCurrentPreview", json!({"path": elsewhere})).unwrap(); + assert_eq!(f.read("preview.mp4"), RENDERED); +} + +#[test] +fn save_current_preview_checks_sequence_frames_as_well_as_the_pattern() { + let mut f = Fixture::new("preview-sequence"); + f.sequence("plate", 0..5); + f.comp("Main"); + let error = f.s.execute("render.saveCurrentPreview", json!({"path": f.text("plate_[#####].png")})).unwrap_err().to_string(); + assert!(error.contains("plate_000") && error.contains("footage"), "{error}"); + for frame in 0..5 { + assert_eq!(f.read(&format!("plate_{frame:05}.png")), SOURCE); + } + assert!(f.exported().is_empty()); +} + +/// A watched folder renders unattended; delete the status file to render a project again. +#[test] +fn a_watched_folder_still_renders_a_project_again_over_its_outputs() { + let mut f = Fixture::new("watch"); + f.comp("Main"); + let out = f.text("Main.mp4"); + f.s.execute("renderQueue.add", json!({"output": out})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.write("Main.mp4", EARLIER); + let folder = f.dir.to_string_lossy().into_owned(); + let done = crate::commands::watch_folder::poll(&mut f.s, &folder).unwrap(); + assert_eq!(done.len(), 1, "{done:?}"); + assert_eq!(done[0]["state"], "done", "{:?}", done[0]); + assert_eq!(f.read("Main.mp4"), RENDERED); +} diff --git a/crates/project/src/render_queue.rs b/crates/project/src/render_queue.rs index 2d3ab741b..c1cda7087 100644 --- a/crates/project/src/render_queue.rs +++ b/crates/project/src/render_queue.rs @@ -1455,22 +1455,56 @@ pub fn expand_template(template: &str, format: OutputFormat, v: &TemplateVars) - /// Replace the first run of `#` (optionally in brackets, `[####]`) with the zero-padded frame /// number. Paths without a run get `_NNNNN` before the extension. pub fn sequence_path(path: &str, frame: i64) -> String { + let (before, width, after) = sequence_parts(path); + format!("{before}{frame:0width$}{after}") +} + +/// A sequence output path around its frame number: what comes before it, how many digits it is +/// padded to and what comes after (`out/a_[#####].png` → `out/a_`, 5, `.png`). +pub fn sequence_parts(path: &str) -> (String, usize, String) { if let Some(start) = path.find('#') { let len = path[start..].chars().take_while(|c| *c == '#').count(); let (mut a, mut b) = (start, start + len); - if a > 0 && path.as_bytes()[a - 1] == b'[' && path.as_bytes().get(b) == Some(&b']') { + if a > 0 && path.as_bytes().get(a - 1) == Some(&b'[') && path.as_bytes().get(b) == Some(&b']') { a -= 1; b += 1; } - return format!("{}{:0width$}{}", &path[..a], frame, &path[b..], width = len); + return (path[..a].to_string(), len, path[b..].to_string()); } let p = std::path::Path::new(path); match p.extension() { - Some(ext) => format!("{}_{frame:05}.{}", p.with_extension("").to_string_lossy(), ext.to_string_lossy()), - None => format!("{path}_{frame:05}"), + Some(ext) => (format!("{}_", p.with_extension("").to_string_lossy()), 5, format!(".{}", ext.to_string_lossy())), + None => (format!("{path}_"), 5, String::new()), } } +/// The frame number `candidate` is the file of, when `path` (a sequence output path) writes it: +/// the inverse of [`sequence_path`]. +pub fn sequence_frame(path: &str, candidate: &str) -> Option { + let (before, width, after) = sequence_parts(path); + let digits = candidate.strip_prefix(before.as_str())?.strip_suffix(after.as_str())?; + let frame: i64 = digits.parse().ok()?; + (format!("{frame:0width$}") == digits).then_some(frame) +} + +/// `template` with `_n` added to the name: before the frame-number run of a sequence, else before +/// the extension (`[compName].[fileExtension]` → `[compName]_1.[fileExtension]`, +/// `[compName]_[#####].[fileExtension]` → `[compName]_1_[#####].[fileExtension]`). +pub fn numbered_template(template: &str, n: u32) -> String { + let at = match template.find('#') { + Some(hash) => { + // The run's `[`, then the separator that sets it off from the name. + let run = if template[..hash].ends_with('[') { hash - 1 } else { hash }; + if template[..run].ends_with(['_', '-', '.', ' ']) { run - 1 } else { run } + } + None => { + let name = template.rfind(['/', '\\']).map_or(0, |i| i + 1); + template[name..].rfind('.').map_or(template.len(), |i| name + i) + } + }; + format!("{}_{n}{}", &template[..at], &template[at..]) +} + /// Make a name safe as a file name component. pub fn sanitize(name: &str) -> String { name.chars().map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect() @@ -1490,6 +1524,44 @@ mod tests { assert_eq!(sequence_path("a.png", 3), "a_00003.png"); } + /// `sequence_frame` reads back what `sequence_path` writes, and nothing else. + #[test] + fn sequence_frames_read_back() { + for path in ["out/a_[#####].png", "a###.tif", "a.png", "/x/[compName]/shot_[##].exr", "noext", "dir.v2/clip"] { + for frame in [0, 7, 42, 99_999, 123_456, -3] { + assert_eq!(sequence_frame(path, &sequence_path(path, frame)), Some(frame), "{path} {frame}"); + } + } + let (before, width, after) = sequence_parts("out/a_[#####].png"); + assert_eq!((before.as_str(), width, after.as_str()), ("out/a_", 5, ".png")); + let (before, width, after) = sequence_parts("a.png"); + assert_eq!((before.as_str(), width, after.as_str()), ("a_", 5, ".png")); + // Other numbers, other widths, other names are not frames of the sequence. + assert_eq!(sequence_frame("a_[#####].png", "a_0042.png"), None, "padded to four"); + assert_eq!(sequence_frame("a_[####].png", "a_+042.png"), None, "a sign is not padding"); + assert_eq!(sequence_frame("a_[#####].png", "a_00042.jpg"), None); + assert_eq!(sequence_frame("a_[#####].png", "b_00042.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_00x42.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_99999999999999999999.png"), None, "too big for a frame number"); + } + + #[test] + fn numbered_templates() { + assert_eq!(numbered_template(DEFAULT_TEMPLATE, 1), "[compName]_1.[fileExtension]"); + assert_eq!(numbered_template(DEFAULT_SEQUENCE_TEMPLATE, 2), "[compName]_2_[#####].[fileExtension]"); + assert_eq!(numbered_template("[compName]_frame.[fileExtension]", 3), "[compName]_frame_3.[fileExtension]"); + assert_eq!(numbered_template("out/[compName]###.png", 1), "out/[compName]_1###.png"); + assert_eq!(numbered_template("my.dir/clip", 4), "my.dir/clip_4", "a dot in a folder is not an extension"); + assert_eq!(numbered_template("clip.v2.mov", 1), "clip.v2_1.mov"); + assert_eq!(numbered_template("", 1), "_1"); + assert_eq!(numbered_template("#", 1), "_1#"); + assert_eq!(numbered_template("[#]", 1), "_1[#]"); + // A numbered default is still a default: it expands to the numbered name. + let v = TemplateVars { comp_name: "footage", project_name: "P", width: 64, height: 36, frame_rate: 30.0, start_frame: 0, end_frame: 29 }; + assert_eq!(expand_template(&numbered_template(DEFAULT_TEMPLATE, 1), OutputFormat::H264, &v), "footage_1.mp4"); + } + #[test] fn set_format_fixes_names() { let mut m = OutputModule::default(); diff --git a/docs/agents.md b/docs/agents.md index 5c58d24b7..183315366 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -4,6 +4,10 @@ Everything in EffectCraft is an engine command with a stable id and JSON params. panel gestures, the CLI, the control channel and MCP all go through the same commands, so anything a user can do, an agent can do too. There are three ways in: +Render-queue outputs cannot replace footage, proxies or the project file. See +[render source protection](render-source-protection.md) for aliases, default names and sequence +limits. This guard does not change ordinary-file overwrite defaults or add another confirmation API. + | Interface | Best for | Needs a window | |---|---|---| | **MCP** (`effectcraft-cli mcp`) | Claude Code and other MCP clients | no (headless), or yes with `--bridge` | diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md new file mode 100644 index 000000000..238684ed4 --- /dev/null +++ b/docs/render-source-protection.md @@ -0,0 +1,26 @@ +# Render source protection + +The render queue refuses an output that would replace footage, an attached proxy, or the +project file. It checks every output module and segment before dispatching the item. A refused +item is marked Failed with its reason; other valid queued items can still render. Render logs +are checked too. No render parameter overrides this protection. + +Checks compare normalized paths and native file identities, so an existing symbolic link, +hard link, junction or Windows short name cannot disguise a source as another output file. +Source identities are indexed once per check and matching identities are verified. The index +does not keep file handles open for the lifetime of a long sequence. + +Default output names move aside when they would replace a protected file. This also applies to +Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving +the previous proxy file intact. An explicit output path is kept as entered and refused if it +would replace a protected file. + +Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves +existing frames alone. If a frame number appears in a folder name, the checker probes each +frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. +Choose a fixed output folder or a shorter range for those exports. + +This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement +defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files +belong to the exporter. The source check runs before export; filesystem changes between that +check and a write still require exporter-side revalidation. From 39137fbd784ca202142b349fa3c9db544b04f58c Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:28:42 +0000 Subject: [PATCH 2/8] #1743: report refused queue items to CLI callers --- .../workflows/render-source-protection.yml | 2 + apps/effectcraft-cli/src/main.rs | 3 +- apps/effectcraft-cli/tests/it/cli.rs | 43 +++++++++++++++++++ crates/engine/src/commands/render_queue.rs | 12 +++++- crates/engine/src/tests_render_overwrite.rs | 35 +++++++++++++++ docs/render-source-protection.md | 4 ++ 6 files changed, 97 insertions(+), 2 deletions(-) diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml index 3c040d831..a61c3bd4c 100644 --- a/.github/workflows/render-source-protection.yml +++ b/.github/workflows/render-source-protection.yml @@ -28,6 +28,8 @@ jobs: CARGO_INCREMENTAL: 0 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@1.95.0 - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index 6e5b8d6cc..c858dec18 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -783,6 +783,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { r["outputModuleSummary"].as_str().unwrap_or("") ); } + let attempted: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); s.execute("renderQueue.render", json!({"wait": false})).map_err(err)?; let tty = std::io::IsTerminal::is_terminal(&std::io::stderr()); // `--progress`: one line per update for a program reading stderr (a terminal gets the live line). @@ -821,7 +822,7 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { } let mut results = vec![]; let mut failed = vec![]; - for it in s.project.render_queue.iter().filter(|i| i.render && i.started.is_some()) { + for it in s.project.render_queue.iter().filter(|i| attempted.contains(&i.id)) { let name = s.project.item(it.comp).map(|i| i.name.clone()).unwrap_or_else(|| "?".into()); match &it.status { RenderStatus::Done => { diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 538887d9b..97c604ce4 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -533,6 +533,49 @@ fn a_failed_render_under_run_and_exec_exits_1() { assert!(v["error"].as_str().is_some_and(|e| e.starts_with("T: ")), "{v}"); } +/// A source refusal happens before encoding and has no started timestamp. It must still +/// fail the CLI, while the valid queue item finishes and the imported frame stays intact. +#[test] +fn a_partly_protected_queue_exits_1_under_render_run_and_exec() { + let root = tmp("protected-mixed-queue"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let source = root.join("source_00000.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&source).unwrap(); + let original = std::fs::read(&source).unwrap(); + let project = root.join("q.ecproj"); + let good = root.join("good.gif"); + let import = json!({"paths": [source]}).to_string(); + let blocked = json!({"format": "png", "output": root.join("source_[#####].png")}).to_string(); + let valid = json!({"format": "gif", "output": good}).to_string(); + ok_json(&[ + "run", + "--empty", + "--save-as", + project.to_str().unwrap(), + "comp.new", + r#"{"name":"Main","width":16,"height":16,"frameRate":10,"duration":0.1}"#, + "layer.newSolid", + "{}", + "file.import", + &import, + "renderQueue.add", + &blocked, + "renderQueue.add", + &valid, + ]); + let p = project.to_str().unwrap(); + for args in [vec!["render", "--queue", "--project", p], vec!["run", p, "renderQueue.render", "{}"], vec!["exec", "renderQueue.render", "--project", p]] { + std::fs::write(&good, b"previous output").unwrap(); + let (code, result) = run_json(&args); + assert_eq!(code, 1, "{args:?}: {result}"); + assert!(result["error"].as_str().is_some_and(|e| e.contains("footage") && e.contains("source_00000.png")), "{result}"); + assert_eq!(std::fs::read(&source).unwrap(), original); + assert!(std::fs::read(&good).unwrap().starts_with(b"GIF8"), "the valid item still rendered"); + } + std::fs::remove_dir_all(root).unwrap(); +} + /// `render --bitrate` (#440) targets the bitrate in VP9 WebM. #[test] fn render_bitrate_applies_to_vp9_webm() { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index d7e914b1a..5b71cfca5 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -681,8 +681,18 @@ fn render(s: &mut Session, p: &Value) -> Result { { log::warn!("{e}"); } + let queued: std::collections::HashSet = s.project.render_queue.iter().filter(|i| i.is_queued()).map(|i| i.id).collect(); let ids = s.start_render(wait).map_err(EngineError::Other)?; - let items: Vec = s.project.render_queue.iter().enumerate().filter(|(_, i)| ids.contains(&i.id)).map(|(k, i)| item_json(s, i, k)).collect(); + // Source refusals fail before the exporter starts. Report them just like encoding + // failures, so CLI/MCP callers do not mistake a partly refused queue for full success. + let items: Vec = s + .project + .render_queue + .iter() + .enumerate() + .filter(|(_, i)| ids.contains(&i.id) || (queued.contains(&i.id) && matches!(i.status, RenderStatus::Failed(_)))) + .map(|(k, i)| item_json(s, i, k)) + .collect(); Ok(json!({"rendering": s.is_rendering(), "items": items})) } diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs index a2473c6e8..420763da2 100644 --- a/crates/engine/src/tests_render_overwrite.rs +++ b/crates/engine/src/tests_render_overwrite.rs @@ -230,6 +230,7 @@ fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { assert_eq!(f.read("main.mp4"), RENDERED); assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"].get("Failed").is_some()), "the reply includes the refused item: {r}"); assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); } @@ -313,6 +314,40 @@ fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { } } +/// A pre-render becomes imported footage. Its old output stays protected even from the +/// same queue item; issuing Pre-render again supplies a fresh default output instead. +#[test] +fn refreshing_a_pre_render_uses_a_new_default_without_exempting_imported_footage() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("pre-render missing".into()); + } + Ok(Footage { path: path.into(), kind: FootageKind::Video, width: 16, height: 16, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("refresh-pre-render"); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let first = f.output(0); + f.render(json!({})).unwrap(); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": true})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 1); + f.s.execute("renderQueue.setRender", json!({"index": 1, "render": false})).unwrap(); + f.s.execute("render.preRender", json!({"format": "gif"})).unwrap(); + let second = f.output(1); + assert_ne!(first, second); + f.render(json!({})).unwrap(); + assert_eq!(std::fs::read(first).unwrap(), RENDERED); + assert_eq!(std::fs::read(second).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); +} + /// Exercise the engine's import and project serialization paths, rather than injecting a /// Footage item directly. The importer reads dimensions from a real, generated PNG. #[test] diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md index 238684ed4..cf15f6797 100644 --- a/docs/render-source-protection.md +++ b/docs/render-source-protection.md @@ -15,6 +15,10 @@ Create Proxy: recreating a proxy chooses a new default name and attaches the new the previous proxy file intact. An explicit output path is kept as entered and refused if it would replace a protected file. +After Pre-render imports its result, that file is footage too. Re-queueing the same item +with the same output is refused; run Pre-render again for a fresh default output, or choose +another Output To. An item's `last_output` is not permission to replace imported footage. + Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves existing frames alone. If a frame number appears in a folder name, the checker probes each frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. From 199af9213c629efbc69c2b73f452519afe5c531f Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:10:36 +0100 Subject: [PATCH 3/8] #1743: use existing CI and consolidate render source documentation --- .../workflows/render-source-protection.yml | 36 ------------------- docs/agents.md | 2 +- docs/file-format-parity.md | 32 +++++++++++++++++ docs/render-source-protection.md | 30 ---------------- 4 files changed, 33 insertions(+), 67 deletions(-) delete mode 100644 .github/workflows/render-source-protection.yml delete mode 100644 docs/render-source-protection.md diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml deleted file mode 100644 index 4bb44c8bf..000000000 --- a/.github/workflows/render-source-protection.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: Render source protection - -on: - pull_request: - paths: - - .github/workflows/render-source-protection.yml - - Cargo.toml - - Cargo.lock - - crates/engine/** - - crates/project/src/render_queue.rs - workflow_dispatch: - -permissions: - contents: read - -jobs: - regression: - name: Source guard (${{ matrix.os }}) - runs-on: ${{ matrix.os }} - timeout-minutes: 30 - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest] - env: - CARGO_PROFILE_DEV_DEBUG: 0 - CARGO_PROFILE_TEST_DEBUG: 0 - CARGO_INCREMENTAL: 0 - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - - uses: dtolnay/rust-toolchain@1.95.0 - - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests - - run: cargo test --locked -p effectcraft-engine --lib tests::render_overwrite - - run: cargo test --locked -p effectcraft-project render_queue::tests diff --git a/docs/agents.md b/docs/agents.md index 1781c7163..9672d9c42 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -5,7 +5,7 @@ panel gestures, the CLI, the control channel and MCP all go through the same com user can do, an agent can do too. There are three ways in: Render-queue outputs cannot replace footage, proxies or the project file. See -[render source protection](render-source-protection.md) for aliases, default names and sequence +[render source protection](file-format-parity.md#render-source-protection) for aliases, default names and sequence limits. This guard does not change ordinary-file overwrite defaults or add another confirmation API. | Interface | Best for | Needs a window | diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 9e2249e9b..19447ae3d 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -105,6 +105,37 @@ single-file video (Animation / PNG codecs), DPX / Targa / Photoshop sequences, M encoders ([hardware-parity.md](hardware-parity.md)), and rendering safely over an existing file (#325). +### Render source protection + +The render queue refuses an output that would replace footage, an attached proxy, or the +project file. It checks every output module and segment before dispatching the item. A refused +item is marked Failed with its reason; other valid queued items can still render. Render logs +are checked too. No render parameter overrides this protection. + +Checks compare normalized paths and native file identities, so an existing symbolic link, +hard link, junction or Windows short name cannot disguise a source as another output file. +Source identities are indexed once per check and matching identities are verified. The index +does not keep file handles open for the lifetime of a long sequence. + +Default output names move aside when they would replace a protected file. This also applies to +Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving +the previous proxy file intact. An explicit output path is kept as entered and refused if it +would replace a protected file. + +After Pre-render imports its result, that file is footage too. Re-queueing the same item +with the same output is refused; run Pre-render again for a fresh default output, or choose +another Output To. An item's `last_output` is not permission to replace imported footage. + +Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves +existing frames alone. If a frame number appears in a folder name, the checker probes each +frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. +Choose a fixed output folder or a shorter range for those exports. + +This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement +defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files +belong to the exporter. The source check runs before export; filesystem changes between that +check and a write still require exporter-side revalidation. + ## Estimate **≈ 130–230 h**: `.aep` / `.aepx` import 80–160 h (after the owner decision), wiring TGA / DPX / @@ -115,5 +146,6 @@ relinking 6–12 h, `.mogrt`-compatible templates 15–30 h (partly parallel wit | Date | Change | Summary | |---|---|---| +| 2026-10-30 | minor | Render-queue source-protection behavior and limits consolidated here from the standalone page | | 2026-10-12 | minor | #324 and #514 marked closed; open `.aep` PRs #251 and #860 noted | | 2026-10-10 | major | First file format parity document, from the After Effects 26.5 bundle and this repository's import/export registries | diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md deleted file mode 100644 index cf15f6797..000000000 --- a/docs/render-source-protection.md +++ /dev/null @@ -1,30 +0,0 @@ -# Render source protection - -The render queue refuses an output that would replace footage, an attached proxy, or the -project file. It checks every output module and segment before dispatching the item. A refused -item is marked Failed with its reason; other valid queued items can still render. Render logs -are checked too. No render parameter overrides this protection. - -Checks compare normalized paths and native file identities, so an existing symbolic link, -hard link, junction or Windows short name cannot disguise a source as another output file. -Source identities are indexed once per check and matching identities are verified. The index -does not keep file handles open for the lifetime of a long sequence. - -Default output names move aside when they would replace a protected file. This also applies to -Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving -the previous proxy file intact. An explicit output path is kept as entered and refused if it -would replace a protected file. - -After Pre-render imports its result, that file is footage too. Re-queueing the same item -with the same output is refused; run Pre-render again for a fresh default output, or choose -another Output To. An item's `last_output` is not permission to replace imported footage. - -Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves -existing frames alone. If a frame number appears in a folder name, the checker probes each -frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. -Choose a fixed output folder or a shorter range for those exports. - -This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement -defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files -belong to the exporter. The source check runs before export; filesystem changes between that -check and a write still require exporter-side revalidation. From ccae8411b5d6d30410637619ddf03058d527c196 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 01:41:06 +0000 Subject: [PATCH 4/8] i18n(ja): fix one mistranslation and unify terms the menu and the panels spell differently "Warn User When Executing Files" said the cursor, not the user, was warned. The other 27 rows give the same English term one Japanese spelling in the menu table and another in the UI table (track/tracking, Region of Interest, Time Remapping, Separate Dimensions, Stabilize Motion, Hold, Brightness, Vector Layer, Small Screen, Straight, Expression and a few more), or read awkwardly ("({}) Selected Objects", "Interpret Unlabeled Alpha As"). Each now follows the spelling the rest of the file already uses. Only Japanese values change; no key or English text does. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01K5NRZNWfaSLBWVzV8hivyS --- crates/ui-egui/src/i18n.rs | 30 +++++++++++++++--------------- crates/ui-egui/src/i18n/ui.rs | 26 +++++++++++++------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 14afe59cc..7f4a77989 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -1874,7 +1874,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("comp.settings", "Composition Settings...", "コンポジション設定..."), ("comp.setPosterTime", "Set Poster Time", "ポスター時間を設定"), ("comp.trimToWorkArea", "Trim Comp to Work Area", "コンポジションをワークエリアにトリミング"), - ("comp.cropToRegionOfInterest", "Crop Comp to Region of Interest", "コンポジションを目標範囲にクロップ"), + ("comp.cropToRegionOfInterest", "Crop Comp to Region of Interest", "コンポジションを関心領域にクロップ"), ("comp.cropToLayerBounds", "Crop Comp to Selected Layer(s) Bounds", "コンポジションを選択レイヤーの境界にクロップ"), ("render.addOutputModule", "Add Output Module", "出力モジュールを追加"), ("", "Preview", "プレビュー"), @@ -1927,7 +1927,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.mask.reset", "Reset Mask", "マスクをリセット"), ("layer.mask.remove", "Remove Mask", "マスクを削除"), ("layer.mask.removeAll", "Remove All Masks", "すべてのマスクを削除"), - ("track.mask", "Track Mask", "マスクをトラック"), + ("track.mask", "Track Mask", "マスクをトラッキング"), ("", "Mode", "モード"), ("layer.mask.mode", "None", "なし"), ("layer.mask.mode", "Add", "加算"), @@ -1992,8 +1992,8 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.transform", "Fit to Comp Height", "コンポジションの高さに合わせる"), ("layer.autoOrient", "Auto-Orient...", "自動方向..."), ("", "Time", "時間"), - ("layer.enableTimeRemap", "Enable Time Remapping", "タイムリマップ使用可能"), - ("layer.timeReverse", "Time-Reverse Layer", "時間反転レイヤー"), + ("layer.enableTimeRemap", "Enable Time Remapping", "時間リマップを有効化"), + ("layer.timeReverse", "Time-Reverse Layer", "レイヤーを時間反転"), ("layer.timeStretch", "Time Stretch...", "時間伸縮..."), ("layer.freezeFrame", "Freeze Frame", "フレームを固定"), ("layer.freezeOnLastFrame", "Freeze On Last Frame", "最後のフレームで固定"), @@ -2061,7 +2061,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.trackMatte", "Matte with Layer Above", "上のレイヤーをマットにする"), ("layer.trackMatte", "Matte with Layer Below", "下のレイヤーをマットにする"), ("", "Layer Styles", "レイヤースタイル"), - ("layer.style.options", "Layer Style Options...", "レイヤースタイルオプション..."), + ("layer.style.options", "Layer Style Options...", "レイヤースタイルのオプション..."), ("layer.style.convertToEditable", "Convert to Editable Styles", "編集可能なスタイルに変換"), ("layer.style.showAll", "Show All", "すべてを表示"), ("layer.style.removeAll", "Remove All", "すべてを削除"), @@ -2090,7 +2090,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.create", "Convert to Editable Text", "編集可能なテキストに変換"), ("layer.create", "Create Shapes from Text", "テキストからシェイプを作成"), ("layer.create", "Create Masks from Text", "テキストからマスクを作成"), - ("layer.create", "Create Shapes from Vector Layer", "ベクトルレイヤーからシェイプを作成"), + ("layer.create", "Create Shapes from Vector Layer", "ベクターレイヤーからシェイプを作成"), ("layer.create", "Create Keyframes from Data", "データからキーフレームを作成"), ("layer.create", "Null Controllers for Positional Points", "位置のポイント用ヌルコントローラー"), ("layer.create", "Null Controllers for Path Points", "パスのポイント用ヌルコントローラー"), @@ -2140,7 +2140,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.applyTextPreset", "Jitter", "ジッター"), ("layer.applyTextPreset", "Drop In Lines", "行をドロップイン"), ("anim.addKeyframe", "Add Keyframe", "キーフレームを追加"), - ("keys.toggleHold", "Toggle Hold Keyframe", "停止したキーフレームの切り替え"), + ("keys.toggleHold", "Toggle Hold Keyframe", "ホールドキーフレームの切り替え"), ("keys.setMuted", "Mute Keyframes", "キーフレームをミュート"), ("keys.setMuted", "Unmute Keyframes", "キーフレームのミュートを解除"), ("keys.interpolation", "Keyframe Interpolation...", "キーフレーム補間法..."), @@ -2156,7 +2156,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.sequence", "Sequence Layers...", "シーケンスレイヤー..."), ("keys.timeReverse", "Time-Reverse Keyframes", "時間反転キーフレーム"), ("", "Animate Text", "テキストをアニメート"), - ("layer.enablePerChar3D", "Enable Per-character 3D", "文字ごとの3Dを使用可能にする"), + ("layer.enablePerChar3D", "Enable Per-character 3D", "文字ごとの3Dを有効化"), ("layer.addTextAnimator", "Anchor Point", "アンカーポイント"), ("layer.addTextAnimator", "Position", "位置"), ("layer.addTextAnimator", "Scale", "スケール"), @@ -2168,7 +2168,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("layer.addTextAnimator", "RGB", "RGB"), ("layer.addTextAnimator", "Hue", "色相"), ("layer.addTextAnimator", "Saturation", "彩度"), - ("layer.addTextAnimator", "Brightness", "明度"), + ("layer.addTextAnimator", "Brightness", "明るさ"), ("", "Stroke Color", "線のカラー"), ("layer.addTextAnimator", "Stroke Width", "線幅"), ("", "Front", "前面"), @@ -2190,12 +2190,12 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("text.removeAllAnimators", "Remove All Text Animators", "すべてのテキストアニメーターを削除"), ("prop.setExpression", "Add Expression", "エクスプレッションを追加"), ("essential.addProperty", "Add Property to Essential Graphics", "エッセンシャルグラフィックスにプロパティを追加"), - ("prop.separateDimensions", "Separate Dimensions", "次元に分割"), - ("track.camera", "Track Camera", "カメラをトラック"), + ("prop.separateDimensions", "Separate Dimensions", "次元を分離"), + ("track.camera", "Track Camera", "カメラをトラッキング"), ("track.warpStabilizer", "Warp Stabilizer VFX", "ワープスタビライザーVFX"), - ("track.motion", "Track Motion", "モーションをトラック"), - ("track.stabilize", "Stabilize Motion", "モーションをスタビライズ"), - ("track.property", "Track this Property", "このプロパティをトラック"), + ("track.motion", "Track Motion", "モーションをトラッキング"), + ("track.stabilize", "Stabilize Motion", "モーションを安定化"), + ("track.property", "Track this Property", "このプロパティをトラッキング"), ("anim.reveal", "Reveal Properties with Keyframes", "キーフレームのあるプロパティを表示"), ("anim.reveal", "Reveal Properties with Animation", "アニメーションのあるプロパティを表示"), ("anim.reveal", "Reveal All Modified Properties", "変更されたすべてのプロパティを表示"), @@ -2275,7 +2275,7 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("window.workspace", "Default", "初期設定"), ("window.workspace", "Review", "レビュー"), ("window.workspace", "Learn", "学習"), - ("window.workspace", "Small Screen", "小画面"), + ("window.workspace", "Small Screen", "小さい画面"), ("window.workspace", "Standard", "標準"), ("window.workspace", "All Panels", "すべてのパネル"), ("window.workspace", "Animation", "アニメーション"), diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 1da662aad..ab9b0eba2 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -455,7 +455,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("None", "なし"), ("Not applied: tick it in the list to add it.", "未適用：リストでチェックを入れると追加されます。"), ("Not assigned", "未割り当て"), - ("Note: Shadows are only cast from layers with Cast Shadows enabled.", "注意：シャドウは「シャドウを投影」が有効なレイヤーからのみ投影されます。"), + ("Note: Shadows are only cast from layers with Cast Shadows enabled.", "注意：「シャドウを落とす」が有効なレイヤーだけがシャドウを落とします。"), ("Notify", "通知"), ("OK", "OK"), ("Offset:", "オフセット:"), @@ -537,7 +537,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Select a command to change its shortcut.", "ショートカットを変更するコマンドを選択してください。"), ("Select a layer to see its effects", "レイヤーを選択するとエフェクトが表示されます"), ("Select a layer to see its properties", "レイヤーを選択するとプロパティが表示されます"), - ("({}) Selected Objects", "({}) 個の選択されたオブジェクト"), + ("({}) Selected Objects", "選択したオブジェクト（{}）"), ( "Select a layer with a mask or shape path (or the path property), or Puppet pins.", "マスクまたはシェイプパスのあるレイヤー（またはパスプロパティ）か、パペットピンを選択してください。", @@ -685,7 +685,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Import Image Sequence", "画像シーケンスを読み込み"), ("Import PDF / Illustrator File", "PDF / Illustratorファイルを読み込み"), ("Import Photoshop File", "Photoshopファイルを読み込み"), - ("Interpret Unlabeled Alpha As", "ラベルのないアルファを変換"), + ("Interpret Unlabeled Alpha As", "ラベルのないアルファの解釈"), ("Join the ArtCraft Discord", "ArtCraftのDiscordに参加"), ("Mercury GPU Acceleration ({})", "Mercury GPUアクセラレーション（{}）"), ("Mercury Software Only", "Mercuryソフトウェアのみ"), @@ -803,7 +803,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Cycle Mask Colors", "新規マスクに別のカラーを使用"), ("Use Gradients", "グラデーションを使用"), ("Menu Bar", "メニューバー"), - ("Grids & Guides", "グリッド＆ガイド"), + ("Grids & Guides", "グリッドとガイド"), ("Gridline Every", "グリッドの間隔"), ("Subdivisions", "分割数"), ("Style", "スタイル"), @@ -835,7 +835,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Ask User", "ユーザーに確認"), ("Guess", "推測"), ("Ignore", "無視"), - ("Straight", "そのまま"), + ("Straight", "ストレート"), ("Premultiplied", "プリマルチプライ"), ("Default Drag Import As", "複数アイテムのドラッグ読み込みの初期設定"), ("Footage", "フッテージ"), @@ -885,7 +885,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Scripting & Expressions", "スクリプトとエクスプレッション"), ("Application Scripting", "アプリケーションのスクリプト"), ("Allow Scripts to Write Files and Access Network", "スクリプトによるファイルへの書き込みとネットワークへのアクセスを許可"), - ("Warn User When Executing Files", "ファイルの実行時にカーソルに警告"), + ("Warn User When Executing Files", "ファイルの実行時にユーザーに警告"), ("Enable JavaScript Debugger", "JavaScript デバッガーを使用"), ("Expressions Editor", "エクスプレッションエディター"), ("Font Size", "フォントサイズ"), @@ -894,7 +894,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Auto-complete", "オートコンプリート"), ("Bracket Matching", "対応する括弧をハイライト"), ("Word Wrap", "折り返し"), - ("Show Expression Error Banner", "式のエラーがある場合に警告バナーを表示"), + ("Show Expression Error Banner", "エクスプレッションのエラーがある場合に警告バナーを表示"), ( "Expressions use the JavaScript engine. Script errors report their file and line in the Script Console; there is no step debugger.", "エクスプレッションはJavaScriptエンジンを使用します。スクリプトのエラーはファイルと行をスクリプトコンソールに報告します。ステップ実行デバッガーはありません。", @@ -1003,8 +1003,8 @@ const JAPANESE: &[(&str, &str)] = &[ ("Create Shapes from Vector Layer", "ベクターレイヤーからシェイプを作成"), ("Create Shapes from Text", "テキストからシェイプを作成"), ("Create Masks from Text", "テキストからマスクを作成"), - ("New Mask", "新しいマスク"), - ("Center In View", "ビュー内で中央揃え"), + ("New Mask", "新規マスク"), + ("Center In View", "ビューの中央に配置"), ("Fit to Comp", "コンポジションに合わせる"), ("Flip Horizontal", "水平方向に反転"), ("Flip Vertical", "垂直方向に反転"), @@ -1033,8 +1033,8 @@ const JAPANESE: &[(&str, &str)] = &[ ("All Transform Properties", "すべてのトランスフォームプロパティ"), ("Line Anchor", "行のアンカー"), ("Line Spacing", "行間"), - ("Character Offset", "文字オフセット"), - ("Character Value", "文字値"), + ("Character Offset", "文字のオフセット"), + ("Character Value", "文字の値"), ("Fill Color: RGB", "塗りの色: RGB"), ("Fill Color: Hue", "塗りの色: 色相"), ("Fill Color: Saturation", "塗りの色: 彩度"), @@ -1060,7 +1060,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("ScriptUI Panel", "ScriptUI パネル"), ("Lumetri Scopes", "Lumetri スコープ"), ("Media Browser", "メディアブラウザー"), - ("Content-Aware Fill", "コンテンツに応じた塗り"), + ("Content-Aware Fill", "コンテンツに応じた塗りつぶし"), ("Create Nulls From Paths", "パスからヌルを作成"), ("VR Comp Editor", "VR コンポジションエディター"), ("Media Duration", "メディアの長さ"), @@ -1095,7 +1095,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Background Color", "背景色"), ("Basic", "基本"), ("Shutter Angle", "シャッター角度"), - ("Shutter Phase", "シャッター位相"), + ("Shutter Phase", "シャッターフェーズ"), ("Samples Per Frame", "フレームあたりのサンプル数"), ("Adaptive Sample Limit", "適応サンプル数の上限"), ("X and Y", "X と Y"), From 8a317537fa5332ba2c10f69e422f2dcd04ba1fc6 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:44:56 +0100 Subject: [PATCH 5/8] #1805: keep marker names and duration handles visible with shared GPU test gating --- crates/ui-egui/src/panels/markers_ui.rs | 57 +++++++++++++++---- crates/ui-egui/tests/it/gpu_test.rs | 43 ++++++++++++++ crates/ui-egui/tests/it/main.rs | 1 + .../ui-egui/tests/it/render_queue_snapshot.rs | 5 +- crates/ui-egui/tests/it/ui_3d.rs | 10 ++-- .../tests/it/ui_blank_context_menus.rs | 3 +- crates/ui-egui/tests/it/ui_camera_tracker.rs | 6 +- .../tests/it/ui_character_eyedropper.rs | 5 +- crates/ui-egui/tests/it/ui_dock_resize.rs | 4 +- crates/ui-egui/tests/it/ui_env_light_gpu.rs | 1 + crates/ui-egui/tests/it/ui_font_search.rs | 6 +- crates/ui-egui/tests/it/ui_gpu.rs | 1 + crates/ui-egui/tests/it/ui_guide_layers.rs | 3 +- .../tests/it/ui_layer_source_picker.rs | 4 +- crates/ui-egui/tests/it/ui_marker_names.rs | 33 +++++++++-- .../tests/it/ui_media_browser_cache.rs | 3 +- .../tests/it/ui_media_browser_copy_path.rs | 3 +- crates/ui-egui/tests/it/ui_menus.rs | 3 +- crates/ui-egui/tests/it/ui_panels.rs | 3 +- crates/ui-egui/tests/it/ui_presets_folder.rs | 5 +- crates/ui-egui/tests/it/ui_script_console.rs | 3 +- crates/ui-egui/tests/it/ui_scriptui.rs | 4 +- crates/ui-egui/tests/it/ui_tabs.rs | 3 +- crates/ui-egui/tests/it/ui_theme_favorites.rs | 3 +- crates/ui-egui/tests/it/ui_viewer.rs | 11 ++-- crates/ui-egui/tests/it/ui_viewers.rs | 3 +- crates/ui-egui/tests/it/ui_visual_review.rs | 9 +-- crates/ui-egui/tests/it/ui_web_gpu_storage.rs | 1 + crates/ui-egui/tests/it/ui_web_job_gpu.rs | 8 +-- .../ui-egui/tests/it/ui_workflow_selection.rs | 5 +- crates/ui-egui/tests/it/viewer_geometry.rs | 5 +- 31 files changed, 194 insertions(+), 60 deletions(-) create mode 100644 crates/ui-egui/tests/it/gpu_test.rs diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index cc08f223d..15bfa0f72 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -296,21 +296,13 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: hx += 5.0; } } - p.line_segment([pos2(xe, y0), pos2(xe, y0 + h)], Stroke::new(1.0, col)); } - p.add(egui::Shape::convex_polygon(marker_shape(x, y0, h), col, Stroke::NONE)); if !m.comment.is_empty() { // The name sits in a small outlined box beside the flag, as in After Effects. It is drawn in the theme's field // colours, so it follows Dark, Darker and Light. let galley = p.layout_no_wrap(m.comment.lines().next().unwrap_or_default().to_string(), Tokens::ui(10.0), t.text); - let (box_w, mut box_x) = (galley.size().x + 8.0, x + 6.0); - // A duration that ends under the box would be hidden by its fill: the box steps past the end cap instead. - if let Some(xe) = end - && xe > box_x - && xe < box_x + box_w - { - box_x = xe + 3.0; - } + let box_w = galley.size().x + 8.0; + let box_x = name_box_x(x, box_w, end, p.clip_rect()); let box_rect = Rect::from_min_size(pos2(box_x, y0), vec2(box_w, h.max(galley.size().y + 2.0))); p.rect(box_rect, t.radius_sm, t.field_bg, Stroke::new(1.0, t.field_border), egui::StrokeKind::Inside); p.galley(pos2(box_rect.min.x + 4.0, box_rect.center().y - galley.size().y / 2.0), galley, t.text); @@ -385,6 +377,35 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: }); } +fn name_box_x(x: f32, width: f32, end: Option, clip: Rect) -> f32 { + let start = x + 6.0; + if let Some(end) = end + && end > start + && end < start + width + { + let right = end + 3.0; + if right + width <= clip.right() { + return right; + } + let left = x - 6.0 - width; + if left >= clip.left() { + return left; + } + } + start +} + +/// Flags and duration handles sit above every name box in their marker strip. +fn marker_glyphs(p: &egui::Painter, tokens: &Tokens, tm: TMap, marker: &Marker, time: f64, y0: f32, y1: f32) { + let color = marker_color(tokens, marker.label); + let h = (y1 - y0).min(10.0); + p.add(egui::Shape::convex_polygon(marker_shape(tm.x(time), y0, h), color, Stroke::NONE)); + if marker.duration > Tick(0) { + let end = tm.x(time + marker.duration.seconds()); + p.line_segment([pos2(end, y0), pos2(end, y0 + h)], Stroke::new(1.0, color)); + } +} + /// Composition markers in the ruler strip `[y0, y1]`, protected regions over `rows`. pub(crate) fn comp_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &Comp, tm: TMap, strip: Rect, rows: Rect) { let p = ui.painter().with_clip_rect(strip); @@ -398,6 +419,9 @@ pub(crate) fn comp_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, comp: &C for (i, m) in comp.markers.iter().enumerate() { marker(app, ui, &p, comp, tm, m, m.time.seconds(), MarkerRef { layer: None, index: i }, strip.min.y, strip.max.y); } + for m in &comp.markers { + marker_glyphs(&p, &app.tokens, tm, m, m.time.seconds(), strip.min.y, strip.max.y); + } } /// The composition marker bin at the right end of the time ruler (`bin`), as in After Effects @@ -439,6 +463,9 @@ pub(crate) fn layer_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, clip: R for (i, m) in layer.markers.iter().enumerate() { marker(app, ui, &p, comp, tm, m, layer.comp_time(m.time).seconds(), MarkerRef { layer: Some(layer.id.0), index: i }, r.min.y + 3.0, r.max.y - 3.0); } + for m in &layer.markers { + marker_glyphs(&p, &app.tokens, tm, m, layer.comp_time(m.time).seconds(), r.min.y + 3.0, r.max.y - 3.0); + } } /// The nested comp's markers on a precomp layer's bar (`markers.nested`). @@ -488,6 +515,16 @@ fn nested_markers(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter mod tests { use super::*; + #[test] + fn duration_name_box_stays_visible_near_the_right_clip_edge() { + let clip = Rect::from_min_max(pos2(0.0, 0.0), pos2(100.0, 20.0)); + let x = name_box_x(70.0, 20.0, Some(85.0), clip); + assert!(x >= clip.left() && x + 20.0 <= clip.right()); + assert!(x + 20.0 < 70.0 - 4.0, "the left-hand box must not cover its own marker flag"); + assert!(85.0 < x || 85.0 > x + 20.0, "the end cap must remain outside the box"); + assert_eq!(name_box_x(20.0, 20.0, Some(35.0), clip), 38.0); + } + #[test] fn marker_dialog_reads_and_writes_every_field() { let mut s = effectcraft_engine::Session::default(); diff --git a/crates/ui-egui/tests/it/gpu_test.rs b/crates/ui-egui/tests/it/gpu_test.rs new file mode 100644 index 000000000..0156fbb4d --- /dev/null +++ b/crates/ui-egui/tests/it/gpu_test.rs @@ -0,0 +1,43 @@ +//! Shared device-creation gate for the integration-test executable. + +static GPU_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +thread_local! { + static HELD: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +pub(crate) struct Guard { + held: Option>, +} + +impl Drop for Guard { + fn drop(&mut self) { + if self.held.is_some() { + HELD.with(|held| held.set(false)); + } + } +} + +/// GPU-backed harnesses hold this for their whole test; screenshots may enter it again. +pub(crate) fn lock() -> Guard { + let held = if HELD.with(std::cell::Cell::get) { + None + } else { + let guard = GPU_TEST_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + HELD.with(|held| held.set(true)); + Some(guard) + }; + Guard { held } +} + +/// Plain harnesses create their GPU renderer lazily on the first screenshot. +pub(crate) trait RenderSerial { + fn render_serialized(&mut self) -> Result; +} + +impl RenderSerial for egui_kittest::Harness<'_, State> { + fn render_serialized(&mut self) -> Result { + let _device = lock(); + self.render().map_err(|error| format!("{error:?}")) + } +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 7d38e3089..17fe06cae 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -1,5 +1,6 @@ //! The crate's integration tests, one binary (each module is a former `tests/.rs`). +mod gpu_test; mod native_menu_text_keys; mod render_queue_snapshot; mod shared_docking_ui; diff --git a/crates/ui-egui/tests/it/render_queue_snapshot.rs b/crates/ui-egui/tests/it/render_queue_snapshot.rs index d054511af..27054119d 100644 --- a/crates/ui-egui/tests/it/render_queue_snapshot.rs +++ b/crates/ui-egui/tests/it/render_queue_snapshot.rs @@ -2,6 +2,7 @@ //! adapter); run with //! `RQ_SNAPSHOT=/abs/out.png cargo test -p effectcraft-ui-egui --test it render_queue_snapshot:: -- --ignored`. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::render_queue::RenderStatus; use effectcraft_ui_egui::EffectcraftApp; @@ -34,7 +35,7 @@ fn render_queue_panel_snapshot() { h.ctx.data_mut(|d| d.insert_temp(egui::Id::unique("rq-ui"), (ids, None::))); h.state_mut().show_panel(effectcraft_ui_egui::dock::PanelKind::RenderQueue); h.run_steps(4); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("RQ_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/render_queue.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); @@ -57,7 +58,7 @@ fn templates_dialog_snapshot() { let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_| app.take().expect("app")); h.run_steps(4); assert!(h.state().dialog.is_some(), "the Templates dialog is open"); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("RQ_TEMPLATES_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/rq_templates.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_3d.rs b/crates/ui-egui/tests/it/ui_3d.rs index a654f745c..0f7a10b7e 100644 --- a/crates/ui-egui/tests/it/ui_3d.rs +++ b/crates/ui-egui/tests/it/ui_3d.rs @@ -2,6 +2,7 @@ //! with wgpu and writes PNGs when `EC_SNAPSHOT_DIR` is set (`cargo test -p effectcraft-ui-egui //! --test it ui_3d:: -- --ignored`); the other test only runs the UI logic. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::render::three_d::View3D; use effectcraft_ui_egui::EffectcraftApp; @@ -109,6 +110,7 @@ fn camera_and_light_dialogs_create_layers() { #[test] #[ignore = "renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot() { + let _device = crate::gpu_test::lock(); let dir = std::env::var("EC_SNAPSHOT_DIR").ok(); let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).with_pixels_per_point(1.0).wgpu().build_eframe(|_| app()); let shots: [(&str, Option<&str>); 4] = @@ -118,7 +120,7 @@ fn snapshot() { h.state_mut().session.execute(c, json!({})).unwrap(); } settle(&mut h); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_{name}.png")).unwrap(); } @@ -126,14 +128,14 @@ fn snapshot() { let ctx = h.ctx.clone(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.newCamera", json!({})).unwrap(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_camera_dialog.png")).unwrap(); } h.state_mut().dialog = None; effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.newLight", json!({})).unwrap(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_light_dialog.png")).unwrap(); } @@ -143,7 +145,7 @@ fn snapshot() { h.state_mut().session.execute("layer.select", json!({"layers": ["Card Blue"]})).unwrap(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "timeline.reveal.rotation", json!({})).unwrap(); settle(&mut h); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Some(d) = &dir { img.save(format!("{d}/ui3d_reveal_rotation.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_blank_context_menus.rs b/crates/ui-egui/tests/it/ui_blank_context_menus.rs index 52417326f..84080be08 100644 --- a/crates/ui-egui/tests/it/ui_blank_context_menus.rs +++ b/crates/ui-egui/tests/it/ui_blank_context_menus.rs @@ -1,4 +1,5 @@ //! Right-click actions in the blank Project and Timeline areas, and on Timeline property names. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemKind; use effectcraft_ui_egui::dock::WorkspaceExt as _; @@ -20,7 +21,7 @@ fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect { } fn snapshot(h: &mut Harness<'_, EffectcraftApp>, name: &str) { if let Ok(dir) = std::env::var("BLANK_CONTEXT_MENU_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } } fn right_click(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { diff --git a/crates/ui-egui/tests/it/ui_camera_tracker.rs b/crates/ui-egui/tests/it/ui_camera_tracker.rs index f7d349342..1f42879a9 100644 --- a/crates/ui-egui/tests/it/ui_camera_tracker.rs +++ b/crates/ui-egui/tests/it/ui_camera_tracker.rs @@ -3,6 +3,7 @@ //! The solve is a synthetic one written into the effect (the analysis itself is tested in the //! engine and track crates). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::effects::camera_tracker as ct; use effectcraft_engine::track::camtrack::{CameraSolve, ShotType, SolveMethod, SolvedFrame, SolvedPoint}; @@ -217,6 +218,7 @@ fn settle(h: &mut Harness<'_, EffectcraftApp>) { #[test] #[ignore = "analyses the demo's 3D Showcase and renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot_showcase_track() { + let _device = crate::gpu_test::lock(); let dir = std::env::var("EC_SNAPSHOT_DIR").ok(); let mut s = Session::default(); s.execute("file.openDemoProject", json!({})).unwrap(); @@ -235,7 +237,7 @@ fn snapshot_showcase_track() { h.input_mut().events.push(egui::Event::PointerMoved(r.center() + egui::vec2(12.0, 12.0))); h.run_steps(2); if let Some(d) = &dir { - h.render().expect("render").save(format!("{d}/camera_tracker_hover.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{d}/camera_tracker_hover.png")).unwrap(); } let sel: Vec = pts.iter().take(5).filter_map(|i| i.rsplit('.').next()?.parse().ok()).collect(); h.state_mut().session.execute("camera.selectPoints", json!({"points": sel})).unwrap(); @@ -243,6 +245,6 @@ fn snapshot_showcase_track() { h.state_mut().session.execute("layer.select", json!({"layers": ["#3"]})).unwrap(); settle(&mut h); if let Some(d) = &dir { - h.render().expect("render").save(format!("{d}/camera_tracker_solid.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{d}/camera_tracker_solid.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_character_eyedropper.rs b/crates/ui-egui/tests/it/ui_character_eyedropper.rs index ea2578a0d..ea4eecd99 100644 --- a/crates/ui-egui/tests/it/ui_character_eyedropper.rs +++ b/crates/ui-egui/tests/it/ui_character_eyedropper.rs @@ -1,4 +1,5 @@ //! Character Fill and Stroke can sample a displayed Composition pixel. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, commands::text_edit::layer_doc, project::LayerId}; use effectcraft_ui_egui::{ EffectcraftApp, @@ -37,6 +38,7 @@ fn settle(h: &mut Harness<'_, EffectcraftApp>) { #[test] fn character_fill_eyedropper_samples_viewer_and_keeps_text_selection() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180, "duration": 2})).unwrap(); s.execute("layer.newSolid", json!({"name": "Background", "color": "#20c040"})).unwrap(); @@ -64,6 +66,7 @@ fn character_fill_eyedropper_samples_viewer_and_keeps_text_selection() { #[test] fn character_stroke_eyedropper_samples_viewer() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180, "duration": 2})).unwrap(); s.execute("layer.newSolid", json!({"name": "brat green", "color": "#8ACE00"})).unwrap(); @@ -79,7 +82,7 @@ fn character_stroke_eyedropper_samples_viewer() { settle(&mut h); click_auto(&mut h, "character.stroke.eyedropper"); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/character-eyedropper.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/character-eyedropper.png")).unwrap(); } let sample = comp_to_screen(&h.ctx, [20.0, 20.0]).unwrap(); click(&mut h, sample); diff --git a/crates/ui-egui/tests/it/ui_dock_resize.rs b/crates/ui-egui/tests/it/ui_dock_resize.rs index 863fe697e..11ca740cb 100644 --- a/crates/ui-egui/tests/it/ui_dock_resize.rs +++ b/crates/ui-egui/tests/it/ui_dock_resize.rs @@ -1,5 +1,6 @@ //! Shared pane resizing in the full application, including application shortcut ownership. +use crate::gpu_test::RenderSerial; use craft_ui::docking::{Divider, DockArea, Hit}; use effectcraft_engine::Session; use effectcraft_ui_egui::EffectcraftApp; @@ -16,7 +17,7 @@ fn snapshots(h: &mut Harness<'_, EffectcraftApp>, directory: Option<&std::path:: let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, theme); h.run_steps(3); - h.render().unwrap().save(directory.join(format!("effectcraft-{phase}-{name}.png"))).unwrap(); + h.render_serialized().unwrap().save(directory.join(format!("effectcraft-{phase}-{name}.png"))).unwrap(); } let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, ThemeKind::Dark); @@ -32,6 +33,7 @@ fn first_size(h: &Harness<'_, EffectcraftApp>) -> f32 { #[test] fn focused_dock_resize_owns_arrows_and_keeps_pointer_dragging() { + let _device = crate::gpu_test::lock(); let mut session = Session::default(); session.execute("comp.new", json!({"name": "Split test", "width": 160, "height": 90, "duration": 1})).unwrap(); session.execute("layer.newSolid", json!({"name": "Selected", "color": [1.0, 0.0, 0.0, 1.0]})).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_env_light_gpu.rs b/crates/ui-egui/tests/it/ui_env_light_gpu.rs index 171cc5728..71e4fae67 100644 --- a/crates/ui-egui/tests/it/ui_env_light_gpu.rs +++ b/crates/ui-egui/tests/it/ui_env_light_gpu.rs @@ -42,6 +42,7 @@ fn same_adapter(name: String) -> eframe::egui_wgpu::WgpuSetup { /// viewer's GPU frames change when the light is added. #[test] fn environment_light_lights_the_scene_on_the_gpu() { + let _device = crate::gpu_test::lock(); let Some(probe) = effectcraft_gpu::Gpu::headless() else { eprintln!("no GPU adapter: skipping"); return; diff --git a/crates/ui-egui/tests/it/ui_font_search.rs b/crates/ui-egui/tests/it/ui_font_search.rs index 77984cf6b..44b650be4 100644 --- a/crates/ui-egui/tests/it/ui_font_search.rs +++ b/crates/ui-egui/tests/it/ui_font_search.rs @@ -1,4 +1,5 @@ //! Real keyboard/pointer coverage for the font menus in both text panels. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, commands::text_edit::layer_doc, project::LayerId}; use effectcraft_ui_egui::{EffectcraftApp, dock::PanelKind}; use egui::{Event, Key, Modifiers, PointerButton}; @@ -29,6 +30,7 @@ fn type_text(h: &mut Harness<'_, EffectcraftApp>, text: &str) { #[test] fn font_search_filters_selects_and_undoes_in_both_panels() { + let _device = crate::gpu_test::lock(); for (panel, prefix) in [(PanelKind::Character, "character.font"), (PanelKind::Properties, "properties.text.font")] { let mut s = Session::default(); s.execute("comp.new", json!({"width": 320, "height": 180})).unwrap(); @@ -56,7 +58,7 @@ fn font_search_filters_selects_and_undoes_in_both_panels() { assert!(!rows.is_empty()); assert!(rows.iter().all(|r| r.label.to_lowercase().contains("jetbrains"))); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/{prefix}-search.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{prefix}-search.png")).unwrap(); } assert_eq!(layer_doc(&h.state().session, LayerId(lid)).unwrap().font, "Inter", "typing only filters"); key(&mut h, Key::Enter); @@ -76,7 +78,7 @@ fn font_search_filters_selects_and_undoes_in_both_panels() { // Reopening clears the previous query, and mouse selection still works. h.run_steps(2); if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().unwrap().save(format!("{dir}/{prefix}-reopened.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{prefix}-reopened.png")).unwrap(); } assert!(h.state().auto.query(&format!("{prefix}.option.")).len() > 1); type_text(&mut h, "JetBrains"); diff --git a/crates/ui-egui/tests/it/ui_gpu.rs b/crates/ui-egui/tests/it/ui_gpu.rs index 355c35bf5..c8cdc428a 100644 --- a/crates/ui-egui/tests/it/ui_gpu.rs +++ b/crates/ui-egui/tests/it/ui_gpu.rs @@ -41,6 +41,7 @@ fn same_adapter(name: String) -> eframe::egui_wgpu::WgpuSetup { #[test] fn viewer_draws_gpu_frames_that_match_the_cpu() { + let _device = crate::gpu_test::lock(); let Some(probe) = effectcraft_gpu::Gpu::headless() else { eprintln!("no GPU adapter: skipping"); return; diff --git a/crates/ui-egui/tests/it/ui_guide_layers.rs b/crates/ui-egui/tests/it/ui_guide_layers.rs index 37158658f..92196f5f1 100644 --- a/crates/ui-egui/tests/it/ui_guide_layers.rs +++ b/crates/ui-egui/tests/it/ui_guide_layers.rs @@ -1,6 +1,7 @@ //! Guide Layer in the layer context menus (Timeline rows and the Composition viewer), a plain item //! (no check mark) that toggles the switch on the selection, and the guide mark by a guide layer's //! name. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::LayerId; use effectcraft_ui_egui::EffectcraftApp; @@ -45,7 +46,7 @@ fn guide(h: &Harness<'_, EffectcraftApp>, id: LayerId) -> bool { fn snapshot(h: &mut Harness<'_, EffectcraftApp>, name: &str) { if let Ok(dir) = std::env::var("GUIDE_LAYER_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_layer_source_picker.rs b/crates/ui-egui/tests/it/ui_layer_source_picker.rs index 78a0f8821..ca10e4249 100644 --- a/crates/ui-egui/tests/it/ui_layer_source_picker.rs +++ b/crates/ui-egui/tests/it/ui_layer_source_picker.rs @@ -3,6 +3,7 @@ //! Effect Controls' layer parameters (a Displacement Map's layer) pick through the same widget, //! and both write an undoable `prop.set`. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::keyframe::Value; use effectcraft_engine::project::LayerId; @@ -183,6 +184,7 @@ fn effect_controls_layer_param_picks_through_the_shared_widget() { #[test] #[ignore = "renders with wgpu; run with --ignored (set EC_SNAPSHOT_DIR to keep PNGs)"] fn snapshot_source_dropdown_open_and_chosen() { + let _device = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Main", "width": 1280, "height": 720, "frameRate": 30, "duration": 5})).unwrap(); s.execute("comp.settings", json!({"renderer": "advanced3D"})).unwrap(); @@ -200,7 +202,7 @@ fn snapshot_source_dropdown_open_and_chosen() { h.run_steps(4); let save = |h: &mut Harness<'_, EffectcraftApp>, name: &str| { if let Ok(dir) = std::env::var("EC_SNAPSHOT_DIR") { - h.render().expect("render").save(format!("{dir}/{name}")).expect("save"); + h.render_serialized().expect("render").save(format!("{dir}/{name}")).expect("save"); } }; save(&mut h, "source_unset.png"); diff --git a/crates/ui-egui/tests/it/ui_marker_names.rs b/crates/ui-egui/tests/it/ui_marker_names.rs index 99adf41f3..190bd77bb 100644 --- a/crates/ui-egui/tests/it/ui_marker_names.rs +++ b/crates/ui-egui/tests/it/ui_marker_names.rs @@ -8,16 +8,13 @@ use effectcraft_ui_egui::theme::ThemeKind; use egui_kittest::Harness; use serde_json::json; -/// One render at a time: two tests creating a GPU device together crash some Vulkan loaders. -static RENDER: std::sync::Mutex<()> = std::sync::Mutex::new(()); - fn close(got: [u8; 4], want: egui::Color32) -> bool { got[..3].iter().zip(want.to_array()).all(|(a, b)| a.abs_diff(b) <= 3) } #[test] fn the_name_box_follows_the_theme() { - let _one_at_a_time = RENDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _one_at_a_time = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Song", "width": 640, "height": 360, "duration": 4})).unwrap(); s.execute("layer.newSolid", json!({"name": "Bed", "color": "#204060", "width": 640, "height": 360})).unwrap(); @@ -58,7 +55,7 @@ fn the_name_box_follows_the_theme() { /// stays visible: the cap is drawn, the bar shows left of it, and the box starts after it. #[test] fn a_short_duration_stays_visible_beside_the_name_box() { - let _one_at_a_time = RENDER.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _one_at_a_time = crate::gpu_test::lock(); let mut s = Session::default(); s.execute("comp.new", json!({"name": "Song", "width": 640, "height": 360, "duration": 4})).unwrap(); s.execute("layer.newSolid", json!({"name": "Bed", "color": "#204060", "width": 640, "height": 360})).unwrap(); @@ -95,3 +92,29 @@ fn a_short_duration_stays_visible_beside_the_name_box() { let fill = at(xe + 3.0 + 2.5, mid); assert!(close(fill, tokens.field_bg), "the box begins after the end cap: {fill:?}, field_bg {:?}", tokens.field_bg); } + +#[test] +fn a_later_name_box_does_not_hide_an_earlier_duration_cap() { + let _one_at_a_time = crate::gpu_test::lock(); + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 640, "height": 360, "duration": 4})).unwrap(); + s.execute("markers.set", json!({"new": true, "time": 1.0, "duration": 0.5, "label": "Red"})).unwrap(); + s.execute("markers.set", json!({"new": true, "time": 1.3, "comment": "This name overlaps the earlier endpoint", "label": "Blue"})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(5); + let a = h.state().auto.find("timeline.marker.0").unwrap().rect; + let b = h.state().auto.find("timeline.marker.1").unwrap().rect; + let x = |r: [f32; 4]| r[0] + r[2] / 2.0; + let end = x(a) + (x(b) - x(a)) * 0.5 / 0.3; + let y = a[1] + 1.0 + (a[3] - 1.0).min(10.0) / 2.0; + let image = h.render().unwrap(); + if let Some(dir) = std::env::var_os("MARKER_NAMES_SNAPSHOT") { + image.save(std::path::Path::new(&dir).join("marker-overlapping-cap.png")).unwrap(); + } + let scale = image.width() as f32 / 1600.0; + let cap = (-1..=1) + .map(|offset| image.get_pixel(((end + offset as f32) * scale) as u32, (y * scale) as u32).0) + .max_by_key(|pixel| i32::from(pixel[0]) - i32::from(pixel[1])) + .unwrap(); + assert!(i32::from(cap[0]) - i32::from(cap[1]) > 40, "the earlier red duration cap stays above the later name: {cap:?}"); +} diff --git a/crates/ui-egui/tests/it/ui_media_browser_cache.rs b/crates/ui-egui/tests/it/ui_media_browser_cache.rs index ddfc53e22..44ca9dd81 100644 --- a/crates/ui-egui/tests/it/ui_media_browser_cache.rs +++ b/crates/ui-egui/tests/it/ui_media_browser_cache.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use std::sync::{ Arc, Mutex, atomic::{AtomicBool, Ordering}, @@ -140,6 +141,6 @@ fn browser_cache_refreshes_filters_navigates_and_recovers_from_errors() { assert_eq!(names(&listing(&h, false).1), ["Sub", "Added.csv", "Clip.csv", "From action.csv"]); assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_some()); if let Ok(path) = std::env::var("MEDIA_BROWSER_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs b/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs index dff60ddd0..8372b2be5 100644 --- a/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs +++ b/crates/ui-egui/tests/it/ui_media_browser_copy_path.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use std::sync::Arc; use effectcraft_engine::{ @@ -74,7 +75,7 @@ fn file_folder_and_virtual_paths_copy_from_the_context_menu_without_document_cha if name == "作品 clip.csv" && let Ok(file) = std::env::var("MEDIA_BROWSER_COPY_SNAPSHOT") { - h.render().unwrap().save(file).unwrap(); + h.render_serialized().unwrap().save(file).unwrap(); } assert_eq!(click(&mut h, &id, PointerButton::Primary), [path]); h.run_steps(3); diff --git a/crates/ui-egui/tests/it/ui_menus.rs b/crates/ui-egui/tests/it/ui_menus.rs index 424b75c48..ce0cbab1a 100644 --- a/crates/ui-egui/tests/it/ui_menus.rs +++ b/crates/ui-egui/tests/it/ui_menus.rs @@ -1,6 +1,7 @@ //! The menu bar from the keyboard, and the app's shortcuts waiting while a menu is open (#279) //! (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::EffectcraftApp; use egui::{Event, Key, Modifiers, Pos2, Rect, pos2, vec2}; @@ -226,7 +227,7 @@ fn relink_is_reachable_through_the_dependencies_menu() { h.run_steps(8); let relink = h.query_by_label_contains("Relink Missing Footage").unwrap().rect(); if let Some(path) = std::env::var_os("EFFECTCRAFT_RELINK_MENU_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } click_at(&mut h, relink.center()); assert!(picked.get()); diff --git a/crates/ui-egui/tests/it/ui_panels.rs b/crates/ui-egui/tests/it/ui_panels.rs index db9ef7373..a29d3e358 100644 --- a/crates/ui-egui/tests/it/ui_panels.rs +++ b/crates/ui-egui/tests/it/ui_panels.rs @@ -2,6 +2,7 @@ //! registers its automation ids; the Footage panel's buttons edit into the comp; the Progress //! panel lists and cancels a job; Lumetri Scopes switch scope through their dropdown. +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::dock::WorkspaceExt as _; use std::sync::Arc; @@ -448,7 +449,7 @@ fn panels_snapshot() { } open(&mut h, panel); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); img.save(format!("{dir}/{panel}{scope}.png")).unwrap(); } } diff --git a/crates/ui-egui/tests/it/ui_presets_folder.rs b/crates/ui-egui/tests/it/ui_presets_folder.rs index e11efe944..1bcd9189c 100644 --- a/crates/ui-egui/tests/it/ui_presets_folder.rs +++ b/crates/ui-egui/tests/it/ui_presets_folder.rs @@ -1,5 +1,6 @@ //! Effects & Presets: copy the configured folder path through the contents menu. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::panels::effects_presets::PresetFile; use effectcraft_ui_egui::{ControlRequest, EffectcraftApp, control, dock::PanelKind}; @@ -52,7 +53,7 @@ fn presets_menu_copies_the_configured_folder_without_editing_the_project() { click(&mut h, "effects.menu"); h.run_steps(3); if let Ok(path) = std::env::var("PRESETS_FOLDER_SNAPSHOT") { - h.render().unwrap().save(path).unwrap(); + h.render_serialized().unwrap().save(path).unwrap(); } assert_eq!(click(&mut h, "effects.copyPresetsFolderPath"), [expected.as_str()]); h.run_steps(2); @@ -156,7 +157,7 @@ fn shared_preset_rows_search_apply_and_undo_the_selected_definition() { assert_eq!(h.state().session.active_comp().unwrap().layers[0].props.prop("transform/opacity").unwrap().value.as_f64(), expected); assert_eq!(h.state().session.history.undo.len(), history + 1); if let Ok(dir) = std::env::var("PRESET_CACHE_SNAPSHOTS") { - h.render().unwrap().save(format!("{dir}/{name}.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/{name}.png")).unwrap(); } h.state_mut().session.execute("edit.undo", json!({})).unwrap(); h.run_steps(2); diff --git a/crates/ui-egui/tests/it/ui_script_console.rs b/crates/ui-egui/tests/it/ui_script_console.rs index e705a20b7..9430bc7a2 100644 --- a/crates/ui-egui/tests/it/ui_script_console.rs +++ b/crates/ui-egui/tests/it/ui_script_console.rs @@ -1,6 +1,7 @@ //! Window ▸ Script Console: the panel shows, runs JavaScript through `script.run` (persistent //! context) and logs output, results and errors (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::EffectcraftApp; use effectcraft_ui_egui::dock::PanelKind; use egui_kittest::Harness; @@ -59,7 +60,7 @@ fn script_console_snapshot() { } h.state_mut().ui.script_console.input = "app.project.numItems".into(); h.run_steps(3); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); let out = std::env::var("SC_SNAPSHOT").unwrap_or_else(|_| concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out/script_console.png").into()); if let Some(d) = std::path::Path::new(&out).parent() { std::fs::create_dir_all(d).unwrap(); diff --git a/crates/ui-egui/tests/it/ui_scriptui.rs b/crates/ui-egui/tests/it/ui_scriptui.rs index 09986f6d1..6199462d0 100644 --- a/crates/ui-egui/tests/it/ui_scriptui.rs +++ b/crates/ui-egui/tests/it/ui_scriptui.rs @@ -2,6 +2,7 @@ //! windows (clicks reach the script's handlers), the branching History panel, and the Window / //! File ▸ Scripts menus listing scripts (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_ui_egui::EffectcraftApp; use effectcraft_ui_egui::dock::PanelKind; use effectcraft_ui_egui::dock::WorkspaceExt as _; @@ -210,6 +211,7 @@ fn history_panel_jumps_between_branches() { /// onDraw fills of concave paths and real images (drawImage, image controls), rendered. #[test] fn on_draw_concave_fills_and_images_render() { + let _device = crate::gpu_test::lock(); let dir = std::env::temp_dir().join(format!("effectcraft-scriptui-img-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let png = dir.join("green.png"); @@ -244,7 +246,7 @@ fn on_draw_concave_fills_and_images_render() { let g = &win.root.children[0]; assert!(matches!(&g.draw[1], effectcraft_engine::scriptui::DrawOp::Image { image: Some(i), .. } if i.src.as_deref() == Some(png.as_str()))); assert_eq!(win.root.children[1].image.as_ref().and_then(|i| i.src.clone()).as_deref(), Some(png.as_str())); - let img = h.render().expect("render"); + let img = h.render_serialized().expect("render"); if let Ok(d) = std::env::var("EC_SNAPSHOT_DIR") { img.save(format!("{d}/scriptui_paint.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_tabs.rs b/crates/ui-egui/tests/it/ui_tabs.rs index 5fc3d8598..cc9e56041 100644 --- a/crates/ui-egui/tests/it/ui_tabs.rs +++ b/crates/ui-egui/tests/it/ui_tabs.rs @@ -2,6 +2,7 @@ //! as its own Timeline tab (not a rename, and not in place of the comp shown), the tabs switch and //! close comps, and a dragged tab lands where its insertion mark shows. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemId; use effectcraft_ui_egui::EffectcraftApp; @@ -135,7 +136,7 @@ fn tab_drag_snapshot() { } let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out"); std::fs::create_dir_all(dir).unwrap(); - h.render().expect("render").save(format!("{dir}/tab-drag.png")).unwrap(); + h.render_serialized().expect("render").save(format!("{dir}/tab-drag.png")).unwrap(); } /// A precomp layer's bar shows its comp's markers; a double-click opens that comp at the marker. diff --git a/crates/ui-egui/tests/it/ui_theme_favorites.rs b/crates/ui-egui/tests/it/ui_theme_favorites.rs index 1b700f51f..424b28c49 100644 --- a/crates/ui-egui/tests/it/ui_theme_favorites.rs +++ b/crates/ui-egui/tests/it/ui_theme_favorites.rs @@ -1,3 +1,4 @@ +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, config::MemoryConfig, prefs::Prefs}; use effectcraft_ui_egui::{EffectcraftApp, menus}; use egui::{Event, Modifiers, PointerButton, Pos2, pos2, vec2}; @@ -49,7 +50,7 @@ fn star_controls_save_and_favorite_menu_selects_the_family() { assert_eq!(h.state().session.prefs.appearance.favorite_themes, vec!["light"]); if let Some(path) = std::env::var_os("FAVORITES_SCREENSHOT_DIR") { std::fs::create_dir_all(&path).unwrap(); - h.render().unwrap().save(std::path::Path::new(&path).join("effectcraft-favorites.png")).unwrap(); + h.render_serialized().unwrap().save(std::path::Path::new(&path).join("effectcraft-favorites.png")).unwrap(); } assert_eq!(h.state().session.prefs.appearance.appearance_mode, "dark"); let p = at(&h, "settings.ok"); diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index 41cf5fe43..48bc36a44 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -2,6 +2,7 @@ //! interest drawing, rulers and guides, snapping, the shape Pen and motion-path key drags //! (egui_kittest, UI logic only). +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::commands::shape_tool::PaintKind; use effectcraft_engine::project::LayerId; @@ -448,7 +449,7 @@ fn mid_drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2, mods: egu h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (i as f32 / 8.0))); h.step(); } - h.render().expect("the mid-drag frame") + h.render_serialized().expect("the mid-drag frame") } /// Release the button where the drag left it. @@ -490,7 +491,7 @@ fn shape_drag_ghost_shows_the_tools_own_shape_while_dragging() { h.state_mut().ui.tool = tool; settle_viewer(&mut h); let (a, b) = (screen(&h, from), screen(&h, to)); - let before = h.render().expect("the frame before the drag"); + let before = h.render_serialized().expect("the frame before the drag"); let mid = mid_drag(&mut h, a, b, egui::Modifiers::default()); let fill = fill_rgb(&h); @@ -526,7 +527,7 @@ fn shape_drag_ghost_normalises_the_same_way_as_the_release() { h.state_mut().ui.tool = Tool::Ellipse; settle_viewer(&mut h); let (a, b) = (screen(&h, from.map(|v| v as f32)), screen(&h, to.map(|v| v as f32))); - let before = h.render().expect("the frame before the drag"); + let before = h.render_serialized().expect("the frame before the drag"); // Measured with the button down, so this is the ghost and not the committed shape. let mid = mid_drag(&mut h, a, b, mods); let on = [centre[0] as f32 + size[0] as f32 / 4.0, centre[1] as f32 + size[1] as f32 / 4.0]; @@ -595,7 +596,7 @@ fn a_layer_drag_moves_the_picture_while_the_button_is_down() { h.input_mut().events.push(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); h.step(); // Where the layer is not (yet): the plate behind it. - let plate = px_at(&h.render().unwrap(), &h, [440.0, 200.0]); + let plate = px_at(&h.render_serialized().unwrap(), &h, [440.0, 200.0]); // Drag in small steps, still holding the button, and give the renders a chance to land. let mut moved = false; @@ -606,7 +607,7 @@ fn a_layer_drag_moves_the_picture_while_the_button_is_down() { std::thread::sleep(std::time::Duration::from_millis(15)); // 30 comp px behind the pointer: inside the layer, well away from its outline, its anchor // icon and where it started. - let at = px_at(&h.render().expect("mid-drag frame"), &h, [320.0 + 200.0 * (i as f32 / 40.0) - 30.0, 200.0]); + let at = px_at(&h.render_serialized().expect("mid-drag frame"), &h, [320.0 + 200.0 * (i as f32 / 40.0) - 30.0, 200.0]); if i >= 30 && delta(at, plate) > 30 { moved = true; break; diff --git a/crates/ui-egui/tests/it/ui_viewers.rs b/crates/ui-egui/tests/it/ui_viewers.rs index 833ab0a60..bfbcad51e 100644 --- a/crates/ui-egui/tests/it/ui_viewers.rs +++ b/crates/ui-egui/tests/it/ui_viewers.rs @@ -2,6 +2,7 @@ //! comp goes to an unlocked viewer (a new one when every viewer is locked); a click on another //! viewer makes it the active one; closing a viewer forgets it. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_engine::project::ItemId; use effectcraft_ui_egui::EffectcraftApp; @@ -171,5 +172,5 @@ fn viewers_snapshot() { h.run_steps(4); let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/../../target/test-out"); std::fs::create_dir_all(dir).unwrap(); - h.render().unwrap().save(format!("{dir}/viewers.png")).unwrap(); + h.render_serialized().unwrap().save(format!("{dir}/viewers.png")).unwrap(); } diff --git a/crates/ui-egui/tests/it/ui_visual_review.rs b/crates/ui-egui/tests/it/ui_visual_review.rs index 70cc5ed84..6d079f1f5 100644 --- a/crates/ui-egui/tests/it/ui_visual_review.rs +++ b/crates/ui-egui/tests/it/ui_visual_review.rs @@ -1,5 +1,6 @@ //! Opt-in visual review of every built-in panel, workspaces and common dialogs. //! Original fixture; captures are review outputs, not shipped assets. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::{Dialog, EffectcraftApp, dock::PanelKind}; use egui_kittest::Harness; @@ -19,18 +20,18 @@ fn all_ui_review_captures() { std::sync::Arc::make_mut(&mut s.project).settings.gpu_acceleration = false; let mut h = Harness::builder().with_size(size).build_eframe(|_| EffectcraftApp::new(s)); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-workspace.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-workspace.png"))).unwrap(); let ctx = h.ctx.clone(); h.state_mut().set_theme(&ctx, effectcraft_ui_egui::theme::ThemeKind::Light); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-workspace-light.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-workspace-light.png"))).unwrap(); h.state_mut().set_theme(&ctx, effectcraft_ui_egui::theme::ThemeKind::Dark); for p in PanelKind::ALL { h.state_mut().show_panel(p); h.state_mut().ui.maximized = Some(p); h.run_steps(4); assert!(h.state().auto.find(&format!("panel.{}", p.id())).is_some(), "{} opens", p.title()); - h.render().unwrap().save(out.join(format!("{label}-{}.png", p.id()))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-{}.png", p.id()))).unwrap(); } h.state_mut().ui.maximized = None; for (name, dialog) in [ @@ -42,7 +43,7 @@ fn all_ui_review_captures() { ] { h.state_mut().dialog = Some(dialog); h.run_steps(4); - h.render().unwrap().save(out.join(format!("{label}-dialog-{name}.png"))).unwrap(); + h.render_serialized().unwrap().save(out.join(format!("{label}-dialog-{name}.png"))).unwrap(); h.state_mut().dialog = None; h.run_steps(2); } diff --git a/crates/ui-egui/tests/it/ui_web_gpu_storage.rs b/crates/ui-egui/tests/it/ui_web_gpu_storage.rs index a742226de..024bd1cd0 100644 --- a/crates/ui-egui/tests/it/ui_web_gpu_storage.rs +++ b/crates/ui-egui/tests/it/ui_web_gpu_storage.rs @@ -122,6 +122,7 @@ fn cpu_pixels(f: &FrameImage) -> Vec { #[test] fn gpu_worker_renders_effect_frames_and_the_disk_cache_survives_a_reload() { + let _device = crate::gpu_test::lock(); let (Some(page_gpu), Some(worker_gpu)) = (Gpu::headless(), Gpu::headless_deferred()) else { eprintln!("no GPU adapter: skipped"); return; diff --git a/crates/ui-egui/tests/it/ui_web_job_gpu.rs b/crates/ui-egui/tests/it/ui_web_job_gpu.rs index 9060e1067..8b8914abc 100644 --- a/crates/ui-egui/tests/it/ui_web_job_gpu.rs +++ b/crates/ui-egui/tests/it/ui_web_job_gpu.rs @@ -10,10 +10,6 @@ use effectcraft_engine::offload::{Inbox, Offload, Post, WorkerReply, WorkerReque use effectcraft_gpu::Gpu; use serde_json::json; -/// One device at a time in this test binary: wgpu's OpenGL backend (Mesa llvmpipe on FreeBSD, -/// Linux without Vulkan) panics or crashes when tests create and drive devices concurrently. -static GPU_LOCK: Mutex<()> = Mutex::new(()); - /// Holds offloaded jobs until the test runs them. #[derive(Default)] struct Manual(Mutex)>>); @@ -63,7 +59,7 @@ fn page() -> (Session, Arc) { #[test] fn render_queue_jobs_render_on_the_worker_gpu() { - let _gpu = GPU_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _gpu = crate::gpu_test::lock(); let Some(g) = Gpu::headless_deferred() else { eprintln!("no GPU adapter: skipped"); return; @@ -102,7 +98,7 @@ fn render_queue_jobs_render_on_the_worker_gpu() { #[test] fn analyses_render_their_input_on_the_worker_gpu() { - let _gpu = GPU_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let _gpu = crate::gpu_test::lock(); let Some(g) = Gpu::headless_deferred() else { eprintln!("no GPU adapter: skipped"); return; diff --git a/crates/ui-egui/tests/it/ui_workflow_selection.rs b/crates/ui-egui/tests/it/ui_workflow_selection.rs index 121c2b888..e7bcedb60 100644 --- a/crates/ui-egui/tests/it/ui_workflow_selection.rs +++ b/crates/ui-egui/tests/it/ui_workflow_selection.rs @@ -1,4 +1,5 @@ //! User-reported selection, Project deletion, and render-queue drop regressions. +use crate::gpu_test::RenderSerial; use effectcraft_engine::{Session, project::LayerId}; use effectcraft_ui_egui::{Dialog, EffectcraftApp, dock::PanelKind}; use egui::{Event, Modifiers, PointerButton, Pos2, Rect, pos2, vec2}; @@ -273,7 +274,7 @@ fn viewer_right_click_targets_hit_and_duplicate_preserves_group() { } assert!(h.state().frames.is_cached(&key)); h.run_steps(3); - h.render().expect("offscreen context menu render").save(out).unwrap(); + h.render_serialized().expect("offscreen context menu render").save(out).unwrap(); } h.get_by_label("Duplicate").click(); h.run_steps(3); @@ -433,7 +434,7 @@ fn workflow_snapshot() { } assert!(h.state().frames.is_cached(&key), "current composition rendered before capture"); h.run_steps(3); - h.render().expect("offscreen UI render").save(out).unwrap(); + h.render_serialized().expect("offscreen UI render").save(out).unwrap(); } /// Filled rects painted this frame (flattening nested shape lists). diff --git a/crates/ui-egui/tests/it/viewer_geometry.rs b/crates/ui-egui/tests/it/viewer_geometry.rs index e3e4128e8..d57a8aa9e 100644 --- a/crates/ui-egui/tests/it/viewer_geometry.rs +++ b/crates/ui-egui/tests/it/viewer_geometry.rs @@ -1,5 +1,6 @@ //! Inspect the actual viewer's painted colors and pointer-driven control geometry. +use crate::gpu_test::RenderSerial; use effectcraft_engine::Session; use effectcraft_ui_egui::{ EffectcraftApp, @@ -269,10 +270,10 @@ fn viewer_theme_and_width_visual_matrix() { } fn save_stable_snapshot(h: &mut Harness<'_, EffectcraftApp>, path: &str, width: f32) { - let image = h.render().unwrap(); + let image = h.render_serialized().unwrap(); image.save(path).unwrap(); h.run_steps(3); - let repeated = h.render().unwrap(); + let repeated = h.render_serialized().unwrap(); assert!(image == repeated, "settled synthetic viewer pixels must repeat exactly: {path}"); if width == 900.0 { let times: Vec<_> = h From 528a9ff7430743a7e2999bad05b7832ba2678edf Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 02:58:56 +0100 Subject: [PATCH 6/8] #1805: keep ordinary marker names inside the viewport too --- crates/ui-egui/src/panels/markers_ui.rs | 27 ++++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index 15bfa0f72..03c45d32b 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -379,18 +379,16 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: fn name_box_x(x: f32, width: f32, end: Option, clip: Rect) -> f32 { let start = x + 6.0; - if let Some(end) = end - && end > start - && end < start + width - { - let right = end + 3.0; - if right + width <= clip.right() { - return right; - } - let left = x - 6.0 - width; - if left >= clip.left() { - return left; - } + let right = match end { + Some(end) if end > start && end < start + width => end + 3.0, + _ => start, + }; + if right + width <= clip.right() { + return right; + } + let left = x - 6.0 - width; + if left >= clip.left() { + return left; } start } @@ -523,6 +521,11 @@ mod tests { assert!(x + 20.0 < 70.0 - 4.0, "the left-hand box must not cover its own marker flag"); assert!(85.0 < x || 85.0 > x + 20.0, "the end cap must remain outside the box"); assert_eq!(name_box_x(20.0, 20.0, Some(35.0), clip), 38.0); + for end in [None, Some(72.0), Some(110.0)] { + let x = name_box_x(90.0, 20.0, end, clip); + assert!(x >= clip.left() && x + 20.0 <= clip.right(), "all right-edge names must remain visible: {end:?}"); + assert!(x + 20.0 < 90.0 - 4.0); + } } #[test] From f3d46b0fa3a1ce9a871802ee1a2d96c83979e498 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 29 Oct 2026 22:21:27 -0400 Subject: [PATCH 7/8] deps: bump tar, zip, roxmltree, rustls-webpki and symphonia to their latest versions (#2633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `roxmltree`: 0.21.1 → 0.21.2 - `zip`: 9.1.0 → 9.1.3 Lockfile only. `symphonia` deliberately stays 0.5.5: effectcraft names it only to enable AIFF in filmcraft-codecs' symphonia, which is 0.5.5 at the pinned filmcraft rev. Moving to 0.6 waits for that pin to move. --- Cargo.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 629fc65e9..5f4551c19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4292,7 +4292,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" dependencies = [ "libc", - "windows-sys 0.45.0", + "windows-sys 0.61.2", ] [[package]] @@ -4884,9 +4884,9 @@ dependencies = [ [[package]] name = "roxmltree" -version = "0.21.1" +version = "0.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1964b10c76125c36f8afe190065a4bf9a87bf324842c05701330bba9f1cacbb" +checksum = "9da7f59328b09405b181fee1018925fad50f157afb43c0c0f5c522c256dd5907" dependencies = [ "memchr", ] @@ -7469,9 +7469,9 @@ dependencies = [ [[package]] name = "zip" -version = "9.1.0" +version = "9.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2886b00003a72c4ccee3f42cfc34908d33b2508a74d827dc2a4244706475c704" +checksum = "1156b39d142a34b999c2310a50455e0cb3760bf7efad0122f796d813bee26ccb" dependencies = [ "crc32fast", "flate2", From 36e18cfe6addf8d465a0bf5602d2b1ba1b301325 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 29 Oct 2026 22:25:26 -0400 Subject: [PATCH 8/8] CI: workflows only call craft-common-actions at @main (#2625) AGENTS.md (and CLAUDE.md where present) gets a CI workflows section: workflows only call craft-common-actions at @main, with no other jobs; feature and fix PRs don't touch CI and link or open a craft-common-actions issue (a PR only for a small fix that is safe for every app and does not increase CI time); legacy jobs aren't added to and move only as their own task, staying until a shared workflow runs them. windows-arm64.yml moves from its SHA pin to @main. The pinned revision is on craft-common-actions main and the workflow (and the actions it calls) is identical there, so nothing changes in behaviour. --- .github/workflows/windows-arm64.yml | 4 ++-- AGENTS.md | 5 +++++ CLAUDE.md | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/windows-arm64.yml b/.github/workflows/windows-arm64.yml index 2a8144a5b..2cedc1a57 100644 --- a/.github/workflows/windows-arm64.yml +++ b/.github/workflows/windows-arm64.yml @@ -28,8 +28,8 @@ permissions: jobs: windows-arm64: - # Immutable supporting workflow revision until the new install contract lands upstream. - uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@fa5ef18780cf5e7d5d3b8de068de56cffc83cac5 + # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. + uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@main with: app-name: effectcraft app-dir: EffectCraft diff --git a/AGENTS.md b/AGENTS.md index 24abee6a1..24287ff9a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,3 +144,8 @@ as PhotoCraft does with `cargo xtask corpus`. GitHub profiles). Never hand-edit `contributors.json`. - Maintainers refresh the stats with `python3 ../../craftrules/scripts/contributors.py .` (it also re-verifies who wrote each `people.toml` entry; `--check` only verifies). + +## CI workflows +- Workflows in `.github/workflows/` call the shared workflows in [`storytold/craft-common-actions`](https://github.com/storytold/craft-common-actions) at `@main`, with their inputs and secrets. No other jobs: if no shared workflow does what's needed, it doesn't go here. +- Don't add or change CI in a feature or fix PR. If the existing workflows can't build or test a change, link an existing craft-common-actions issue or PR, or open a new issue, or a PR only for a small fix that is safe for every app and does not increase CI time. +- Some workflow files here still run their own jobs. Don't add to them. Moving a job into craft-common-actions is its own task, never part of another PR, and the job stays here until a shared workflow runs it. diff --git a/CLAUDE.md b/CLAUDE.md index 21f2df36b..7bd0e7422 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,3 +30,8 @@ EffectCraft is a clean-room, open-source, pure-Rust motion graphics and visual e - For UI work, **look at the result**. Headless (no window, works while the user is busy on the machine): `cargo run -p effectcraft-ui-egui --example snapshot -- --out ui.png --step '{"method":"engine.execute","params":{"command":"layer.select","params":{"layers":["#2"]}}}'` (steps are control-channel requests; `--script steps.jsonl`; `{"method":"snap","params":{"path":…}}` for intermediate shots). Or drive the live app via the control channel and take `ui.screenshot` (needs the window visible on screen). - After Effects reference: drive AE via ExtendScript (`osascript -e 'tell application "Adobe After Effects 2026" to DoScriptFile "x.jsx"'`) and capture only AE's window (`screencapture -l `); keep captures in `plan/aftereffects/ref/` (gitignored), never commit them, never capture the Home screen. - Parallel agents: separate git worktrees and `CARGO_TARGET_DIR=target/agent-`; keep every `Cargo.toml` valid (the `crates/*` glob). + +## CI workflows +- Workflows in `.github/workflows/` call the shared workflows in [`storytold/craft-common-actions`](https://github.com/storytold/craft-common-actions) at `@main`, with their inputs and secrets. No other jobs: if no shared workflow does what's needed, it doesn't go here. +- Don't add or change CI in a feature or fix PR. If the existing workflows can't build or test a change, link an existing craft-common-actions issue or PR, or open a new issue, or a PR only for a small fix that is safe for every app and does not increase CI time. +- Some workflow files here still run their own jobs. Don't add to them. Moving a job into craft-common-actions is its own task, never part of another PR, and the job stays here until a shared workflow runs it.