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 } }