Merge #2046: Preview: a zoom or resolution change evicts the other sizes' frames first (@arnav-hiwarkar)

crates/ui-egui/src/frames.rs

@@ -152,6 +152,8 @@ struct Cache {
     gpu_budget: usize,
     /// The playing preview, if any: evictions spare the frames it shows next.
     playhead: Option<Playhead>,
+    /// Scale of the frame on screen (the newest urgent request): frames of other scales go first.
+    scale: Option<u32>,
 }
 
 impl Cache {
@@ -173,8 +175,14 @@ impl Cache {
     /// recently shown of the frames it doesn't reach, then the one it reaches last. Plain LRU
     /// evicted the frames just ahead of the playhead first (rendered a while ago, not shown yet)
     /// to keep the ones just shown, so a preview longer than the cache played into a gap that
-    /// prefetch kept refilling while it evicted the next frames (#595).
+    /// prefetch kept refilling while it evicted the next frames (#595). Frames of another scale
+    /// than the one on screen go first (a zoom or resolution change).
     fn victim(&self) -> Option<usize> {
+        if let Some(s) = self.scale
+            && let Some(i) = self.order.iter().position(|k| k.scale != s)
+        {
+            return Some(i);
+        }
         let Some(p) = &self.playhead else { return (!self.order.is_empty()).then_some(0) };
         let mut farthest: Option<(i64, usize)> = None;
         for (i, k) in self.order.iter().enumerate() {
@@ -186,10 +194,13 @@ impl Cache {
         }
         farthest.map(|(_, i)| i)
     }
-    /// Video memory: drop the oldest GPU frames past their own budget.
+    /// Video memory: drop the oldest GPU frames past their own budget, frames of another scale
+    /// than the one on screen first (a zoom or resolution change).
     fn evict_gpu(&mut self) {
         while self.gpu_bytes > self.gpu_budget {
-            let Some(i) = self.order.iter().position(|k| self.map.get(k).is_some_and(FrameImage::is_gpu)) else { break };
+            let is_gpu = |k: &FrameKey| self.map.get(k).is_some_and(FrameImage::is_gpu);
+            let other_scale = self.order.iter().position(|k| Some(k.scale) != self.scale && is_gpu(k));
+            let Some(i) = other_scale.or_else(|| self.order.iter().position(is_gpu)) else { break };
             if let Some(old) = self.order.remove(i) {
                 self.remove(&old);
             }
@@ -475,7 +486,7 @@ impl Default for Frames {
         #[cfg(not(target_arch = "wasm32"))]
         let threads = std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(2, 16);
         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, playhead: 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, playhead: None, scale: None })),
             inflight: Arc::new(Mutex::new(HashSet::new())),
             queue: Arc::new(Mutex::new(Queue::default())),
             #[cfg(not(target_arch = "wasm32"))]
@@ -912,7 +923,19 @@ impl Frames {
     }
 
     fn request_with(&self, src: &RenderSource, key: FrameKey, comp: ItemId, t: Tick, opts: RenderOpts, urgent: bool) {
-        if self.is_cached(&key) {
+        // The frame on screen sets the scale evictions keep: a prefetch or a passive viewer at
+        // another zoom must not evict the active viewer's frames.
+        let cached = self
+            .cache
+            .lock()
+            .map(|mut c| {
+                if urgent {
+                    c.scale = Some(key.scale);
+                }
+                c.map.contains_key(&key)
+            })
+            .unwrap_or(false);
+        if cached {
             return;
         }
         let Ok(mut q) = self.queue.lock() else { return };
@@ -1834,21 +1857,17 @@ mod tests {
         assert!(!f.is_cached(&key(1, 13, 0)) && f.is_cached(&key(1, 50, 0)));
     }
 
-    /// #595: prefetch renders the frames the preview reaches first, whatever order they were
-    /// asked for in (a frame evicted and asked for again waited behind every frame further
-    /// ahead); queued frames the preview no longer reaches are dropped and not left in flight.
-    #[test]
-    fn prefetch_renders_the_frames_the_playhead_reaches_first() {
-        struct Idle;
-        impl RemoteFrames for Idle {
-            fn slots(&self) -> usize {
-                0
-            }
-            fn start(&self, _: RemoteJob, _: RemoteDone) {}
+    /// A frame source that queues jobs and never renders them: the tests check the queue.
+    struct Idle;
+    impl RemoteFrames for Idle {
+        fn slots(&self) -> usize {
+            0
         }
-        let mut f = Frames::default();
-        f.set_remote(Some(Arc::new(Idle)));
-        let src = RenderSource {
+        fn start(&self, _: RemoteJob, _: RemoteDone) {}
+    }
+
+    fn idle_source() -> RenderSource {
+        RenderSource {
             project: Arc::default(),
             footage: Arc::new(effectcraft_engine::render::NoFootage),
             expr: None,
@@ -1856,7 +1875,17 @@ mod tests {
             gpu: None,
             gpu_display: false,
             disk: None,
-        };
+        }
+    }
+
+    /// #595: prefetch renders the frames the preview reaches first, whatever order they were
+    /// asked for in (a frame evicted and asked for again waited behind every frame further
+    /// ahead); queued frames the preview no longer reaches are dropped and not left in flight.
+    #[test]
+    fn prefetch_renders_the_frames_the_playhead_reaches_first() {
+        let mut f = Frames::default();
+        f.set_remote(Some(Arc::new(Idle)));
+        let src = idle_source();
         for frame in [40, 30, 20, 5, 12, 11] {
             f.request(&src, key(1, frame, 0), ItemId(1), Tick::ZERO, RenderOpts::default());
         }
@@ -1873,6 +1902,38 @@ mod tests {
         assert_eq!(f.inflight(), 6, "frame 5, behind the playhead, is no longer in flight");
     }
 
+    /// The cache is full. A zoom (or preview-resolution) change left the frames of the old
+    /// size to the plain least-recently-shown order, so the frames of the zoom the viewer is back
+    /// at went before the ones of the abandoned zoom. The other size's frames go first, then the
+    /// order is as before.
+    #[test]
+    fn a_size_change_evicts_frames_of_other_sizes_first() {
+        let mut f = Frames::default();
+        f.set_remote(Some(Arc::new(Idle)));
+        f.set_budget(4 * 16);
+        let at = |frame, scale| FrameKey { scale, ..key(1, frame, 0) };
+        // 100% (frames 0..=1) is cached, then 50% (2..=3): the 100% frames are the oldest.
+        for (frame, scale) in [(0, 1000), (1, 1000), (2, 500), (3, 500)] {
+            insert(&f, at(frame, scale));
+        }
+        // A passive viewer at 50% asks for a frame: that doesn't change which size is kept.
+        f.request(&idle_source(), at(9, 500), ItemId(1), Tick::ZERO, RenderOpts::default());
+        // Back at 100%: the viewer asks for frame 4.
+        f.request_urgent(&idle_source(), at(4, 1000), ItemId(1), Tick::ZERO, RenderOpts::default());
+        f.request(&idle_source(), at(10, 500), ItemId(1), Tick::ZERO, RenderOpts::default());
+        insert(&f, at(4, 1000));
+        assert!(
+            f.is_cached(&at(0, 1000)) && f.is_cached(&at(1, 1000)) && !f.is_cached(&at(2, 500)) && f.is_cached(&at(3, 500)),
+            "the first frame of the old size goes, not the cached ones of the current size"
+        );
+        insert(&f, at(5, 1000));
+        assert!(!f.is_cached(&at(3, 500)), "then the last frame of the old size");
+        assert_eq!(f.cached_frames(&at(0, 1000)), [0, 1, 4, 5]);
+        // No other size left: least recently shown first again.
+        insert(&f, at(6, 1000));
+        assert_eq!(f.cached_frames(&at(0, 1000)), [1, 4, 5, 6]);
+    }
+
     #[test]
     fn frames_are_keyed_by_content_and_series_are_kept_apart() {
         let f = Frames::default();