From d503b26e21997042a3eae50d8ed6e6547098b962 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 10:43:41 +0530 Subject: [PATCH 1/8] #3493: Apply Color LUT reads a LUT file again once it changes on disk load_lut kept the first result for a path for the life of the process: an edited file, or one created after a failed load, was never read again. Each file-backed entry now keeps the file's size and modification time and is read again once they move; inline .cube text stays cached by content. The cache keeps at most 32 LUTs (least recently used goes first). --- crates/effects/src/lib.rs | 2 +- crates/effects/src/utility.rs | 190 +++++++++++++++++++++++++++++++--- 2 files changed, 175 insertions(+), 17 deletions(-) diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 532bb28c9..9105f4587 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -129,7 +129,7 @@ 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, load_lut}; +pub use utility::{Lut, check_lut_files, load_lut, lut_file, lut_generation, 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/utility.rs b/crates/effects/src/utility.rs index f0275eab1..2c663ebca 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -2,7 +2,8 @@ //! lookup tables (.cube). use std::collections::HashMap; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex, OnceLock, PoisonError}; use effectcraft_color::luminance; use effectcraft_keyframe::Value; @@ -330,28 +331,119 @@ impl Lut { } } -type LutCache = Mutex>>>; +/// 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)>; -/// Load (and cache) a LUT from inline `.cube` text or a file path. +/// At most this many LUTs (files and inline texts) are kept; the least recently used goes first. +const MAX_CACHED_LUTS: usize = 32; + +/// A parsed LUT (or the failure to parse one) and what it was read from. +struct CachedLut { + stamp: FileStamp, + lut: Option>, + used: u64, +} + +/// [`load_lut`]'s cache, by parameter text. +#[derive(Default)] +struct LutCache { + map: HashMap, + clock: u64, +} + +/// How often a cached 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. +pub fn lut_params(effect: &str) -> &'static [&'static str] { + match effect { + "ec.utility.applylut" => &["lut"], + "ec.color.lumetri" => &["basicCorrection/inputLutFile", "creative/lookFile"], + _ => &[], + } +} + +/// The file a LUT parameter names; `None` for inline `.cube` text or an empty parameter. +pub fn lut_file(src: &str) -> Option<&str> { + // `.cube` text always spans lines (a size header, then data rows); a path never does, even one + // whose file name contains `LUT_` (#3454). Trimmed, so a trailing newline is still a path. + let path = src.trim(); + (!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 in the cache (one `metadata` call each) and forget the ones that changed +/// on disk since they were read, so the next [`load_lut`] reads them again. Returns +/// [`lut_generation`]. +pub fn check_lut_files() -> u64 { + let files: Vec<(String, FileStamp)> = lut_cache().map.iter().filter_map(|(src, e)| Some((lut_file(src)?.to_string(), e.stamp))).collect(); + // (stamped without the lock: a file on a slow share must not hold up renders) + let changed: Vec = files.into_iter().filter(|(path, stamp)| file_stamp(path) != *stamp).map(|(path, _)| path).collect(); + if !changed.is_empty() { + let mut c = lut_cache(); + c.map.retain(|src, _| lut_file(src).is_none_or(|p| !changed.iter().any(|c| c == p))); + GENERATION.fetch_add(1, Ordering::AcqRel); + } + lut_generation() +} + +/// 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. pub fn load_lut(src: &str) -> Option> { if src.trim().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 file = lut_file(src); + let stamp = file.and_then(file_stamp); + let seen = { + let mut c = lut_cache(); + c.clock += 1; + let now = c.clock; + match c.map.get_mut(src) { + Some(e) if e.stamp == stamp => { + e.used = now; + return e.lut.clone(); + } + Some(e) => Some(e.stamp), + None => None, + } + }; + // (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), } - // `.cube` text always spans lines (a size header, then data rows); a path never does, even one - // whose file name contains `LUT_` (#3454). Trimmed, as the path is, so a trailing newline is a path. - let inline = src.trim().contains('\n'); - let parsed = if inline { parse_cube(src) } else { std::fs::read_to_string(src.trim()).ok().and_then(|t| parse_cube(&t)) }.map(Arc::new); - if let Ok(mut m) = cache.lock() { - m.insert(src.to_string(), parsed.clone()); + .map(Arc::new); + let mut c = lut_cache(); + if seen.is_some_and(|s| s != stamp) { + GENERATION.fetch_add(1, Ordering::AcqRel); } - parsed + let used = c.clock; + c.map.insert(src.to_string(), CachedLut { stamp, lut: lut.clone(), used }); + while c.map.len() > MAX_CACHED_LUTS { + let Some(oldest) = c.map.iter().min_by_key(|(_, e)| e.used).map(|(k, _)| k.clone()) else { break }; + c.map.remove(&oldest); + } + lut } fn apply_lut(ctx: &EffectCtx, mut b: Buf) -> Buf { @@ -500,6 +592,72 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + /// #3493: a LUT file was read once per process; an edited or newly created file never loaded. + #[test] + fn a_lut_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-3493-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let write = |name: &str, text: &str, secs: u64| { + 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() + }; + let at = |src: &str| load_lut(src).map(|l| l.apply([0.5; 3])); + let edited = write("edited.cube", BLACK, 1_000); + assert_eq!(at(&edited), Some([0.0; 3])); + write("edited.cube", WHITE, 2_000); + assert_eq!(at(&edited), Some([1.0; 3]), "an edited file, same size"); + let created = dir.join("created.cube").to_string_lossy().into_owned(); + assert_eq!(at(&created), None); + write("created.cube", WHITE, 1_000); + assert_eq!(at(&created), Some([1.0; 3]), "a file created after a failed load"); + let repaired = write("repaired.cube", "not a lut", 1_000); + assert_eq!(at(&repaired), None); + write("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 LUT. + let inline = load_lut(BLACK).unwrap(); + assert!(Arc::ptr_eq(&inline, &load_lut(BLACK).unwrap())); + std::fs::remove_dir_all(&dir).unwrap(); + } + + /// A file that changed is noticed without a render loading it, and the generation that tells + /// frames made before it apart moves. + #[test] + fn check_lut_files_forgets_a_changed_file() { + let dir = std::env::temp_dir().join(format!("ec-fx-3493-check-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("grade.cube"); + 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"); + 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_eq!(load_lut(&src).map(|l| l.n3), Some(3)); + std::fs::remove_dir_all(&dir).unwrap(); + } + + /// The cache keeps at most [`MAX_CACHED_LUTS`] LUTs, the least recently used going first. + #[test] + fn the_lut_cache_is_bounded() { + let first = load_lut(&identity_cube(2)).unwrap(); + 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!(!Arc::ptr_eq(&first, &load_lut(&identity_cube(2)).unwrap()), "the oldest was 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 6a78d94a5b733a5f2837ce7c4a210adaca0802b8 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 10:51:27 +0530 Subject: [PATCH 2/8] #3493: key cached layers and frames on the LUT files they read A layer graded with Apply Color LUT or Lumetri's Input LUT and Look was cached by the parameter text alone, so the same frame kept showing the old LUT from memory and from the disk cache after the file changed. A layer's cache key (and the key of a layer reading it, through a precomp too) now holds each LUT file's size and modification time, and a render lists the LUT files it read, so a frame kept on disk made with an older file is not used. --- crates/engine/src/tests.rs | 1 + crates/engine/src/tests/lut_files.rs | 154 ++++++++++++++++++++++++ crates/render/src/cache/dependencies.rs | 2 + crates/render/src/pipeline.rs | 9 ++ crates/render/src/reads.rs | 17 +++ 5 files changed, 183 insertions(+) create mode 100644 crates/engine/src/tests/lut_files.rs diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 19953053e..4a5c32b63 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -29,6 +29,7 @@ mod keyframe_text; mod keylight; mod limits; mod lottie; +mod lut_files; mod m137; mod m145; mod markers; diff --git a/crates/engine/src/tests/lut_files.rs b/crates/engine/src/tests/lut_files.rs new file mode 100644 index 000000000..9388e64bd --- /dev/null +++ b/crates/engine/src/tests/lut_files.rs @@ -0,0 +1,154 @@ +//! A LUT file that changes on disk renders again (#3493): Apply Color LUT and Lumetri's Input +//! LUT name a `.cube` file; an edit to it, or a file created, deleted or repaired, shows on the +//! next render of the same frame, from warm layer caches in memory and on disk, through a precomp +//! and a layer reading that precomp, and a frame kept on disk made with the old file is refused. + +use std::path::Path; +use std::time::{Duration, UNIX_EPOCH}; + +use serde_json::{Value, json}; + +use crate::Session; +use crate::render::RenderOpts; +use effectcraft_project::ItemId; +use effectcraft_render::disk_cache::{DiskCache, comp_content_key, frame_key}; +use effectcraft_render::reads::ReadLog; +use effectcraft_time::Tick; + +/// Every input to black, or to white (1D, two entries: the same size, so only the modification +/// time tells the two apart). +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"; +const GREY: f32 = 128.0 / 255.0; + +/// Write `text` to `path` with its modification time `secs` after the epoch. +fn write(path: &Path, text: &str, secs: u64) { + std::fs::write(path, text).unwrap(); + let f = std::fs::File::options().write(true).open(path).unwrap(); + f.set_modified(UNIX_EPOCH + Duration::from_secs(secs)).unwrap(); +} + +/// A session, its layer buffers written to a disk cache in `disk` when given. +fn session(disk: Option<&Path>) -> Session { + let mut s = Session::default(); + if let Some(dir) = disk { + s.execute_checked("prefs.set", json!({"values": {"disk.diskCacheFolder": dir.to_string_lossy(), "disk.diskCacheEnabled": true}})).unwrap(); + s.disk_cache.as_ref().unwrap().set_min_layer_ms(0); + } + s +} + +/// Comp "Grade": a grey solid graded by `effect` (`params` set on it). Comp "Shot" shows it as a +/// precomp layer, under a solid that copies the precomp's channels (Set Channels): that layer is +/// cached with what the precomp shows. Returns (Grade, Shot). +fn shot(s: &mut Session, effect: &str, params: &[(&str, Value)]) -> (ItemId, ItemId) { + s.execute_checked("comp.new", json!({"name": "Shot", "width": 8, "height": 8, "frameRate": 24, "duration": 1})).unwrap(); + let shot = s.active_comp_id().unwrap(); + let plate = s.execute_checked("layer.newSolid", json!({"name": "Plate", "color": "#808080", "width": 8, "height": 8})).unwrap()["layer"].clone(); + s.execute_checked("effect.apply", json!({"layer": plate, "effect": effect})).unwrap(); + for (path, value) in params { + s.execute_checked("prop.set", json!({"layer": plate, "path": format!("effects/#1/{path}"), "value": value})).unwrap(); + } + let pre = s.execute_checked("layer.precompose", json!({"layers": [plate], "name": "Grade"})).unwrap(); + let grade = ItemId(pre["comp"].as_u64().unwrap()); + let copy = s.execute_checked("layer.newSolid", json!({"name": "Copy", "color": "#00ff00", "width": 8, "height": 8})).unwrap()["layer"].clone(); + s.execute_checked("effect.apply", json!({"layer": copy, "effect": "Set Channels"})).unwrap(); + for i in 1..=3 { + s.execute_checked("prop.set", json!({"layer": copy, "path": format!("effects/#1/sourceLayer{i}"), "value": pre["layer"]})).unwrap(); + } + (grade, shot) +} + +/// The red and green of the centre of both comps' first frame. +fn colours(s: &Session, (grade, shot): (ItemId, ItemId)) -> [f32; 4] { + let at = |comp| { + let img = s.render(comp, Tick::ZERO, RenderOpts::default()); + img.get(i64::from(img.width / 2), i64::from(img.height / 2)) + }; + let (g, c) = (at(grade), at(shot)); + [g[0], g[1], c[0], c[1]] +} + +#[track_caller] +fn assert_all(got: [f32; 4], want: f32) { + assert!(got.iter().all(|v| (v - want).abs() < 0.01), "{got:?}, want {want}"); +} + +#[test] +fn an_edited_lut_file_renders_again_from_warm_caches() { + let dir = tempfile::tempdir().unwrap(); + let lut = dir.path().join("grade.cube"); + write(&lut, BLACK, 1_000); + let mut s = session(None); + let comps = shot(&mut s, "Apply Color LUT", &[("lut", json!(lut.to_string_lossy()))]); + assert_all(colours(&s, comps), 0.0); + let misses = s.layer_cache.stats().misses; + assert_all(colours(&s, comps), 0.0); + assert_eq!(s.layer_cache.stats().misses, misses, "an unchanged file keeps the cached layers"); + // A frame on disk made with the file names it. + let frames = DiskCache::open(dir.path().join("frames"), 1 << 30).unwrap(); + let key = frame_key(comp_content_key(&s.project, comps.1), 0, 1000, 0); + let log = ReadLog::new(); + let mut r = s.renderer(RenderOpts::default()); + r.reads = Some(&log); + let _ = r.comp_frame(comps.1, Tick::ZERO); + let files = log.into_files(); + assert!(files.get(&lut.to_string_lossy()).is_some(), "the render notes the LUT file: {files:?}"); + frames.put_frame_now(key, 1, 1, &[0; 4], &files); + assert!(frames.get_frame(key, None).is_some()); + let generation = effectcraft_effects::lut_generation(); + write(&lut, WHITE, 2_000); + assert_all(colours(&s, comps), 1.0); + assert!(effectcraft_effects::lut_generation() > generation, "the viewer's frames made before are dropped"); + assert!(frames.get_frame(key, None).is_none(), "the frame on disk made with the old file"); +} + +#[test] +fn an_edited_lut_file_is_not_served_from_the_disk_cache() { + let dir = tempfile::tempdir().unwrap(); + let (lut, disk) = (dir.path().join("grade.cube"), dir.path().join("cache")); + write(&lut, BLACK, 1_000); + let mut s = session(Some(&disk)); + let comps = shot(&mut s, "Apply Color LUT", &[("lut", json!(lut.to_string_lossy()))]); + assert_all(colours(&s, comps), 0.0); + let dc = s.disk_cache.clone().unwrap(); + dc.flush(); + assert!(dc.stats().entries > 0, "layer buffers written"); + let project = s.project.clone(); + drop((s, dc)); + // A restart on the same cache, after the file changed. + write(&lut, WHITE, 2_000); + let mut s = session(Some(&disk)); + s.project = project; + s.bump(); + assert_all(colours(&s, comps), 1.0); +} + +#[test] +fn a_lut_file_created_deleted_or_repaired_renders_again() { + let dir = tempfile::tempdir().unwrap(); + let lut = dir.path().join("later.cube"); + let mut s = session(None); + let comps = shot(&mut s, "Apply Color LUT", &[("lut", json!(lut.to_string_lossy()))]); + assert_all(colours(&s, comps), GREY); + write(&lut, WHITE, 1_000); + assert_all(colours(&s, comps), 1.0); + std::fs::remove_file(&lut).unwrap(); + assert_all(colours(&s, comps), GREY); + write(&lut, "not a LUT", 2_000); + assert_all(colours(&s, comps), GREY); + write(&lut, BLACK, 3_000); + assert_all(colours(&s, comps), 0.0); +} + +#[test] +fn lumetri_follows_its_input_lut_file() { + let dir = tempfile::tempdir().unwrap(); + let lut = dir.path().join("input.cube"); + write(&lut, BLACK, 1_000); + let mut s = session(None); + let comps = shot(&mut s, "Lumetri Color", &[("basicCorrection/inputLut", json!(1)), ("basicCorrection/inputLutFile", json!(lut.to_string_lossy()))]); + assert_all(colours(&s, comps), 0.0); + write(&lut, WHITE, 2_000); + assert_all(colours(&s, comps), 1.0); +} diff --git a/crates/render/src/cache/dependencies.rs b/crates/render/src/cache/dependencies.rs index dc4216098..4d5dfb5e9 100644 --- a/crates/render/src/cache/dependencies.rs +++ b/crates/render/src/cache/dependencies.rs @@ -33,6 +33,8 @@ impl Dependencies<'_> { { for g in fx.groups().take(limit.unwrap_or(usize::MAX)).filter(|g| g.enabled) { self.group(ctx, layer, g, 0)?; + // A LUT file edited on disk changes the pixels without an edit (#3493). + crate::reads::lut_files(ctx, layer, g).hash(self.h); } } self.other_times = previous; diff --git a/crates/render/src/pipeline.rs b/crates/render/src/pipeline.rs index b1ec0ca02..900a06e2a 100644 --- a/crates/render/src/pipeline.rs +++ b/crates/render/src/pipeline.rs @@ -757,6 +757,15 @@ impl<'a> Renderer<'a> { return; } let Some(fx) = layer.effects() else { return }; + // The LUT files the stack reads go to this render's files: a frame on disk made with one + // that changed since is not used (#3493). + if let Some(log) = self.reads { + for g in fx.groups().take(limit).filter(|g| g.enabled) { + for f in super::reads::lut_files(ctx, layer, g) { + log.note(f); + } + } + } let lt = layer.layer_time(ctx.time); let layer_size = ctx.effect_bounds(layer).0; // #659 / #1205: an authored layer's buffer spans only its painted content. Effects that diff --git a/crates/render/src/reads.rs b/crates/render/src/reads.rs index ed97e50fd..9021f1e42 100644 --- a/crates/render/src/reads.rs +++ b/crates/render/src/reads.rs @@ -65,6 +65,23 @@ 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 +/// 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))) + } + _ => None, + }) + .collect() +} + /// At most this many files are listed per render or cached buffer; more mark the list /// incomplete (a comp reading more files than this for one frame is not checked file by file). pub const MAX_READ_FILES: usize = 4096; From 21665966259c27c1d039a10719a43f7dbb7cf789 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:01:32 +0530 Subject: [PATCH 3/8] #3493: drop the viewer's frames once a LUT file in use changes Renders now follow a LUT file, but the viewer and RAM preview kept the frames already rendered with the old one. A small worker stamps the LUT files in use every two seconds and wakes the interface when one changed; the interface then drops its frames, and a frame whose render started before the change is not kept when it finishes. --- crates/engine/src/footage.rs | 1 + crates/engine/src/footage/lut_reload.rs | 84 +++++++++++++++++++++++++ crates/engine/src/lib.rs | 1 + crates/ui-egui/src/app.rs | 2 + crates/ui-egui/src/app/frame.rs | 15 +++++ crates/ui-egui/src/frames.rs | 15 ++++- crates/ui-egui/src/frames/tests.rs | 23 +++++++ crates/ui-egui/src/lib.rs | 4 ++ 8 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 crates/engine/src/footage/lut_reload.rs diff --git a/crates/engine/src/footage.rs b/crates/engine/src/footage.rs index b921dd128..d64afcaf7 100644 --- a/crates/engine/src/footage.rs +++ b/crates/engine/src/footage.rs @@ -1,6 +1,7 @@ pub mod doc_path; pub mod footage_check; pub mod footage_reload; +pub mod lut_reload; pub mod media_browser; pub mod media_cache; pub mod sequence; diff --git a/crates/engine/src/footage/lut_reload.rs b/crates/engine/src/footage/lut_reload.rs new file mode 100644 index 000000000..96cb14c4a --- /dev/null +++ b/crates/engine/src/footage/lut_reload.rs @@ -0,0 +1,84 @@ +//! LUT files that change on disk (#3493). +//! +//! A render follows a LUT file by itself: [`effectcraft_effects::load_lut`] stamps the file and +//! reads it again once it changed, and a layer's cache key holds the stamp. What the desktop app +//! keeps of earlier renders, the frames of its viewer and RAM preview, is dropped when +//! [`effectcraft_effects::lut_generation`] moves. The [`Watcher`] looks at the LUT files in use +//! every two seconds, so an edit shows without anything rendering first, and wakes the interface +//! only when one changed. + +use std::sync::mpsc::{RecvTimeoutError, SyncSender}; +use std::time::Duration; + +/// Moves each time a LUT file in use was found changed on disk: frames rendered before may show +/// the old LUT ([`effectcraft_effects::lut_generation`]). +pub fn generation() -> u64 { + effectcraft_effects::lut_generation() +} + +/// How often the LUT files in use are stamped. +pub const INTERVAL: Duration = Duration::from_secs(2); + +/// A worker thread that stamps the LUT files in use ([`effectcraft_effects::check_lut_files`]) +/// and runs `wake` when one changed. Dropping the watcher ends the thread. +pub struct Watcher { + _stop: SyncSender<()>, +} + +impl Watcher { + /// Start the worker. `None` where there are no threads (the web app, whose LUTs don't come + /// from files) or one couldn't be started. + pub fn start(wake: impl Fn() + Send + 'static) -> Option { + Self::start_with(wake, INTERVAL) + } + + /// [`Watcher::start`] stamping every `every` (tests). + pub fn start_with(wake: impl Fn() + Send + 'static, every: Duration) -> Option { + if cfg!(target_arch = "wasm32") { + return None; + } + let (tx, rx) = std::sync::mpsc::sync_channel::<()>(1); + std::thread::Builder::new() + .name("lut-watch".into()) + .spawn(move || { + let mut seen = effectcraft_effects::lut_generation(); + // (ends once the watcher, the only sender, is dropped) + while let Err(RecvTimeoutError::Timeout) = rx.recv_timeout(every) { + let now = effectcraft_effects::check_lut_files(); + if now != seen { + seen = now; + wake(); + } + } + }) + .ok()?; + Some(Watcher { _stop: tx }) + } +} + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod tests { + use super::*; + + /// A LUT file edited on disk wakes the interface without a render loading it. + #[test] + fn an_edited_lut_file_wakes_the_interface() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("watched.cube"); + std::fs::write(&path, "LUT_1D_SIZE 2\n0 0 0\n1 1 1\n").unwrap(); + let src = path.to_string_lossy().into_owned(); + assert!(effectcraft_effects::load_lut(&src).is_some()); + let before = effectcraft_effects::lut_generation(); + let (tx, rx) = std::sync::mpsc::channel(); + let _watcher = Watcher::start_with( + move || { + let _ = tx.send(()); + }, + Duration::from_millis(10), + ) + .unwrap(); + std::fs::write(&path, "LUT_1D_SIZE 3\n0 0 0\n0.5 0.5 0.5\n1 1 1\n").unwrap(); + rx.recv_timeout(Duration::from_secs(10)).unwrap(); + assert!(effectcraft_effects::lut_generation() > before); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f8edfbe08..0b7608756 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -61,6 +61,7 @@ pub use effectcraft_track as track; pub use footage::doc_path; pub use footage::footage_check; pub use footage::footage_reload; +pub use footage::lut_reload; pub use footage::media_browser; pub use footage::media_cache; pub use footage::sequence; diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 66387bc31..bf5964ba6 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -97,6 +97,8 @@ impl EffectcraftApp { gpu_failure_applied: None, footage_watcher: None, footage_watch_key: None, + lut_watcher: None, + lut_generation: effectcraft_engine::lut_reload::generation(), device_lost_close: None, dialog_answers: std::sync::mpsc::channel(), file_dialogs: FileDialogsUp::default(), diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index c3a5bfbf6..c76e3f410 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -654,6 +654,21 @@ impl EffectcraftApp { ctx.request_repaint_after(std::time::Duration::from_secs(10)); } self.tick_footage_reload(ctx); + self.tick_lut_files(ctx); + } + + /// A LUT file in use changed on disk (#3493): the frames kept were rendered with the old one. + /// Renders read it again by themselves; the watcher wakes the interface when no render did. + fn tick_lut_files(&mut self, ctx: &egui::Context) { + if self.lut_watcher.is_none() { + let wake = ctx.clone(); + self.lut_watcher = effectcraft_engine::lut_reload::Watcher::start(move || wake.request_repaint()); + } + let generation = effectcraft_engine::lut_reload::generation(); + if generation != self.lut_generation { + self.lut_generation = generation; + self.frames.clear_stale(); + } } /// One background worker tracks source availability and optional content reloads. It wakes diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index a0c2f94b5..ccc4123d2 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -298,6 +298,9 @@ struct Cache { playhead: Option, /// Scale of the frame on screen (the newest urgent request): frames of other scales go first. scale: Option, + /// Frames whose render started before this are not kept: a source file they read changed + /// since ([`Frames::clear_stale`]). + stale_before: Option, } impl Cache { @@ -768,7 +771,7 @@ impl Frames { #[cfg(target_arch = "wasm32")] let queue = Queue::default(); Frames { - cache: Arc::new(Mutex::new(Cache { map: HashMap::new(), order: VecDeque::new(), bytes: 0, gpu_bytes: 0, budget: 3 << 30, gpu_budget: GPU_BUDGET, gpu_pressure: None, gpu_adjusted: None, playhead: None, scale: None })), + cache: Arc::new(Mutex::new(Cache { map: HashMap::new(), order: VecDeque::new(), bytes: 0, gpu_bytes: 0, budget: 3 << 30, gpu_budget: GPU_BUDGET, gpu_pressure: None, gpu_adjusted: None, playhead: None, scale: None, stale_before: None })), inflight: Arc::new(Mutex::new(HashSet::new())), queue: Arc::new(Mutex::new(queue)), #[cfg(not(target_arch = "wasm32"))] @@ -1403,6 +1406,15 @@ impl Frames { } } + /// Drop every frame, and the ones still rendering when they finish: a source file changed on + /// disk (a LUT file, #3493), and a render that started before may have read the old one. + pub fn clear_stale(&self) { + self.clear(); + if let Ok(mut c) = self.cache.lock() { + c.stale_before = Some(web_time::Instant::now()); + } + } + /// The playing preview's position (`None` when no preview plays): prefetch renders the /// frames it reaches first before the others, evictions spare the frames it shows next, and /// queued prefetch frames of its series it no longer reaches are dropped (#595). @@ -1899,6 +1911,7 @@ impl Worker { } if let Ok(mut c) = self.cache.lock() && (!img.is_gpu() || !self.gpu_retired.load(Ordering::Acquire)) + && c.stale_before.is_none_or(|at| t0 >= at) { c.insert_with(key, img, files, since); } diff --git a/crates/ui-egui/src/frames/tests.rs b/crates/ui-egui/src/frames/tests.rs index 45398de21..d6bf56a20 100644 --- a/crates/ui-egui/src/frames/tests.rs +++ b/crates/ui-egui/src/frames/tests.rs @@ -218,6 +218,29 @@ fn retirement_reaches_existing_workers_and_keeps_cpu_frames_and_completion_live( assert!(frames.queue.lock().unwrap().running.is_empty()); } +/// #3493: a LUT file changed on disk drops the frames kept, and a frame whose render started +/// before is not kept when it finishes (it may have read the old file); one started after is. +#[test] +fn clear_stale_drops_kept_frames_and_ones_rendering() { + let frames = Frames::default(); + let worker = frames.worker(); + let kept = key(1, 0, 0); + insert(&frames, kept); + let (late, fresh) = (key(1, 1, 0), key(1, 2, 0)); + let started = web_time::Instant::now(); + frames.clear_stale(); + assert!(!frames.is_cached(&kept)); + for k in [late, fresh] { + frames.inflight.lock().unwrap().insert(k); + frames.queue.lock().unwrap().running.insert(k); + } + worker.finish_key(late, false, img(), None, None, started); + assert!(!frames.is_cached(&late), "a render started before the change"); + worker.finish_key(fresh, false, img(), None, None, web_time::Instant::now()); + assert!(frames.is_cached(&fresh), "a render started after it"); + assert_eq!(frames.inflight(), 0, "both jobs are released"); +} + fn key(content: u64, frame: i64, opts: u64) -> FrameKey { FrameKey { revision: content, content, comp: 1, frame, scale: 1000, view: 0, opts } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 2e911ea54..699f68e0f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -373,6 +373,10 @@ pub struct EffectcraftApp { footage_watcher: Option, /// The project revision and preference the watched set was last computed for. footage_watch_key: Option<(u64, String, bool)>, + /// The worker that stamps the LUT files in use ([`Self::tick_lut_files`], #3493). + lut_watcher: Option, + /// The LUT generation the frames kept were rendered at ([`effectcraft_engine::lut_reload::generation`]). + lut_generation: u64, /// The graphics device was lost and the window is closing (see `close_after_device_loss`); /// `true` keeps crash recovery for the next launch. device_lost_close: Option, From 63aca62574e6ba27db589b9a7751fce9f35f47a3 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:13:12 +0530 Subject: [PATCH 4/8] #3493: follow LUT files whose LUT left the cache; bound it by bytes too The watcher stamped only the files whose parsed LUT was still cached, so with more LUTs than the cache keeps, an edit to an evicted one left the viewer's frames stale. The cache now remembers the stamp each LUT file was last read with (bounded separately), and keeps at most 64 LUTs and 512 MiB of tables. Frames finishing while the viewer empties its cache are refused. --- crates/effects/src/utility.rs | 123 ++++++++++++++++++++++++++-------- crates/ui-egui/src/frames.rs | 3 +- 2 files changed, 98 insertions(+), 28 deletions(-) diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index 2c663ebca..1995c2881 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -335,24 +335,63 @@ impl Lut { /// that could not be described). A file whose stamp moved is read again (#3493). type FileStamp = Option<(u64, Option)>; -/// At most this many LUTs (files and inline texts) are kept; the least recently used goes first. -const MAX_CACHED_LUTS: usize = 32; +/// 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; /// 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. +/// [`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, } -/// How often a cached LUT file was found changed on disk ([`lut_generation`]). +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, _| 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> { @@ -388,17 +427,25 @@ pub fn lut_generation() -> u64 { GENERATION.load(Ordering::Acquire) } -/// Look at every LUT file in the cache (one `metadata` call each) and forget the ones that changed -/// on disk since they were read, so the next [`load_lut`] reads them again. Returns -/// [`lut_generation`]. +/// 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().map.iter().filter_map(|(src, e)| Some((lut_file(src)?.to_string(), e.stamp))).collect(); + 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 = files.into_iter().filter(|(path, stamp)| file_stamp(path) != *stamp).map(|(path, _)| path).collect(); + 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(); - c.map.retain(|src, _| lut_file(src).is_none_or(|p| !changed.iter().any(|c| c == p))); - GENERATION.fetch_add(1, Ordering::AcqRel); + 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() } @@ -413,19 +460,17 @@ pub fn load_lut(src: &str) -> Option> { } let file = lut_file(src); let stamp = file.and_then(file_stamp); - let seen = { + { let mut c = lut_cache(); c.clock += 1; let now = c.clock; - match c.map.get_mut(src) { - Some(e) if e.stamp == stamp => { - e.used = now; - return e.lut.clone(); - } - Some(e) => Some(e.stamp), - None => None, + if let Some(e) = c.map.get_mut(src) + && e.stamp == stamp + { + e.used = now; + return e.lut.clone(); } - }; + } // (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 { @@ -433,16 +478,17 @@ pub fn load_lut(src: &str) -> Option> { 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 seen.is_some_and(|s| s != stamp) { - GENERATION.fetch_add(1, Ordering::AcqRel); + 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 }); - while c.map.len() > MAX_CACHED_LUTS { - let Some(oldest) = c.map.iter().min_by_key(|(_, e)| e.used).map(|(k, _)| k.clone()) else { break }; - c.map.remove(&oldest); - } + c.map.insert(src.to_string(), CachedLut { stamp, lut: lut.clone(), used, bytes }); + c.bytes = c.bytes.saturating_add(bytes); + c.evict(src); lut } @@ -656,6 +702,29 @@ mod tests { assert!(lut_cache().map.len() <= MAX_CACHED_LUTS); } assert!(!Arc::ptr_eq(&first, &load_lut(&identity_cube(2)).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); + } + + /// A LUT file whose LUT left the cache is still followed: its frames are dropped when it + /// changes, though no render loads it again first. + #[test] + fn a_file_whose_lut_left_the_cache_is_still_followed() { + let dir = std::env::temp_dir().join(format!("ec-fx-3493-evicted-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("old.cube"); + std::fs::write(&path, identity_cube(2)).unwrap(); + let src = path.to_string_lossy().into_owned(); + assert!(load_lut(&src).is_some()); + 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"); + 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"); + std::fs::remove_dir_all(&dir).unwrap(); } #[test] diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index ccc4123d2..6d99a8cd2 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -1409,10 +1409,11 @@ impl Frames { /// Drop every frame, and the ones still rendering when they finish: a source file changed on /// disk (a LUT file, #3493), and a render that started before may have read the old one. pub fn clear_stale(&self) { - self.clear(); + // (marked first: a frame finishing while the cache is emptied is refused, not kept) if let Ok(mut c) = self.cache.lock() { c.stale_before = Some(web_time::Instant::now()); } + self.clear(); } /// The playing preview's position (`None` when no preview plays): prefetch renders the From 82c2a23f0793c6c0aa1deeaea8aa926336a0cd51 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:29:57 +0530 Subject: [PATCH 5/8] #3493: keep following the LUT file just read past the followed-files limit Past 1024 followed files, the trim kept only files with a cached LUT and dropped the one being noted (its LUT is cached right after), so a later edit to it went unnoticed. The tests start from an empty folder. --- crates/effects/src/utility.rs | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index 1995c2881..8d9c38927 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -378,7 +378,7 @@ impl LutCache { } if self.files.len() > MAX_FOLLOWED_FILES { let map = &self.map; - self.files.retain(|p, _| map.keys().any(|src| lut_file(src) == Some(p.as_str()))); + self.files.retain(|p, _| p.as_str() == path || map.keys().any(|src| lut_file(src) == Some(p.as_str()))); } } @@ -644,6 +644,7 @@ mod tests { 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-3493-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); let write = |name: &str, text: &str, secs: u64| { let path = dir.join(name); @@ -678,6 +679,7 @@ mod tests { #[test] fn check_lut_files_forgets_a_changed_file() { let dir = std::env::temp_dir().join(format!("ec-fx-3493-check-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); let path = dir.join("grade.cube"); std::fs::write(&path, identity_cube(2)).unwrap(); @@ -695,13 +697,15 @@ mod tests { /// The cache keeps at most [`MAX_CACHED_LUTS`] LUTs, the least recently used going first. #[test] fn the_lut_cache_is_bounded() { - let first = load_lut(&identity_cube(2)).unwrap(); + // (text no other test loads: a load elsewhere would make it recently used) + let oldest = format!("TITLE \"bounded first\"\n{}", identity_cube(2)); + let first = load_lut(&oldest).unwrap(); 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!(!Arc::ptr_eq(&first, &load_lut(&identity_cube(2)).unwrap()), "the oldest was let go"); + 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); @@ -712,6 +716,7 @@ mod tests { #[test] fn a_file_whose_lut_left_the_cache_is_still_followed() { let dir = std::env::temp_dir().join(format!("ec-fx-3493-evicted-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); let path = dir.join("old.cube"); std::fs::write(&path, identity_cube(2)).unwrap(); @@ -727,6 +732,22 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + /// Past [`MAX_FOLLOWED_FILES`], the file just read is still followed. + #[test] + fn the_file_just_read_stays_followed_past_the_limit() { + let dir = std::env::temp_dir().join(format!("ec-fx-3493-many-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + for i in 0..MAX_FOLLOWED_FILES + 8 { + let path = dir.join(format!("{i}.cube")); + std::fs::write(&path, identity_cube(2)).unwrap(); + let src = path.to_string_lossy().into_owned(); + assert!(load_lut(&src).is_some()); + assert!(lut_cache().files.contains_key(&src), "{i}: the file just read"); + } + std::fs::remove_dir_all(&dir).unwrap(); + } + #[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 d036127bf4076e16af45909c713529284a34aac1 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:29:57 +0530 Subject: [PATCH 6/8] #3493: key a layer reading a precomp on the LUT file its override names A precomp instance whose Essential Property names another LUT file renders with that file, but a layer reading the instance through a layer parameter was keyed on the nested comp's own file, so editing the overriding file left that layer cached. --- crates/engine/src/tests/lut_files.rs | 34 +++++++++++++++++++++++++ crates/render/src/cache/dependencies.rs | 11 ++++++++ 2 files changed, 45 insertions(+) diff --git a/crates/engine/src/tests/lut_files.rs b/crates/engine/src/tests/lut_files.rs index 9388e64bd..57d163b36 100644 --- a/crates/engine/src/tests/lut_files.rs +++ b/crates/engine/src/tests/lut_files.rs @@ -152,3 +152,37 @@ fn lumetri_follows_its_input_lut_file() { write(&lut, WHITE, 2_000); assert_all(colours(&s, comps), 1.0); } + +/// A precomp instance whose Essential Property names another LUT file follows that file, also +/// for a layer that reads the instance through a layer parameter. +#[test] +fn an_overridden_lut_file_renders_again() { + let dir = tempfile::tempdir().unwrap(); + let (base, overridden) = (dir.path().join("base.cube"), dir.path().join("override.cube")); + write(&base, WHITE, 1_000); + write(&overridden, BLACK, 1_000); + let mut s = session(None); + s.execute_checked("comp.new", json!({"name": "Grade", "width": 8, "height": 8, "frameRate": 24, "duration": 1})).unwrap(); + let grade = s.active_comp_id().unwrap(); + let plate = s.execute_checked("layer.newSolid", json!({"name": "Plate", "color": "#808080", "width": 8, "height": 8})).unwrap()["layer"].clone(); + s.execute_checked("effect.apply", json!({"layer": plate, "effect": "Apply Color LUT"})).unwrap(); + s.execute_checked("prop.set", json!({"layer": plate, "path": "effects/#1/lut", "value": base.to_string_lossy()})).unwrap(); + let control = s.execute_checked("essential.addProperty", json!({"layer": plate, "path": "effects/#1/lut", "name": "LUT"})).unwrap()["controls"][0].clone(); + s.execute_checked("comp.new", json!({"name": "Shot", "width": 8, "height": 8, "frameRate": 24, "duration": 1})).unwrap(); + let shot = s.active_comp_id().unwrap(); + let instance = s.execute_checked("layer.addItem", json!({"item": grade.0})).unwrap()["layer"].clone(); + s.execute_checked("essential.set", json!({"layer": instance, "control": control, "value": overridden.to_string_lossy()})).unwrap(); + let copy = s.execute_checked("layer.newSolid", json!({"name": "Copy", "color": "#00ff00", "width": 8, "height": 8})).unwrap()["layer"].clone(); + s.execute_checked("effect.apply", json!({"layer": copy, "effect": "Set Channels"})).unwrap(); + for i in 1..=3 { + s.execute_checked("prop.set", json!({"layer": copy, "path": format!("effects/#1/sourceLayer{i}"), "value": instance})).unwrap(); + } + let centre = |s: &Session| { + let img = s.render(shot, Tick::ZERO, RenderOpts::default()); + let p = img.get(i64::from(img.width / 2), i64::from(img.height / 2)); + [p[0], p[1], p[0], p[1]] + }; + assert_all(centre(&s), 0.0); + write(&overridden, WHITE, 2_000); + assert_all(centre(&s), 1.0); +} diff --git a/crates/render/src/cache/dependencies.rs b/crates/render/src/cache/dependencies.rs index 4d5dfb5e9..5e8a8845b 100644 --- a/crates/render/src/cache/dependencies.rs +++ b/crates/render/src/cache/dependencies.rs @@ -171,6 +171,17 @@ impl Dependencies<'_> { ItemKind::Comp(comp) => { let time = ctx.source_time(layer); time.0.hash(self.h); + // The nested layers are walked without this instance's Essential + // 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); + } + } let nested = EvalCtx::new(ctx.project, item, comp, time); for child in &comp.layers { self.layer(&nested, child)?; From dc3e5245c2ca9a2f1e64a4452d7f3dccb1d3d064 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:29:57 +0530 Subject: [PATCH 7/8] #3493: show the frame again in the viewers once a LUT file changes The viewers replace their picture only when the frame key changes, and a frame rendered again after a LUT change had the same key, so the old look stayed on screen. The viewer frame keys carry the LUT generation (in memory only), which also leaves frames still rendering with the old LUT unasked for, so the refusal of late frames is no longer needed. --- crates/engine/src/footage/lut_reload.rs | 8 +++- crates/ui-egui/src/app/frame.rs | 4 +- crates/ui-egui/src/app/viewer_frames.rs | 50 ++++++++++++++++++++++++- crates/ui-egui/src/frames.rs | 16 +------- crates/ui-egui/src/frames/tests.rs | 23 ------------ 5 files changed, 60 insertions(+), 41 deletions(-) diff --git a/crates/engine/src/footage/lut_reload.rs b/crates/engine/src/footage/lut_reload.rs index 96cb14c4a..54211eac0 100644 --- a/crates/engine/src/footage/lut_reload.rs +++ b/crates/engine/src/footage/lut_reload.rs @@ -16,6 +16,12 @@ pub fn generation() -> u64 { effectcraft_effects::lut_generation() } +/// Stamp the LUT files in use now ([`effectcraft_effects::check_lut_files`]): the ones that +/// changed are read again by the next render. Returns [`generation`]. +pub fn check() -> u64 { + effectcraft_effects::check_lut_files() +} + /// How often the LUT files in use are stamped. pub const INTERVAL: Duration = Duration::from_secs(2); @@ -44,7 +50,7 @@ impl Watcher { let mut seen = effectcraft_effects::lut_generation(); // (ends once the watcher, the only sender, is dropped) while let Err(RecvTimeoutError::Timeout) = rx.recv_timeout(every) { - let now = effectcraft_effects::check_lut_files(); + let now = check(); if now != seen { seen = now; wake(); diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index c76e3f410..488b3c5ef 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -659,6 +659,8 @@ impl EffectcraftApp { /// A LUT file in use changed on disk (#3493): the frames kept were rendered with the old one. /// Renders read it again by themselves; the watcher wakes the interface when no render did. + /// The frame keys carry the generation ([`Self::frame_series`]), so the viewers show the + /// frames rendered again, and a frame still rendering with the old LUT is never asked for. fn tick_lut_files(&mut self, ctx: &egui::Context) { if self.lut_watcher.is_none() { let wake = ctx.clone(); @@ -667,7 +669,7 @@ impl EffectcraftApp { let generation = effectcraft_engine::lut_reload::generation(); if generation != self.lut_generation { self.lut_generation = generation; - self.frames.clear_stale(); + self.frames.clear(); } } diff --git a/crates/ui-egui/src/app/viewer_frames.rs b/crates/ui-egui/src/app/viewer_frames.rs index fe3ad8abb..d7c02a99c 100644 --- a/crates/ui-egui/src/app/viewer_frames.rs +++ b/crates/ui-egui/src/app/viewer_frames.rs @@ -128,7 +128,7 @@ impl EffectcraftApp { frame: 0, scale: (scale * 1000.0).round() as u32, view: self.view_hash(comp), - opts: frames::opts_hash(&self.frame_opts(comp, scale)) ^ self.roto_model_stamp(), + opts: frames::opts_hash(&self.frame_opts(comp, scale)) ^ self.roto_model_stamp() ^ self.lut_stamp(), } } @@ -142,6 +142,13 @@ impl EffectcraftApp { h.finish() | 1 } + /// Changes when a LUT file in use changed on disk (#3493): the frame on screen and the ones + /// still rendering were made with the old LUT, so the new ones must not share their keys + /// (the viewers show a frame again only when its key changes). 0 until a LUT file changes. + fn lut_stamp(&self) -> u64 { + self.lut_generation.wrapping_mul(0x9e37_79b9_7f4a_7c15) + } + /// The frame series the viewer of `comp` shows now (its scale, view and options, for the /// comp's current content): what the cache bars count. pub fn shown_series(&self, comp: ItemId) -> FrameKey { @@ -238,3 +245,44 @@ impl EffectcraftApp { request(&self.frames, &self.render_source(), key, comp, t, opts); } } + +#[cfg(test)] +mod tests { + /// #3493: the viewer shows the frame again once the LUT file it was graded with changes, + /// though its key was otherwise the same (same project revision, frame and options). + #[test] + fn the_viewer_shows_a_lut_file_changed_on_disk() { + let dir = tempfile::tempdir().unwrap(); + let lut = dir.path().join("viewer.cube"); + let write = |text: &str, secs: u64| { + std::fs::write(&lut, text).unwrap(); + let f = std::fs::File::options().write(true).open(&lut).unwrap(); + f.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(secs)).unwrap(); + }; + write("LUT_1D_SIZE 2\n0 0 0\n0 0 0\n", 1_000); + let mut session = effectcraft_engine::Session::default(); + session.execute("comp.new", serde_json::json!({"width": 64, "height": 36, "duration": 2})).unwrap(); + session.execute("layer.newSolid", serde_json::json!({"color": "#808080"})).unwrap(); + session.execute("effect.apply", serde_json::json!({"layer": 1, "effect": "Apply Color LUT"})).unwrap(); + session.execute("prop.set", serde_json::json!({"layer": 1, "path": "effects/#1/lut", "value": lut.to_string_lossy()})).unwrap(); + let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| crate::EffectcraftApp::new(session)); + let centre = |h: &egui_kittest::Harness<'_, crate::EffectcraftApp>| { + h.state().viewer_image.as_ref().map(|img| img.pixels[img.pixels.len() / 2 + img.size[0] / 2].r()) + }; + let settle = |h: &mut egui_kittest::Harness<'_, crate::EffectcraftApp>, want: u8| { + for _ in 0..400 { + h.step(); + if centre(h) == Some(want) { + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + panic!("the viewer shows {:?}, not {want}", centre(h)); + }; + settle(&mut harness, 0); + write("LUT_1D_SIZE 2\n1 1 1\n1 1 1\n", 2_000); + // (what the app's watcher does every two seconds) + effectcraft_engine::lut_reload::check(); + settle(&mut harness, 255); + } +} diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 6d99a8cd2..a0c2f94b5 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -298,9 +298,6 @@ struct Cache { playhead: Option, /// Scale of the frame on screen (the newest urgent request): frames of other scales go first. scale: Option, - /// Frames whose render started before this are not kept: a source file they read changed - /// since ([`Frames::clear_stale`]). - stale_before: Option, } impl Cache { @@ -771,7 +768,7 @@ impl Frames { #[cfg(target_arch = "wasm32")] let queue = Queue::default(); Frames { - cache: Arc::new(Mutex::new(Cache { map: HashMap::new(), order: VecDeque::new(), bytes: 0, gpu_bytes: 0, budget: 3 << 30, gpu_budget: GPU_BUDGET, gpu_pressure: None, gpu_adjusted: None, playhead: None, scale: None, stale_before: None })), + cache: Arc::new(Mutex::new(Cache { map: HashMap::new(), order: VecDeque::new(), bytes: 0, gpu_bytes: 0, budget: 3 << 30, gpu_budget: GPU_BUDGET, gpu_pressure: None, gpu_adjusted: None, playhead: None, scale: None })), inflight: Arc::new(Mutex::new(HashSet::new())), queue: Arc::new(Mutex::new(queue)), #[cfg(not(target_arch = "wasm32"))] @@ -1406,16 +1403,6 @@ impl Frames { } } - /// Drop every frame, and the ones still rendering when they finish: a source file changed on - /// disk (a LUT file, #3493), and a render that started before may have read the old one. - pub fn clear_stale(&self) { - // (marked first: a frame finishing while the cache is emptied is refused, not kept) - if let Ok(mut c) = self.cache.lock() { - c.stale_before = Some(web_time::Instant::now()); - } - self.clear(); - } - /// The playing preview's position (`None` when no preview plays): prefetch renders the /// frames it reaches first before the others, evictions spare the frames it shows next, and /// queued prefetch frames of its series it no longer reaches are dropped (#595). @@ -1912,7 +1899,6 @@ impl Worker { } if let Ok(mut c) = self.cache.lock() && (!img.is_gpu() || !self.gpu_retired.load(Ordering::Acquire)) - && c.stale_before.is_none_or(|at| t0 >= at) { c.insert_with(key, img, files, since); } diff --git a/crates/ui-egui/src/frames/tests.rs b/crates/ui-egui/src/frames/tests.rs index d6bf56a20..45398de21 100644 --- a/crates/ui-egui/src/frames/tests.rs +++ b/crates/ui-egui/src/frames/tests.rs @@ -218,29 +218,6 @@ fn retirement_reaches_existing_workers_and_keeps_cpu_frames_and_completion_live( assert!(frames.queue.lock().unwrap().running.is_empty()); } -/// #3493: a LUT file changed on disk drops the frames kept, and a frame whose render started -/// before is not kept when it finishes (it may have read the old file); one started after is. -#[test] -fn clear_stale_drops_kept_frames_and_ones_rendering() { - let frames = Frames::default(); - let worker = frames.worker(); - let kept = key(1, 0, 0); - insert(&frames, kept); - let (late, fresh) = (key(1, 1, 0), key(1, 2, 0)); - let started = web_time::Instant::now(); - frames.clear_stale(); - assert!(!frames.is_cached(&kept)); - for k in [late, fresh] { - frames.inflight.lock().unwrap().insert(k); - frames.queue.lock().unwrap().running.insert(k); - } - worker.finish_key(late, false, img(), None, None, started); - assert!(!frames.is_cached(&late), "a render started before the change"); - worker.finish_key(fresh, false, img(), None, None, web_time::Instant::now()); - assert!(frames.is_cached(&fresh), "a render started after it"); - assert_eq!(frames.inflight(), 0, "both jobs are released"); -} - fn key(content: u64, frame: i64, opts: u64) -> FrameKey { FrameKey { revision: content, content, comp: 1, frame, scale: 1000, view: 0, opts } } From 63520598a016b407ed457a56bf679f4d3437f862 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:36:40 +0530 Subject: [PATCH 8/8] #3493: test the followed-files limit on a cache of its own The test filled the shared cache past the limit, which could let go of a file another test running alongside still follows. --- crates/effects/src/utility.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index 8d9c38927..9d5fec491 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -732,20 +732,17 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } - /// Past [`MAX_FOLLOWED_FILES`], the file just read is still followed. + /// 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 dir = std::env::temp_dir().join(format!("ec-fx-3493-many-{}", std::process::id())); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).unwrap(); + let mut c = LutCache::default(); for i in 0..MAX_FOLLOWED_FILES + 8 { - let path = dir.join(format!("{i}.cube")); - std::fs::write(&path, identity_cube(2)).unwrap(); - let src = path.to_string_lossy().into_owned(); - assert!(load_lut(&src).is_some()); - assert!(lut_cache().files.contains_key(&src), "{i}: the file just read"); + let path = format!("/followed/{i}.cube"); + c.read_file(&path, Some((1, None))); + assert!(c.files.contains_key(&path), "{i}: the file just read"); } - std::fs::remove_dir_all(&dir).unwrap(); + assert!(c.files.len() <= MAX_FOLLOWED_FILES, "the others no longer cached are let go"); } #[test]