Merge pull request #3561 from arnav-hiwarkar/fix/3539-viewer-reload

Viewers show footage reloaded with unchanged metadata (#3539)

crates/ui-egui/src/app.rs

@@ -99,6 +99,7 @@ impl EffectcraftApp {
             footage_watch_key: None,
             lut_watcher: None,
             lut_generation: effectcraft_engine::lut_reload::generation(),
+            purges: 0,
             device_lost_close: None,
             dialog_answers: std::sync::mpsc::channel(),
             file_dialogs: FileDialogsUp::default(),

crates/ui-egui/src/app/frame.rs

@@ -151,7 +151,11 @@ impl EffectcraftApp {
                         self.ui.status = e;
                     }
                 }
-                effectcraft_engine::Event::PurgeCaches => self.frames.clear(),
+                effectcraft_engine::Event::PurgeCaches => {
+                    self.frames.clear();
+                    // (frames made after it get keys of their own: see `Self::purge_stamp`)
+                    self.purges = self.purges.wrapping_add(1);
+                }
                 effectcraft_engine::Event::CopyImage { width, height, rgba } => {
                     if let Err(e) = copy_image(ctx, width, height, &rgba) {
                         self.ui.status = e;

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() ^ self.lut_stamp(),
+            opts: frames::opts_hash(&self.frame_opts(comp, scale)) ^ self.roto_model_stamp() ^ self.lut_stamp() ^ self.purge_stamp(),
         }
     }
 
@@ -149,6 +149,14 @@ impl EffectcraftApp {
         self.lut_generation.wrapping_mul(0x9e37_79b9_7f4a_7c15)
     }
 
+    /// Changes when the caches are purged (Event::PurgeCaches: footage reloaded, Purge): the
+    /// frames on screen and the ones still rendering may show the files as they were, and a reload
+    /// that changes no metadata keeps the project's revision, so the new frames must not share
+    /// their keys (#3539; the viewers show a frame again only when its key changes). 0 until then.
+    fn purge_stamp(&self) -> u64 {
+        self.purges.wrapping_mul(0xc2b2_ae3d_27d4_eb4f)
+    }
+
     /// 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 {
@@ -285,4 +293,54 @@ mod tests {
         effectcraft_engine::lut_reload::check();
         settle(&mut harness, 255);
     }
+
+    /// #3539: a still re-saved at the same size and reloaded as changed on disk keeps the project
+    /// as it was (same revision and content key); the viewers showed the old picture, the new
+    /// frame having the key of the one on screen. A reload's purge gives the frames new keys, once.
+    #[test]
+    fn the_viewers_show_a_still_reloaded_at_the_same_size() {
+        let dir = tempfile::tempdir().unwrap();
+        let png = dir.path().join("still.png");
+        let save = |rgb: [u8; 3]| image::RgbaImage::from_pixel(64, 36, image::Rgba([rgb[0], rgb[1], rgb[2], 255])).save(&png).unwrap();
+        save([255, 0, 0]);
+        let mut session = effectcraft_host::session();
+        session.execute("comp.new", serde_json::json!({"width": 64, "height": 36, "duration": 2})).unwrap();
+        let cid = session.active_comp_id().unwrap();
+        let item = session.execute("file.import", serde_json::json!({"paths": [png.to_string_lossy()]})).unwrap()["items"][0].clone();
+        session.execute("layer.addItem", serde_json::json!({"item": item})).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, 255);
+        // (every viewer, passive ones too, asks for its frame by this key)
+        let key = |h: &egui_kittest::Harness<'_, crate::EffectcraftApp>| h.state().frame_key(cid, 0, 1.0);
+        let before = key(&harness);
+        harness.step();
+        assert_eq!(key(&harness), before, "no purge, the same key: nothing is uploaded again");
+        // Reload Footage Changed on Disk: the app's watcher saw the file's stamp move.
+        let path = png.to_string_lossy().into_owned();
+        let stamps = |n| vec![(path.clone(), Some((n, u128::from(n))))];
+        effectcraft_engine::footage_reload::apply_stamps(&mut harness.state_mut().session, stamps(1)).unwrap();
+        save([0, 0, 255]);
+        let revision = harness.state().session.revision;
+        let reloaded = effectcraft_engine::footage_reload::apply_stamps(&mut harness.state_mut().session, stamps(2)).unwrap();
+        assert_eq!(reloaded["files"], 1, "{reloaded}");
+        assert_eq!(harness.state().session.revision, revision, "the project is as it was");
+        settle(&mut harness, 0);
+        let after = key(&harness);
+        assert_ne!(after, before, "the reloaded frame has a key of its own");
+        harness.step();
+        assert_eq!(key(&harness), after, "one new key per purge");
+    }
 }

crates/ui-egui/src/lib.rs

@@ -377,6 +377,8 @@ pub struct EffectcraftApp {
     lut_watcher: Option<effectcraft_engine::lut_reload::Watcher>,
     /// The LUT generation the frames kept were rendered at ([`effectcraft_engine::lut_reload::generation`]).
     lut_generation: u64,
+    /// How many times the caches were purged ([`Self::purge_stamp`]).
+    purges: 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<bool>,