#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

@@ -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();

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();
         }
     }
 

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);
+    }
+}

crates/ui-egui/src/frames.rs

@@ -298,9 +298,6 @@ struct Cache {
     playhead: Option<Playhead>,
     /// Scale of the frame on screen (the newest urgent request): frames of other scales go first.
     scale: Option<u32>,
-    /// Frames whose render started before this are not kept: a source file they read changed
-    /// since ([`Frames::clear_stale`]).
-    stale_before: Option<web_time::Instant>,
 }
 
 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);
         }

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