From 0547c51e476bc161262a09f4124c7562feb227ba Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 14:33:21 +0530 Subject: [PATCH 1/5] #3538: test that OCIO transform files and configs are read again once they change --- crates/effects/src/ocio/tests.rs | 40 +++++++++++++++++++++++ crates/effects/src/ocio_config/tests.rs | 42 +++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/crates/effects/src/ocio/tests.rs b/crates/effects/src/ocio/tests.rs index fe93dd25a..e17fd782a 100644 --- a/crates/effects/src/ocio/tests.rs +++ b/crates/effects/src/ocio/tests.rs @@ -396,3 +396,43 @@ fn color_stabilizer_samples_the_reference_at_the_points_reference_positions() { assert!((o(x) - (a + b * v(x))).abs() < 1e-3, "x {x}"); } } + +/// Writes `text` to `dir/name` with modification time `secs` (so an edit within the file system's +/// time resolution still moves the stamp), returning the path. +fn write_dated(dir: &std::path::Path, name: &str, text: &str, secs: u64) -> String { + let path = dir.join(name); + std::fs::write(&path, text).unwrap(); + let f = std::fs::File::options().write(true).open(&path).unwrap(); + f.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(secs)).unwrap(); + path.to_string_lossy().into_owned() +} + +/// #3538: OCIO File Transform read a file once per process; an edited, created, repaired or +/// deleted file was never followed. +#[test] +fn an_ocio_transform_file_is_read_again_once_it_changes() { + const BLACK: &str = "LUT_1D_SIZE 2\n0 0 0\n0 0 0\n"; + const WHITE: &str = "LUT_1D_SIZE 2\n1 1 1\n1 1 1\n"; + let dir = std::env::temp_dir().join(format!("ec-fx-3538-file-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let at = |src: &str| load_file(src).map(|x| x.apply([0.5; 3], 1, false, "")); + let edited = write_dated(dir.as_path(), "edited.cube", BLACK, 1_000); + assert_eq!(at(&edited), Some([0.0; 3])); + write_dated(dir.as_path(), "edited.cube", WHITE, 2_000); + assert_eq!(at(&edited), Some([1.0; 3]), "an edited file, same size"); + let created = dir.as_path().join("created.cube").to_string_lossy().into_owned(); + assert_eq!(at(&created), None); + write_dated(dir.as_path(), "created.cube", WHITE, 1_000); + assert_eq!(at(&created), Some([1.0; 3]), "a file created after a failed load"); + let repaired = write_dated(dir.as_path(), "repaired.cube", "not a lut", 1_000); + assert_eq!(at(&repaired), None); + write_dated(dir.as_path(), "repaired.cube", WHITE, 2_000); + assert_eq!(at(&repaired), Some([1.0; 3]), "a repaired file"); + std::fs::remove_file(&repaired).unwrap(); + assert_eq!(at(&repaired), None, "a deleted file"); + // Inline text is its own content: the same text is the same transform. + let inline = load_file(BLACK).unwrap(); + assert!(Arc::ptr_eq(&inline, &load_file(BLACK).unwrap())); + std::fs::remove_dir_all(&dir).unwrap(); +} diff --git a/crates/effects/src/ocio_config/tests.rs b/crates/effects/src/ocio_config/tests.rs index b616e6263..c36e467ea 100644 --- a/crates/effects/src/ocio_config/tests.rs +++ b/crates/effects/src/ocio_config/tests.rs @@ -410,3 +410,45 @@ fn builtin_styles() { fn rgb_white_xyz() -> [f64; 3] { [D65[0] / D65[1], 1.0, (1.0 - D65[0] - D65[1]) / D65[1]] } + +/// #3538: a custom `.ocio` config, and the LUTs it reads, were read once per process. +#[test] +fn an_ocio_config_and_its_luts_are_read_again_once_they_change() { + const BLACK: &str = "LUT_1D_SIZE 2\n0 0 0\n0 0 0\n"; + const WHITE: &str = "LUT_1D_SIZE 2\n1 1 1\n1 1 1\n"; + let dir = std::env::temp_dir().join(format!("ec-fx-3538-config-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::create_dir_all(dir.as_path().join("luts")).unwrap(); + let write = |name: &str, text: &str, secs: u64| { + let path = dir.as_path().join(name); + std::fs::write(&path, text).unwrap(); + let f = std::fs::File::options().write(true).open(&path).unwrap(); + f.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(secs)).unwrap(); + path.to_string_lossy().into_owned() + }; + let config = |space: &str| { + format!( + "ocio_profile_version: 2\nsearch_path: luts\nroles:\n scene_linear: lin\ncolorspaces:\n - !\n name: lin\n - !\n name: {space}\n to_reference: ! {{src: grade.cube, interpolation: linear}}\n - !\n name: later\n to_reference: ! {{src: later.cube, interpolation: linear}}\n" + ) + }; + let src = dir.as_path().join("config.ocio").to_string_lossy().into_owned(); + assert!(load_config(&src).is_err(), "no config yet"); + write("config.ocio", &config("graded"), 1_000); + write("luts/grade.cube", BLACK, 1_000); + let to_lin = |space: &str| { + let c = load_config(&src).unwrap(); + c.convert(c.space(space).unwrap(), c.space("lin").unwrap(), [0.5; 3]) + }; + assert_eq!(to_lin("graded"), [0.0; 3], "a config created after a failed load"); + write("luts/grade.cube", WHITE, 2_000); + assert_eq!(to_lin("graded"), [1.0; 3], "a LUT the config reads, edited"); + assert_eq!(to_lin("later"), [0.5; 3], "a LUT not there yet passes through"); + write("luts/later.cube", BLACK, 1_000); + assert_eq!(to_lin("later"), [0.0; 3], "a LUT the config reads, created"); + write("config.ocio", &config("regraded"), 2_000); + assert!(load_config(&src).unwrap().space("regraded").is_some(), "an edited config"); + std::fs::remove_file(&src).unwrap(); + assert!(load_config(&src).is_err(), "a deleted config"); + std::fs::remove_dir_all(&dir).unwrap(); +} From ef137cb2ce4e28824f6f08c5821573f96d65a766 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 14:43:54 +0530 Subject: [PATCH 2/5] #3538: OCIO transform files and configs follow their files through #3493's LUT cache Move the LUT cache that follows files by size and modification time into effects::file_cache and use it for ocio::lut::load_file and ocio_config::load_config. A config's entry depends on the .ocio file, the LUTs its FileTransforms read and the search-path places it looked for missing ones. check_lut_files and lut_generation now cover all three caches. --- crates/effects/src/file_cache.rs | 415 ++++++++++++++++++++++++ crates/effects/src/lib.rs | 4 +- crates/effects/src/ocio.rs | 4 +- crates/effects/src/ocio/lut.rs | 50 +-- crates/effects/src/ocio_config.rs | 56 ++-- crates/effects/src/ocio_config/tests.rs | 23 ++ crates/effects/src/utility.rs | 198 +++-------- 7 files changed, 546 insertions(+), 204 deletions(-) create mode 100644 crates/effects/src/file_cache.rs diff --git a/crates/effects/src/file_cache.rs b/crates/effects/src/file_cache.rs new file mode 100644 index 000000000..2bcbefede --- /dev/null +++ b/crates/effects/src/file_cache.rs @@ -0,0 +1,415 @@ +//! Caches of what effects read from files named by a parameter (a LUT, an OCIO transform file, an +//! OCIO config), keyed by the parameter text and checked against the files each result was read +//! from (#3493, #3538). +//! +//! A cached result remembers every file its load read, with the file's size and modification time +//! (its stamp) taken before reading, including the files a nested cached load read (a config's +//! `FileTransform` LUTs) and files looked for but missing. Each lookup stamps those files again +//! (one `metadata` call each) and loads again once one moved, so a file edited, created, deleted or +//! repaired on disk is followed. Inline text reads no file and is its own content. An edit that keeps +//! both the size and the modification time can't be told apart. +//! +//! Every file read is also followed by [`check_lut_files`], and [`lut_generation`] moves when one is +//! found changed: the viewers and frame caches drop the frames made before. + +use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Mutex, MutexGuard, OnceLock, PoisonError}; +use std::time::SystemTime; + +/// A file's size and modification time when it was read (`None`: missing, or a file that could +/// not be described). A file whose stamp moved is read again. +pub(crate) type FileStamp = Option<(u64, Option)>; + +/// The files a load read and their stamps. +type Files = Vec<(String, FileStamp)>; + +/// At most this many files are followed by [`check_lut_files`]; the least recently read go first. +const MAX_FOLLOWED_FILES: usize = 1024; + +/// One cached result and what it was read from. +struct Entry { + files: Files, + value: V, + used: u64, + /// The result's size estimate and the parameter text, counted against the cache's byte bound. + bytes: usize, +} + +/// Results by parameter text, at most `max_entries` and `max_bytes` of them, the least recently +/// used going first. +pub(crate) struct SourceCache { + map: HashMap>, + clock: u64, + bytes: usize, + max_entries: usize, + max_bytes: usize, +} + +impl SourceCache { + pub(crate) fn new(max_entries: usize, max_bytes: usize) -> Self { + SourceCache { map: HashMap::new(), clock: 0, bytes: 0, max_entries, max_bytes } + } + + fn remove(&mut self, src: &str) { + if let Some(e) = self.map.remove(src) { + self.bytes = self.bytes.saturating_sub(e.bytes); + } + } + + /// Forget the results read from `path`. + fn forget_file(&mut self, path: &str) { + let stale: Vec = self.map.iter().filter(|(_, e)| e.files.iter().any(|(p, _)| p == path)).map(|(k, _)| k.clone()).collect(); + for src in stale { + self.remove(&src); + } + } + + /// Keep within the bounds, always the result for `keep`. + fn evict(&mut self, keep: &str) { + while self.map.len() > self.max_entries || (self.bytes > self.max_bytes && self.map.len() > 1) { + let Some(oldest) = self.map.iter().filter(|(k, _)| k.as_str() != keep).min_by_key(|(_, e)| e.used).map(|(k, _)| k.clone()) else { break }; + self.remove(&oldest); + } + } + + /// The files the result cached for `src` was read from. + pub(crate) fn files(&self, src: &str) -> Option> { + self.map.get(src).map(|e| e.files.iter().map(|(p, _)| p.clone()).collect()) + } + + #[cfg(test)] + pub(crate) fn len(&self) -> usize { + self.map.len() + } + + #[cfg(test)] + pub(crate) fn contains(&self, src: &str) -> bool { + self.map.contains_key(src) + } + + #[cfg(test)] + pub(crate) fn bytes(&self) -> (usize, usize, usize) { + (self.bytes, self.map.values().map(|e| e.bytes).sum(), self.max_bytes) + } +} + +/// Lock a cache; a panic elsewhere while it was held leaves a usable map. +pub(crate) fn lock(cache: &Mutex>) -> MutexGuard<'_, SourceCache> { + cache.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// A cache that registers with [`check_lut_files`] on first use. +pub(crate) struct Registered { + cache: OnceLock>>, + max_entries: usize, + max_bytes: usize, +} + +impl Registered { + pub(crate) const fn new(max_entries: usize, max_bytes: usize) -> Self { + Registered { cache: OnceLock::new(), max_entries, max_bytes } + } + + pub(crate) fn get(&'static self) -> &'static Mutex> { + self.cache.get_or_init(|| { + registry().push(self); + Mutex::new(SourceCache::new(self.max_entries, self.max_bytes)) + }) + } +} + +/// A cache [`check_lut_files`] tells to forget the results read from a changed file. +trait ForgetFile: Sync { + fn forget_file(&self, path: &str); + /// Add the files the cached results were read from to `out`. + fn held_files(&self, out: &mut HashSet); +} + +impl ForgetFile for Registered { + fn forget_file(&self, path: &str) { + if let Some(c) = self.cache.get() { + lock(c).forget_file(path); + } + } + + fn held_files(&self, out: &mut HashSet) { + if let Some(c) = self.cache.get() { + out.extend(lock(c).map.values().flat_map(|e| e.files.iter().map(|(p, _)| p.clone()))); + } + } +} + +fn registry() -> MutexGuard<'static, Vec<&'static dyn ForgetFile>> { + static REGISTRY: OnceLock>> = OnceLock::new(); + REGISTRY.get_or_init(Mutex::default).lock().unwrap_or_else(PoisonError::into_inner) +} + +thread_local! { + /// The files read by the loads running on this thread, innermost last. + static READING: RefCell> = const { RefCell::new(Vec::new()) }; +} + +/// Note that the running load read (or looked for) `path`, stamped `stamp`. +pub(crate) fn note_file(path: &str, stamp: FileStamp) { + READING.with(|r| { + if let Ok(mut r) = r.try_borrow_mut() + && let Some(top) = r.last_mut() + && !top.iter().any(|(p, _)| p == path) + { + top.push((path.to_string(), stamp)); + } + }); +} + +/// Pops the files of a load when it ends, even by a panic. +struct Reading { + popped: bool, +} + +impl Reading { + fn start() -> Reading { + READING.with(|r| { + if let Ok(mut r) = r.try_borrow_mut() { + r.push(Vec::new()); + } + }); + Reading { popped: false } + } + + fn pop() -> Option { + READING.with(|r| r.try_borrow_mut().ok().and_then(|mut r| r.pop())) + } + + fn files(mut self) -> Files { + self.popped = true; + Self::pop().unwrap_or_default() + } +} + +impl Drop for Reading { + fn drop(&mut self) { + if !self.popped { + let _ = Self::pop(); + } + } +} + +/// The stamp of the file at `path` now: one `metadata` call. +pub(crate) fn file_stamp(path: &str) -> FileStamp { + std::fs::metadata(path).ok().map(|md| (md.len(), md.modified().ok())) +} + +/// Read the text file at `path` for the running load, stamped before reading (a file written +/// after it was stamped has a newer stamp, and is read again next time). +pub(crate) fn read(path: &str) -> std::io::Result { + note_file(path, file_stamp(path)); + std::fs::read_to_string(path) +} + +/// Note that the running load looked for `path` and did not find it: it loads again once the file +/// appears. +pub(crate) fn note_missing(path: &str) { + note_file(path, None); +} + +/// The result cached for `src`, or `load`'s (and its size estimate in bytes), cached. `load` reads +/// files with [`read`] (and notes the ones it looked for with [`note_missing`]); no lock is held +/// while it runs or while the files are stamped. +pub(crate) fn cached(cache: &Mutex>, src: &str, load: impl FnOnce() -> (V, usize)) -> V { + let files = lock(cache).map.get(src).map(|e| e.files.clone()); + let stale = files.is_some(); + if let Some(files) = files + && files.iter().all(|(p, s)| file_stamp(p) == *s) + { + let mut c = lock(cache); + c.clock += 1; + let now = c.clock; + if let Some(e) = c.map.get_mut(src) + && e.files == files + { + e.used = now; + let value = e.value.clone(); + drop(c); + // (an enclosing load depends on these files too) + for (p, s) in &files { + note_file(p, *s); + } + return value; + } + } + let reading = Reading::start(); + let (value, size) = load(); + let files = reading.files(); + follow(&files, stale); + for (p, s) in &files { + note_file(p, *s); + } + let bytes = size.saturating_add(src.len()); + let mut c = lock(cache); + c.clock += 1; + let used = c.clock; + c.remove(src); + c.map.insert(src.to_string(), Entry { files, value: value.clone(), used, bytes }); + c.bytes = c.bytes.saturating_add(bytes); + c.evict(src); + value +} + +/// How often a file a cached load read was found changed on disk ([`lut_generation`]). +static GENERATION: AtomicU64 = AtomicU64::new(0); + +/// The files followed by [`check_lut_files`]: the stamp each was last read with, and when. +#[derive(Default)] +struct Followed { + files: HashMap, + clock: u64, +} + +fn followed() -> MutexGuard<'static, Followed> { + static FOLLOWED: OnceLock> = OnceLock::new(); + FOLLOWED.get_or_init(Mutex::default).lock().unwrap_or_else(PoisonError::into_inner) +} + +impl Followed { + /// Note that `files` were read with these stamps; true when one has a different stamp than + /// its last read's: a file changed on disk. + fn read(&mut self, files: &Files) -> bool { + let mut moved = false; + for (path, stamp) in files { + self.clock += 1; + let now = self.clock; + if self.files.insert(path.clone(), (*stamp, now)).is_some_and(|(old, _)| old != *stamp) { + moved = true; + } + } + moved + } + + /// Keep at most [`MAX_FOLLOWED_FILES`], letting go of the least recently read first but never + /// of a file in `held` (a cached result was read from it) or in `files` (just read). + fn trim(&mut self, held: &HashSet, files: &Files) { + while self.files.len() > MAX_FOLLOWED_FILES { + let Some(oldest) = self + .files + .iter() + .filter(|(p, _)| !held.contains(p.as_str()) && !files.iter().any(|(f, _)| f == *p)) + .min_by_key(|(_, (_, used))| *used) + .map(|(p, _)| p.clone()) + else { + break; + }; + self.files.remove(&oldest); + } + } +} + +/// The files the registered caches' results were read from. +fn held_files() -> HashSet { + let caches: Vec<&'static dyn ForgetFile> = registry().clone(); + let mut out = HashSet::new(); + for c in caches { + c.held_files(&mut out); + } + out +} + +/// Note that `files` were read with these stamps. A file read with a different stamp than last +/// time, or a load replacing a result whose files moved (`stale`), means a file changed on disk, +/// and frames made before may show the old contents. +fn follow(files: &Files, stale: bool) { + let (moved, over) = { + let mut f = followed(); + (f.read(files), f.files.len() > MAX_FOLLOWED_FILES) + }; + if over { + // (gathered without the followed files' lock: no two of these locks are ever nested) + let held = held_files(); + followed().trim(&held, files); + } + if moved || stale { + GENERATION.fetch_add(1, Ordering::AcqRel); + } +} + +/// Changes each time a file a cached load read is found changed on disk (by a load itself or +/// [`check_lut_files`]): frames made before then may show the old contents. +pub fn lut_generation() -> u64 { + GENERATION.load(Ordering::Acquire) +} + +/// Look at every file read (one `metadata` call each) and forget the results read from the ones +/// that changed on disk since, so the next load reads them again. Returns [`lut_generation`]. +pub fn check_lut_files() -> u64 { + let files: Vec<(String, FileStamp)> = followed().files.iter().map(|(p, (s, _))| (p.clone(), *s)).collect(); + // (stamped without the lock: a file on a slow share must not hold up renders) + let changed: Vec<(String, FileStamp, FileStamp)> = + files.into_iter().filter_map(|(path, old)| Some(file_stamp(&path)).filter(|now| *now != old).map(|now| (path, old, now))).collect(); + let caches: Vec<&'static dyn ForgetFile> = registry().clone(); + for (path, old, now) in changed { + { + let mut f = followed(); + // (a render that read the file meanwhile has noted its new stamp already) + match f.files.get_mut(&path) { + Some((s, _)) if *s == old => *s = now, + _ => continue, + } + } + for c in &caches { + c.forget_file(&path); + } + GENERATION.fetch_add(1, Ordering::AcqRel); + } + lut_generation() +} + +#[cfg(test)] +pub(crate) fn followed_stamp(path: &str) -> Option { + followed().files.get(path).map(|(s, _)| *s) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Past [`MAX_FOLLOWED_FILES`], the file just read is still followed (a list of its own: the + /// shared one's files belong to the other tests too). + #[test] + fn the_file_just_read_stays_followed_past_the_limit() { + let mut f = Followed::default(); + let held: HashSet = ["/followed/0.cube".to_string()].into(); + for i in 0..MAX_FOLLOWED_FILES + 8 { + let files = vec![(format!("/followed/{i}.cube"), Some((1, None)))]; + assert!(!f.read(&files)); + f.trim(&held, &files); + assert!(f.files.contains_key(&files[0].0), "{i}: the file just read"); + assert!(f.files.len() <= MAX_FOLLOWED_FILES); + } + assert!(f.files.contains_key("/followed/0.cube"), "a file a cached result was read from"); + assert!(!f.files.contains_key("/followed/1.cube"), "the least recently read other"); + assert!(f.read(&vec![("/followed/1030.cube".into(), Some((2, None)))]), "a moved stamp"); + } + + /// A nested cached load's files count for the enclosing load. + #[test] + fn an_enclosing_load_depends_on_a_nested_loads_files() { + static INNER: Registered = Registered::new(8, 1 << 20); + static OUTER: Registered = Registered::new(8, 1 << 20); + let v = cached(OUTER.get(), "outer", || { + note_missing("/nested/outer-own.cube"); + let inner = cached(INNER.get(), "inner", || { + note_missing("/nested/inner.cube"); + (7, 0) + }); + (inner, 0) + }); + assert_eq!(v, 7); + let files = lock(OUTER.get()).files("outer").unwrap(); + assert_eq!(files, vec!["/nested/outer-own.cube".to_string(), "/nested/inner.cube".to_string()]); + // A hit on the inner result still reports its files to a new enclosing load. + let w = cached(OUTER.get(), "outer 2", || (cached(INNER.get(), "inner", || (0, 0)), 0)); + assert_eq!(w, 7); + assert_eq!(lock(OUTER.get()).files("outer 2").unwrap(), vec!["/nested/inner.cube".to_string()]); + } +} diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 9105f4587..26b0008ef 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -39,6 +39,7 @@ mod keying2; pub mod keylight; mod matte; +mod file_cache; pub mod migrate; mod misc; pub mod mocha_shape; @@ -123,13 +124,14 @@ pub use channel3d::{ crypto_layer_name, crypto_layer_named, extractor_aux_channel, extractor_own_channel, id_color, is_extractor_copy, matte_only_px, reads_aux, selection_hashes, }; +pub use file_cache::{check_lut_files, lut_generation}; pub use ocio::{ColorOp, Straight, Tf, color_program}; pub use sim::{Acc, Blob, Bubble, Post, Shape, Sprite, SpritePlan, Tint, bubble_list, bubble_sample_bounds, drizzle_drops, mercury_blobs, sprite_plan}; pub use sim2::{ CausticsSetup, FoamDisc, FoamEnv, FoamPlan, Piece, PiecePlan, PieceTex, PixelPlan, WavePlan, Waves, caustics_setup, foam_full_plan, piece_plan, pixel_plan, wave_world_plan, }; -pub use utility::{Lut, check_lut_files, load_lut, lut_file, lut_generation, lut_params}; +pub use utility::{Lut, load_lut, lut_file, lut_files, lut_params}; pub use vr::{equi_dir as vr_equi_dir, rotation as vr_rotation}; /// Effect categories in Effects & Presets order. diff --git a/crates/effects/src/ocio.rs b/crates/effects/src/ocio.rs index 2daa9857d..febbc163c 100644 --- a/crates/effects/src/ocio.rs +++ b/crates/effects/src/ocio.rs @@ -24,8 +24,7 @@ //! * Configuration ▸ Custom reads a `.ocio` file instead ([`crate::ocio_config`]); what it can't //! apply is reported by [`warning`]. -use std::collections::HashMap; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::Arc; use effectcraft_color::ColorSpace; use effectcraft_color::space::{invert, mul, mul_vec, rgb_to_xyz}; @@ -51,6 +50,7 @@ pub use effects::parse_looks; pub use effects::{color_stabilizer_maps, warning}; #[cfg(test)] use effects::{tonemap, tonemap_inv}; +pub(crate) use lut::cached_files; pub use lut::{FileXform, load_file}; #[cfg(test)] pub use lut::{parse_file, parse_spi1d, parse_spi3d, parse_spimtx}; diff --git a/crates/effects/src/ocio/lut.rs b/crates/effects/src/ocio/lut.rs index de931a411..daa584278 100644 --- a/crates/effects/src/ocio/lut.rs +++ b/crates/effects/src/ocio/lut.rs @@ -1,5 +1,6 @@ use super::cdl::{Cdl, parse_cdl_xml}; -use super::{Arc, HashMap, Lut, Mutex, OnceLock, parse_cube}; +use super::{Arc, Lut, parse_cube}; +use crate::file_cache::{self, Registered}; // ---------------------------------------------------------------- LUT files @@ -264,33 +265,38 @@ pub fn parse_file(text: &str, ext: &str) -> Option { parse_3dl(text).map(|lut| FileXform::Lut { lut, shaper: None }) } -type FileCache = Mutex>>>; +/// [`load_file`]'s cache, by parameter text (bounded like [`crate::load_lut`]'s). +static FILES: Registered>> = Registered::new(64, 512 << 20); -/// Load (and cache) a transform from a file path or inline content. +/// The files the transform cached for `src` was read from. +pub(crate) fn cached_files(src: &str) -> Option> { + file_cache::lock(FILES.get()).files(src) +} + +/// Load (and cache) a transform from a file path or inline content. A file is stamped (one +/// `metadata` call) on every load and read again once its size or modification time moved (see +/// [`crate::file_cache`]); inline content is its own content. pub fn load_file(src: &str) -> Option> { let src_t = src.trim(); if src_t.is_empty() { return None; } - static CACHE: OnceLock = OnceLock::new(); - let cache = CACHE.get_or_init(|| Mutex::new(HashMap::new())); - if let Ok(m) = cache.lock() - && let Some(v) = m.get(src) - { - return v.clone(); - } - let inline = src.contains('\n'); - let parsed = if inline { - parse_file(src, "") - } else { - let ext = std::path::Path::new(src_t).extension().and_then(std::ffi::OsStr::to_str).unwrap_or(""); - std::fs::read_to_string(src_t).ok().and_then(|t| parse_file(&t, ext)) - } - .map(Arc::new); - if let Ok(mut m) = cache.lock() { - m.insert(src.to_string(), parsed.clone()); - } - parsed + file_cache::cached(FILES.get(), src, || { + let parsed = if src.contains('\n') { + parse_file(src, "") + } else { + let ext = std::path::Path::new(src_t).extension().and_then(std::ffi::OsStr::to_str).unwrap_or(""); + file_cache::read(src_t).ok().and_then(|t| parse_file(&t, ext)) + } + .map(Arc::new); + let bytes = match parsed.as_deref() { + Some(FileXform::Lut { lut, shaper }) => crate::utility::lut_bytes(lut) + .saturating_add(shaper.as_ref().map_or(0, |s| s.iter().map(Vec::len).sum::().saturating_mul(std::mem::size_of::<(f32, f32)>()))), + Some(FileXform::Cdl(c)) => c.len().saturating_mul(std::mem::size_of::<(String, Cdl)>()), + Some(FileXform::Matrix { .. }) | None => 0, + }; + (parsed, bytes) + }) } impl FileXform { diff --git a/crates/effects/src/ocio_config.rs b/crates/effects/src/ocio_config.rs index e31281ede..17e61d4b1 100644 --- a/crates/effects/src/ocio_config.rs +++ b/crates/effects/src/ocio_config.rs @@ -42,9 +42,10 @@ //! resolving named transforms [`MAX_NAMED_DEPTH`] levels down or past [`MAX_OPS`] steps //! (a config that refers to itself in a loop), so a hostile config can't exhaust the stack. -use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::Arc; + +use crate::file_cache::{self, Registered}; use super::ocio::{ACES_CG, ACES2065_1, Cdl, D65, FileXform, P3, REC709, REC2020, Space, Tf, load_file, matrix, sp}; @@ -882,13 +883,21 @@ fn resolve(src: &str, dir: &Path, search: &[String]) -> Option { if src.trim().is_empty() { return None; } + // (a candidate not there is noted: the config is read again once it appears) + let found = |c: PathBuf| { + let there = c.exists(); + if !there { + file_cache::note_missing(&c.to_string_lossy()); + } + there.then_some(c) + }; let p = Path::new(src); if p.is_absolute() { - return p.exists().then(|| p.to_path_buf()); + return found(p.to_path_buf()); } let mut dirs: Vec = search.iter().map(|s| dir.join(s)).collect(); dirs.push(dir.to_path_buf()); - dirs.into_iter().map(|d| d.join(src)).find(|c| c.exists()) + dirs.into_iter().map(|d| d.join(src)).find_map(found) } // ---------------------------------------------------------------- the config @@ -1338,33 +1347,34 @@ impl<'c> Build<'c> { } } -type ConfigCache = Mutex, String>>>; +/// [`load_config`]'s cache, by parameter text. +static CONFIGS: Registered, String>> = Registered::new(16, 64 << 20); + +/// The files the config cached for `src` was read from: the `.ocio` file, the files its +/// `FileTransform`s read and the places it looked for missing ones. +pub(crate) fn cached_files(src: &str) -> Option> { + file_cache::lock(CONFIGS.get()).files(src) +} /// Load (and cache) a config from a `.ocio` path or the config text itself; why not, when it -/// can't be read. +/// can't be read. The config file and the files its `FileTransform`s read are stamped (one +/// `metadata` call each) on every load, and the config is read again once one of them moved or a +/// missing one appeared (see [`crate::file_cache`]). pub fn load_config(src: &str) -> Result, String> { let t = src.trim(); if t.is_empty() { return Err("no config file chosen".into()); } - static CACHE: OnceLock = OnceLock::new(); - let cache = CACHE.get_or_init(|| Mutex::new(HashMap::new())); - if let Ok(m) = cache.lock() - && let Some(v) = m.get(src) - { - return v.clone(); - } - let parsed = if src.contains('\n') { - Config::parse(src, Path::new(".")) - } else { + file_cache::cached(CONFIGS.get(), src, || { + if src.contains('\n') { + return (Config::parse(src, Path::new(".")).map(Arc::new), 0); + } let p = Path::new(t); - std::fs::read_to_string(p).map_err(|e| format!("{t}: {e}")).and_then(|text| Config::parse(&text, p.parent().unwrap_or(Path::new(".")))) - } - .map(Arc::new); - if let Ok(mut m) = cache.lock() { - m.insert(src.to_string(), parsed.clone()); - } - parsed + match file_cache::read(t) { + Ok(text) => (Config::parse(&text, p.parent().unwrap_or(Path::new("."))).map(Arc::new), text.len()), + Err(e) => (Err(format!("{t}: {e}")), 0), + } + }) } #[cfg(test)] diff --git a/crates/effects/src/ocio_config/tests.rs b/crates/effects/src/ocio_config/tests.rs index c36e467ea..ec34b6723 100644 --- a/crates/effects/src/ocio_config/tests.rs +++ b/crates/effects/src/ocio_config/tests.rs @@ -452,3 +452,26 @@ fn an_ocio_config_and_its_luts_are_read_again_once_they_change() { assert!(load_config(&src).is_err(), "a deleted config"); std::fs::remove_dir_all(&dir).unwrap(); } + +/// A LUT a cached config read that changes is noticed without a render loading the config (the +/// interface's watcher), and the config's files name it (frame and layer cache keys hold them). +#[test] +fn check_lut_files_notices_a_configs_lut() { + let dir = std::env::temp_dir().join(format!("ec-fx-3538-check-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let lut = dir.join("watched.cube"); + std::fs::write(&lut, "LUT_1D_SIZE 2\n0 0 0\n1 1 1\n").unwrap(); + let config = dir.join("config.ocio"); + std::fs::write(&config, "ocio_profile_version: 2\ncolorspaces:\n - !\n name: lin\n - !\n name: graded\n to_reference: ! {src: watched.cube}\n").unwrap(); + let src = config.to_string_lossy().into_owned(); + assert!(load_config(&src).is_ok()); + let files = crate::lut_files(&src); + assert!(files.contains(&lut.to_string_lossy().into_owned()), "{files:?}"); + let before = crate::check_lut_files(); + assert!(cached_files(&src).is_some(), "an unchanged config stays"); + std::fs::write(&lut, "LUT_1D_SIZE 3\n0 0 0\n0.5 0.5 0.5\n1 1 1\n").unwrap(); + assert!(crate::check_lut_files() > before); + assert!(cached_files(&src).is_none(), "a config whose LUT changed is read again"); + std::fs::remove_dir_all(&dir).unwrap(); +} diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index 9d5fec491..d735dc286 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -1,10 +1,9 @@ //! Utility effects: bounds growth, HDR inspection/compression, Cineon log conversion and colour //! lookup tables (.cube). -use std::collections::HashMap; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, Mutex, OnceLock, PoisonError}; +use std::sync::Arc; +use crate::file_cache::{self, Registered}; use effectcraft_color::luminance; use effectcraft_keyframe::Value; use effectcraft_project::ParamUi; @@ -331,84 +330,22 @@ impl Lut { } } -/// A LUT file's size and modification time when it was read (`None`: inline text, or a file -/// that could not be described). A file whose stamp moved is read again (#3493). -type FileStamp = Option<(u64, Option)>; - /// At most this many parsed LUTs (files and inline texts) are kept, and at most /// [`MAX_CACHED_BYTES`] of them; the least recently used go first. const MAX_CACHED_LUTS: usize = 64; const MAX_CACHED_BYTES: usize = 512 << 20; -/// At most this many LUT files are followed by [`check_lut_files`] beyond the ones whose LUT is -/// kept: the frames made with a LUT that left the cache are still dropped when its file changes. -const MAX_FOLLOWED_FILES: usize = 1024; +/// [`load_lut`]'s cache, by parameter text. +static LUTS: Registered>> = Registered::new(MAX_CACHED_LUTS, MAX_CACHED_BYTES); -/// A parsed LUT (or the failure to parse one) and what it was read from. -struct CachedLut { - stamp: FileStamp, - lut: Option>, - used: u64, - /// The parsed tables and the parameter text, counted against [`MAX_CACHED_BYTES`]. - bytes: usize, -} - -/// [`load_lut`]'s cache, by parameter text, and the stamp each LUT file was last read with. -#[derive(Default)] -struct LutCache { - map: HashMap, - files: HashMap, - clock: u64, - bytes: usize, -} - -impl LutCache { - /// Forget the LUT kept for `src`. - fn remove(&mut self, src: &str) { - if let Some(e) = self.map.remove(src) { - self.bytes = self.bytes.saturating_sub(e.bytes); - } - } - - /// Note that the file at `path` was read with `stamp`: a different stamp than the last read's - /// means it changed on disk, and frames made before may show the old LUT. - fn read_file(&mut self, path: &str, stamp: FileStamp) { - if self.files.insert(path.to_string(), stamp).is_some_and(|old| old != stamp) { - GENERATION.fetch_add(1, Ordering::AcqRel); - } - if self.files.len() > MAX_FOLLOWED_FILES { - let map = &self.map; - self.files.retain(|p, _| p.as_str() == path || map.keys().any(|src| lut_file(src) == Some(p.as_str()))); - } - } - - /// Keep at most [`MAX_CACHED_LUTS`] LUTs and [`MAX_CACHED_BYTES`], always the one for `keep`. - fn evict(&mut self, keep: &str) { - while self.map.len() > MAX_CACHED_LUTS || (self.bytes > MAX_CACHED_BYTES && self.map.len() > 1) { - let Some(oldest) = self.map.iter().filter(|(k, _)| k.as_str() != keep).min_by_key(|(_, e)| e.used).map(|(k, _)| k.clone()) else { break }; - self.remove(&oldest); - } - } -} - -/// How often a LUT file was found changed on disk ([`lut_generation`]). -static GENERATION: AtomicU64 = AtomicU64::new(0); - -fn lut_cache() -> std::sync::MutexGuard<'static, LutCache> { - static CACHE: OnceLock> = OnceLock::new(); - CACHE.get_or_init(Mutex::default).lock().unwrap_or_else(PoisonError::into_inner) -} - -/// The stamp of the file at `path` now: one `metadata` call. -fn file_stamp(path: &str) -> FileStamp { - std::fs::metadata(path).ok().map(|md| (md.len(), md.modified().ok())) -} - -/// The parameters (paths in the effect's group) of `effect` that [`load_lut`] reads. +/// The parameters (paths in the effect's group) of `effect` that name a file an effect reads +/// through a cache that follows it ([`load_lut`], the OCIO file transforms and configs). pub fn lut_params(effect: &str) -> &'static [&'static str] { match effect { "ec.utility.applylut" => &["lut"], "ec.color.lumetri" => &["basicCorrection/inputLutFile", "creative/lookFile"], + "ec.color.ociofile" => &["file"], + "ec.color.ociocolorspace" | "ec.color.ociodisplay" => &["configFile"], _ => &[], } } @@ -421,75 +358,37 @@ pub fn lut_file(src: &str) -> Option<&str> { (!path.is_empty() && !path.contains('\n')).then_some(path) } -/// Changes each time a LUT file [`load_lut`] read is found changed on disk (by [`load_lut`] -/// itself or [`check_lut_files`]): frames made before then may show the old LUT. -pub fn lut_generation() -> u64 { - GENERATION.load(Ordering::Acquire) -} - -/// Look at every LUT file read (one `metadata` call each) and forget the LUTs of the ones that -/// changed on disk since, so the next [`load_lut`] reads them again. Returns [`lut_generation`]. -pub fn check_lut_files() -> u64 { - let files: Vec<(String, FileStamp)> = lut_cache().files.iter().map(|(p, s)| (p.clone(), *s)).collect(); - // (stamped without the lock: a file on a slow share must not hold up renders) - let changed: Vec<(String, FileStamp, FileStamp)> = - files.into_iter().filter_map(|(path, old)| Some(file_stamp(&path)).filter(|now| *now != old).map(|now| (path, old, now))).collect(); - if !changed.is_empty() { - let mut c = lut_cache(); - for (path, old, now) in changed { - // (a render that read the file meanwhile has noted its new stamp already) - if c.files.get(&path) == Some(&old) { - let stale: Vec = c.map.keys().filter(|src| lut_file(src) == Some(path.as_str())).cloned().collect(); - for src in stale { - c.remove(&src); - } - c.read_file(&path, now); - } - } - } - lut_generation() +/// The files a [`lut_params`] parameter set to `src` reads: the ones its cached result was read +/// from (an OCIO config's also the LUTs it names, and the places it looked for missing ones), else +/// the file it names. +pub fn lut_files(src: &str) -> Vec { + crate::ocio_config::cached_files(src) + .or_else(|| crate::ocio::cached_files(src)) + .or_else(|| file_cache::lock(LUTS.get()).files(src)) + .unwrap_or_else(|| lut_file(src).map(str::to_string).into_iter().collect()) } /// Load (and cache) a LUT from inline `.cube` text or a file path. A file is stamped (one -/// `metadata` call) on every load and read again once its size or modification time moved, so a -/// file edited, created, deleted or repaired on disk is followed; inline text is its own content. -/// An edit that keeps both the size and the modification time can't be told apart. +/// `metadata` call) on every load and read again once its size or modification time moved (see +/// [`crate::file_cache`]); inline text is its own content. pub fn load_lut(src: &str) -> Option> { if src.trim().is_empty() { return None; } - let file = lut_file(src); - let stamp = file.and_then(file_stamp); - { - let mut c = lut_cache(); - c.clock += 1; - let now = c.clock; - if let Some(e) = c.map.get_mut(src) - && e.stamp == stamp - { - e.used = now; - return e.lut.clone(); + file_cache::cached(LUTS.get(), src, || { + let lut = match lut_file(src) { + Some(path) => file_cache::read(path).ok().and_then(|t| parse_cube(&t)), + None => parse_cube(src), } - } - // (read and parsed without the lock; a file written after it was stamped has a newer stamp, - // and is read again next time) - let lut = match file { - Some(path) => std::fs::read_to_string(path).ok().and_then(|t| parse_cube(&t)), - None => parse_cube(src), - } - .map(Arc::new); - let tables = lut.as_ref().map_or(0, |l| l.d1.len().saturating_add(l.d3.len()).saturating_mul(std::mem::size_of::<[f32; 3]>())); - let bytes = tables.saturating_add(src.len()); - let mut c = lut_cache(); - if let Some(path) = file { - c.read_file(path, stamp); - } - c.remove(src); - let used = c.clock; - c.map.insert(src.to_string(), CachedLut { stamp, lut: lut.clone(), used, bytes }); - c.bytes = c.bytes.saturating_add(bytes); - c.evict(src); - lut + .map(Arc::new); + let bytes = lut.as_ref().map_or(0, |l| lut_bytes(l)); + (lut, bytes) + }) +} + +/// The size of a LUT's tables. +pub(crate) fn lut_bytes(l: &Lut) -> usize { + l.d1.len().saturating_add(l.d3.len()).saturating_mul(std::mem::size_of::<[f32; 3]>()) } fn apply_lut(ctx: &EffectCtx, mut b: Buf) -> Buf { @@ -685,11 +584,11 @@ mod tests { std::fs::write(&path, identity_cube(2)).unwrap(); let src = path.to_string_lossy().into_owned(); assert!(load_lut(&src).is_some()); - let before = check_lut_files(); - assert!(lut_cache().map.contains_key(&src), "an unchanged file stays"); + let before = crate::check_lut_files(); + assert!(file_cache::lock(LUTS.get()).contains(&src), "an unchanged file stays"); std::fs::write(&path, identity_cube(3)).unwrap(); - assert!(check_lut_files() > before); - assert!(!lut_cache().map.contains_key(&src), "a changed file is read again"); + assert!(crate::check_lut_files() > before); + assert!(!file_cache::lock(LUTS.get()).contains(&src), "a changed file is read again"); assert_eq!(load_lut(&src).map(|l| l.n3), Some(3)); std::fs::remove_dir_all(&dir).unwrap(); } @@ -703,12 +602,12 @@ mod tests { for i in 0..MAX_CACHED_LUTS + 8 { let text = format!("TITLE \"bounded {i}\"\n{}", identity_cube(2)); assert!(load_lut(&text).is_some()); - assert!(lut_cache().map.len() <= MAX_CACHED_LUTS); + assert!(file_cache::lock(LUTS.get()).len() <= MAX_CACHED_LUTS); } assert!(!Arc::ptr_eq(&first, &load_lut(&oldest).unwrap()), "the oldest was let go"); - let c = lut_cache(); - assert_eq!(c.bytes, c.map.values().map(|e| e.bytes).sum::(), "the bytes kept add up"); - assert!(c.bytes <= MAX_CACHED_BYTES); + let (bytes, sum, max) = file_cache::lock(LUTS.get()).bytes(); + assert_eq!(bytes, sum, "the bytes kept add up"); + assert!(bytes <= max); } /// A LUT file whose LUT left the cache is still followed: its frames are dropped when it @@ -725,26 +624,13 @@ mod tests { for i in 0..MAX_CACHED_LUTS + 8 { assert!(load_lut(&format!("TITLE \"evict {i}\"\n{}", identity_cube(2))).is_some()); } - assert!(!lut_cache().map.contains_key(&src), "its LUT left the cache"); + assert!(!file_cache::lock(LUTS.get()).contains(&src), "its LUT left the cache"); std::fs::write(&path, identity_cube(3)).unwrap(); - check_lut_files(); - assert_eq!(lut_cache().files.get(&src), Some(&file_stamp(&src)), "the change was noticed"); + crate::check_lut_files(); + assert_eq!(file_cache::followed_stamp(&src), Some(file_cache::file_stamp(&src)), "the change was noticed"); std::fs::remove_dir_all(&dir).unwrap(); } - /// Past [`MAX_FOLLOWED_FILES`], the file just read is still followed (a cache of its own: - /// the shared one's followed files belong to the other tests too). - #[test] - fn the_file_just_read_stays_followed_past_the_limit() { - let mut c = LutCache::default(); - for i in 0..MAX_FOLLOWED_FILES + 8 { - let path = format!("/followed/{i}.cube"); - c.read_file(&path, Some((1, None))); - assert!(c.files.contains_key(&path), "{i}: the file just read"); - } - assert!(c.files.len() <= MAX_FOLLOWED_FILES, "the others no longer cached are let go"); - } - #[test] fn one_d_lut_with_domain() { let lut = parse_cube("LUT_1D_SIZE 2\nDOMAIN_MIN 0 0 0\nDOMAIN_MAX 2 2 2\n1 1 1\n0 0 0\n").unwrap(); From a700c5c8dbc3de214a615b1dea5d75fde5b545e4 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 14:43:54 +0530 Subject: [PATCH 3/5] #3538: layer keys and render reads include OCIO files and a config's LUTs --- crates/engine/src/tests/lut_files.rs | 40 +++++++++++++++++++++++++ crates/render/src/cache/dependencies.rs | 7 ++--- crates/render/src/reads.rs | 23 ++++++++++---- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/tests/lut_files.rs b/crates/engine/src/tests/lut_files.rs index 57d163b36..0153a9997 100644 --- a/crates/engine/src/tests/lut_files.rs +++ b/crates/engine/src/tests/lut_files.rs @@ -186,3 +186,43 @@ fn an_overridden_lut_file_renders_again() { write(&overridden, WHITE, 2_000); assert_all(centre(&s), 1.0); } + +/// #3538: OCIO File Transform and a custom OCIO config (and the LUT it names) follow their files +/// through warm layer caches, and a render notes the config's LUT for the disk cache. +#[test] +fn ocio_files_render_again_from_warm_caches() { + let dir = tempfile::tempdir().unwrap(); + let (lut, config, grade) = (dir.path().join("file.cube"), dir.path().join("config.ocio"), dir.path().join("luts/grade.cube")); + std::fs::create_dir_all(dir.path().join("luts")).unwrap(); + write(&lut, BLACK, 1_000); + write(&grade, BLACK, 1_000); + let text = "ocio_profile_version: 2\nsearch_path: luts\nroles:\n scene_linear: lin\ncolorspaces:\n - !\n name: lin\n - !\n name: graded\n to_reference: ! {src: grade.cube, interpolation: linear}\n"; + write(&config, text, 1_000); + let mut s = session(None); + let file = shot(&mut s, "OCIO File Transform", &[("file", json!(lut.to_string_lossy()))]); + let custom = shot( + &mut s, + "OCIO Color Space Transform", + &[("config", json!(1)), ("configFile", json!(config.to_string_lossy())), ("sourceName", json!("graded")), ("destinationName", json!("lin"))], + ); + assert_all(colours(&s, file), 0.0); + assert_all(colours(&s, custom), 0.0); + // (the first key of a config's layer can't know the LUTs it names before it is read) + assert_all(colours(&s, custom), 0.0); + let misses = s.layer_cache.stats().misses; + assert_all(colours(&s, file), 0.0); + assert_all(colours(&s, custom), 0.0); + assert_eq!(s.layer_cache.stats().misses, misses, "unchanged files keep the cached layers"); + let log = ReadLog::new(); + let mut r = s.renderer(RenderOpts::default()); + r.reads = Some(&log); + let _ = r.comp_frame(custom.1, Tick::ZERO); + let files = log.into_files(); + assert!(files.get(&grade.to_string_lossy()).is_some(), "the render notes the config's LUT: {files:?}"); + let generation = effectcraft_effects::lut_generation(); + write(&lut, WHITE, 2_000); + assert_all(colours(&s, file), 1.0); + write(&grade, WHITE, 2_000); + assert_all(colours(&s, custom), 1.0); + assert!(effectcraft_effects::lut_generation() > generation, "the viewer's frames made before are dropped"); +} diff --git a/crates/render/src/cache/dependencies.rs b/crates/render/src/cache/dependencies.rs index 5e8a8845b..6f14ddcde 100644 --- a/crates/render/src/cache/dependencies.rs +++ b/crates/render/src/cache/dependencies.rs @@ -175,11 +175,8 @@ impl Dependencies<'_> { // Properties overrides, which the render applies: a LUT file one of // them names is read too (#3493). for o in crate::fx_host::essential_overrides(ctx, layer) { - if let effectcraft_keyframe::Value::Str(src) = &o.value - && let Some(path) = effectcraft_effects::lut_file(src) - { - crate::reads::ReadFile::new(path, crate::reads::FileStamp::native(path).unwrap_or(crate::reads::FileStamp::Missing)) - .hash(self.h); + if let effectcraft_keyframe::Value::Str(src) = &o.value { + crate::reads::source_files(src).hash(self.h); } } let nested = EvalCtx::new(ctx.project, item, comp, time); diff --git a/crates/render/src/reads.rs b/crates/render/src/reads.rs index 9021f1e42..73b7ce830 100644 --- a/crates/render/src/reads.rs +++ b/crates/render/src/reads.rs @@ -65,20 +65,31 @@ impl ReadFile { } } -/// The LUT files effect `g` of `layer` reads at the context time (Apply Color LUT, Lumetri's -/// Input LUT and Look), each with its identity now. A layer's cache key holds them and a render +/// The files effect `g` of `layer` reads at the context time (Apply Color LUT, Lumetri's Input +/// LUT and Look, OCIO File Transform, a custom OCIO config and its LUTs: see +/// [`effectcraft_effects::lut_params`]), each with its identity now. A layer's cache key holds them and a render /// notes them, so what was made with a file that changed since is not used (#3493). pub(crate) fn lut_files(ctx: &crate::EvalCtx, layer: &effectcraft_project::Layer, g: &effectcraft_project::PropGroup) -> Vec { let effectcraft_project::GroupKind::Effect { effect } = &g.kind else { return Vec::new() }; effectcraft_effects::lut_params(effect) .iter() .filter_map(|param| match ctx.value(layer, g.prop(param)?) { - effectcraft_keyframe::Value::Str(src) => { - let path = effectcraft_effects::lut_file(&src)?; - Some(ReadFile::new(path, FileStamp::native(path).unwrap_or(FileStamp::Missing))) - } + effectcraft_keyframe::Value::Str(src) => Some(source_files(&src)), _ => None, }) + .flatten() + .collect() +} + +/// The files a file parameter set to `src` reads (an OCIO config's also the LUTs it names, #3538), +/// each with its identity now. +pub(crate) fn source_files(src: &str) -> Vec { + effectcraft_effects::lut_files(src) + .into_iter() + .map(|path| { + let stamp = FileStamp::native(&path).unwrap_or(FileStamp::Missing); + ReadFile::new(path, stamp) + }) .collect() } From 4d24233505250d0dd062fc30101d4cd6fa75ef38 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 15:07:44 +0530 Subject: [PATCH 4/5] #3538: a custom config not read yet is read for its LUT files --- crates/effects/src/ocio_config/tests.rs | 3 ++- crates/effects/src/utility.rs | 10 ++++++++-- crates/engine/src/tests/lut_files.rs | 15 +++++++-------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/effects/src/ocio_config/tests.rs b/crates/effects/src/ocio_config/tests.rs index ec34b6723..31cac56c2 100644 --- a/crates/effects/src/ocio_config/tests.rs +++ b/crates/effects/src/ocio_config/tests.rs @@ -465,9 +465,10 @@ fn check_lut_files_notices_a_configs_lut() { let config = dir.join("config.ocio"); std::fs::write(&config, "ocio_profile_version: 2\ncolorspaces:\n - !\n name: lin\n - !\n name: graded\n to_reference: ! {src: watched.cube}\n").unwrap(); let src = config.to_string_lossy().into_owned(); - assert!(load_config(&src).is_ok()); + // A config not read yet is read: its files name its LUT from the first render on. let files = crate::lut_files(&src); assert!(files.contains(&lut.to_string_lossy().into_owned()), "{files:?}"); + assert!(load_config(&src).is_ok()); let before = crate::check_lut_files(); assert!(cached_files(&src).is_some(), "an unchanged config stays"); std::fs::write(&lut, "LUT_1D_SIZE 3\n0 0 0\n0.5 0.5 0.5\n1 1 1\n").unwrap(); diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index d735dc286..afb430e1d 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -359,9 +359,15 @@ pub fn lut_file(src: &str) -> Option<&str> { } /// The files a [`lut_params`] parameter set to `src` reads: the ones its cached result was read -/// from (an OCIO config's also the LUTs it names, and the places it looked for missing ones), else -/// the file it names. +/// from (an OCIO config's also the LUTs it names, and the places it looked for missing ones; an +/// `.ocio` config not read yet is read), else the file it names. pub fn lut_files(src: &str) -> Vec { + // A config not read yet is read now: the LUTs it names are its files from the first render. + if crate::ocio_config::cached_files(src).is_none() + && lut_file(src).is_some_and(|p| std::path::Path::new(p).extension().is_some_and(|e| e.eq_ignore_ascii_case("ocio"))) + { + let _ = crate::ocio_config::load_config(src); + } crate::ocio_config::cached_files(src) .or_else(|| crate::ocio::cached_files(src)) .or_else(|| file_cache::lock(LUTS.get()).files(src)) diff --git a/crates/engine/src/tests/lut_files.rs b/crates/engine/src/tests/lut_files.rs index 0153a9997..98c95652a 100644 --- a/crates/engine/src/tests/lut_files.rs +++ b/crates/engine/src/tests/lut_files.rs @@ -205,20 +205,19 @@ fn ocio_files_render_again_from_warm_caches() { "OCIO Color Space Transform", &[("config", json!(1)), ("configFile", json!(config.to_string_lossy())), ("sourceName", json!("graded")), ("destinationName", json!("lin"))], ); - assert_all(colours(&s, file), 0.0); - assert_all(colours(&s, custom), 0.0); - // (the first key of a config's layer can't know the LUTs it names before it is read) - assert_all(colours(&s, custom), 0.0); - let misses = s.layer_cache.stats().misses; - assert_all(colours(&s, file), 0.0); - assert_all(colours(&s, custom), 0.0); - assert_eq!(s.layer_cache.stats().misses, misses, "unchanged files keep the cached layers"); + // A render notes the LUT the config names (a frame on disk is made of it too). let log = ReadLog::new(); let mut r = s.renderer(RenderOpts::default()); r.reads = Some(&log); let _ = r.comp_frame(custom.1, Tick::ZERO); let files = log.into_files(); assert!(files.get(&grade.to_string_lossy()).is_some(), "the render notes the config's LUT: {files:?}"); + assert_all(colours(&s, file), 0.0); + assert_all(colours(&s, custom), 0.0); + let misses = s.layer_cache.stats().misses; + assert_all(colours(&s, file), 0.0); + assert_all(colours(&s, custom), 0.0); + assert_eq!(s.layer_cache.stats().misses, misses, "unchanged files keep the cached layers"); let generation = effectcraft_effects::lut_generation(); write(&lut, WHITE, 2_000); assert_all(colours(&s, file), 1.0); From 36eecad00f7a52b2ba9500651c26967ccb361acd Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 15:30:14 +0530 Subject: [PATCH 5/5] #3538: a cached config's size counts the LUTs it holds --- crates/effects/src/file_cache.rs | 43 ++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/crates/effects/src/file_cache.rs b/crates/effects/src/file_cache.rs index 2bcbefede..88fb75bbd 100644 --- a/crates/effects/src/file_cache.rs +++ b/crates/effects/src/file_cache.rs @@ -147,15 +147,16 @@ fn registry() -> MutexGuard<'static, Vec<&'static dyn ForgetFile>> { } thread_local! { - /// The files read by the loads running on this thread, innermost last. - static READING: RefCell> = const { RefCell::new(Vec::new()) }; + /// The loads running on this thread, innermost last: the files each read, and the bytes of + /// the cached results it holds (a config holds the LUTs its `FileTransform`s loaded). + static READING: RefCell> = const { RefCell::new(Vec::new()) }; } /// Note that the running load read (or looked for) `path`, stamped `stamp`. pub(crate) fn note_file(path: &str, stamp: FileStamp) { READING.with(|r| { if let Ok(mut r) = r.try_borrow_mut() - && let Some(top) = r.last_mut() + && let Some((top, _)) = r.last_mut() && !top.iter().any(|(p, _)| p == path) { top.push((path.to_string(), stamp)); @@ -163,6 +164,17 @@ pub(crate) fn note_file(path: &str, stamp: FileStamp) { }); } +/// Note that the running load holds a cached result of `bytes`. +fn note_bytes(bytes: usize) { + READING.with(|r| { + if let Ok(mut r) = r.try_borrow_mut() + && let Some((_, held)) = r.last_mut() + { + *held = held.saturating_add(bytes); + } + }); +} + /// Pops the files of a load when it ends, even by a panic. struct Reading { popped: bool, @@ -172,17 +184,18 @@ impl Reading { fn start() -> Reading { READING.with(|r| { if let Ok(mut r) = r.try_borrow_mut() { - r.push(Vec::new()); + r.push((Vec::new(), 0)); } }); Reading { popped: false } } - fn pop() -> Option { + fn pop() -> Option<(Files, usize)> { READING.with(|r| r.try_borrow_mut().ok().and_then(|mut r| r.pop())) } - fn files(mut self) -> Files { + /// The files the load read, and the bytes of the cached results it holds. + fn finish(mut self) -> (Files, usize) { self.popped = true; Self::pop().unwrap_or_default() } @@ -230,23 +243,27 @@ pub(crate) fn cached(cache: &Mutex>, src: &str, load: i && e.files == files { e.used = now; - let value = e.value.clone(); + let (value, bytes) = (e.value.clone(), e.bytes); drop(c); - // (an enclosing load depends on these files too) + // (an enclosing load depends on these files too, and holds this result) for (p, s) in &files { note_file(p, *s); } + note_bytes(bytes); return value; } } let reading = Reading::start(); let (value, size) = load(); - let files = reading.files(); + let (files, held) = reading.finish(); follow(&files, stale); for (p, s) in &files { note_file(p, *s); } - let bytes = size.saturating_add(src.len()); + // (what it holds counts against this cache's bound too: another cache letting go of it + // frees nothing while this result keeps it) + let bytes = size.saturating_add(held).saturating_add(src.len()); + note_bytes(bytes); let mut c = lock(cache); c.clock += 1; let used = c.clock; @@ -400,16 +417,20 @@ mod tests { note_missing("/nested/outer-own.cube"); let inner = cached(INNER.get(), "inner", || { note_missing("/nested/inner.cube"); - (7, 0) + (7, 100) }); (inner, 0) }); assert_eq!(v, 7); let files = lock(OUTER.get()).files("outer").unwrap(); + let held = |src: &str| lock(OUTER.get()).map.get(src).map(|e| e.bytes); + // ("inner" is 5 bytes, its result 100: "outer" holds it, on a load and on a hit alike) + assert_eq!(held("outer"), Some(100 + 5 + "outer".len()), "the result an enclosing load holds counts"); assert_eq!(files, vec!["/nested/outer-own.cube".to_string(), "/nested/inner.cube".to_string()]); // A hit on the inner result still reports its files to a new enclosing load. let w = cached(OUTER.get(), "outer 2", || (cached(INNER.get(), "inner", || (0, 0)), 0)); assert_eq!(w, 7); assert_eq!(lock(OUTER.get()).files("outer 2").unwrap(), vec!["/nested/inner.cube".to_string()]); + assert_eq!(held("outer 2"), Some(100 + 5 + "outer 2".len())); } }