fix(ui): stop dropping cached tiles when zooming away and back (#2196) (#2323)

- Retain tiles of on-screen pages at both eviction sites in
  crates/ui-egui/src/canvas.rs, so a pan or zoom that moves the viewport no
  longer discards tiles that were rendered sharp moments earlier.
- Stop dropping a zoom's tiles when zooming out far enough shows the page
  whole; that made returning to a visited zoom a cold miss, which re-blurred
  the page and re-interpreted it once per tile.
- Bound older-zoom retention by bytes (TILE_BYTES) as well as by count
  (OLD_TILES): a tile of a large scan can be 4096 squared, about 67 MiB, so a
  count alone ran past the tile allowance.
- Add returning_to_a_visited_zoom_reuses_its_cached_tiles, which fails on the
  reverted fix, and cite both view tests in view.deep-zoom-tiles; the entry
  stays partial because the 8192 px-per-side cap is untouched.

crates/ui-egui/src/canvas.rs

@@ -836,6 +836,13 @@ impl DocView {
         !self.last_queue.is_empty() || self.text_background.pending() || self.move_background.pending()
     }
 
+    /// The cached tile keys of a page, as (scale tag, tile x, tile y) (diagnostics and tests).
+    pub fn tiles_debug(&self, page: usize) -> Vec<(u64, u32, u32)> {
+        let mut k: Vec<(u64, u32, u32)> = self.tiles.keys().filter(|key| key.0 == page).map(|key| (key.1, key.2, key.3)).collect();
+        k.sort_unstable();
+        k
+    }
+
     /// Re-render every page and text layer (the document's appearance changed, e.g. a layer
     /// was toggled). Out-of-date rasters stay on screen until their replacements arrive, so the
     /// view never flashes blank.
@@ -1562,8 +1569,13 @@ impl DocView {
         }
         self.pages.retain(|p, _| pages.contains(p));
         // Tiles of an earlier zoom (not a demanded scale) stay while the canvas keeps them, a
-        // bounded few under a page whose current tiles are still missing (OLD_TILES).
-        self.tiles.retain(|key, _| tiles.contains(key) || !tile_tags.contains(&key.1));
+        // bounded few under a page whose current tiles are still missing (OLD_TILES). Tiles of a
+        // page on screen stay too even when this frame does not demand them (#2196): the viewport
+        // moves while zooming, so a tile that was sharp a moment ago falls outside the demanded
+        // rectangle, and dropping it made every pan or zoom back a cold miss that re-blurred the
+        // page and re-interpreted it once per tile. The per-frame cap below (OLD_TILES,
+        // nearest scale first) is what bounds how much of that stays.
+        self.tiles.retain(|key, _| tiles.contains(key) || !tile_tags.contains(&key.1) || self.frame_visible.contains(&key.0));
         self.stale_tiles.retain(|key| self.tiles.contains_key(key));
         queue.extend(grid);
         // Opening (or turning to) a page queues its pixels alongside thumbnails and
@@ -3391,18 +3403,37 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) {
     let (mut wanted, visible_now) = out.inner;
     let cur = view.current;
     wanted.sort_by_key(|w| (!visible_now.contains(&w.0), w.0.abs_diff(cur), w.3.is_some()));
-    // Tiles of far-away pages are useless: free them. Tiles of other zooms stay only while a
-    // current tile of their page is still missing, at most `OLD_TILES` of them, nearest scale first.
+    // Tiles of far-away pages are useless: free them. Tiles of an earlier zoom stay for a page
+    // that is on screen, whether or not the page is tiled at this zoom (#2196): a zoom out below
+    // the tiling threshold used to drop the tiles of the zoom just left, so coming back to it
+    // was a cold miss and the page re-blurred through the backdrop while every tile was
+    // interpreted again. `OLD_TILES` still bounds how many older tiles are kept, nearest scale
+    // first, and admission keeps them inside `TILE_BYTES`.
     let missing: HashSet<usize> = wanted.iter().filter(|w| w.3.is_some()).map(|w| w.0).collect();
-    view.tiles.retain(|(p, t, _, _), _| p.abs_diff(cur) <= 2 && (*t == tag || missing.contains(p)));
+    let on_screen: HashSet<usize> = visible_now.iter().copied().collect();
+    view.tiles.retain(|(p, t, _, _), _| p.abs_diff(cur) <= 2 && (*t == tag || missing.contains(p) || on_screen.contains(p)));
     let mut older: Vec<(f32, (usize, u64, u32, u32))> =
         view.tiles.keys().filter(|k| k.1 != tag).map(|k| ((tag_scale(k.1) / scale).ln().abs(), *k)).collect();
-    if older.len() > OLD_TILES {
-        older.sort_by(|a, b| a.0.total_cmp(&b.0));
-        for (_, k) in older.drain(OLD_TILES..) {
-            view.tiles.remove(&k);
+    // Tiles of earlier zooms are kept nearest scale first, and bounded by count *and* bytes: a
+    // large scan renders a tile up to 4096² (about 67 MiB), so a count alone let a few tiles of
+    // a few zooms run far past the tile allowance. #2196 keeps a zoom already visited, which
+    // makes that reachable in ordinary zooming, so the byte bound has to be here too.
+    older.sort_by(|a, b| a.0.total_cmp(&b.0));
+    let mut kept = 0usize;
+    let mut kept_bytes = 0usize;
+    let mut drop: Vec<(usize, u64, u32, u32)> = Vec::new();
+    for (_dist, key) in older {
+        let bytes = view.tiles.get(&key).map_or(0, texture_bytes);
+        if kept >= OLD_TILES || kept_bytes.saturating_add(bytes) > TILE_BYTES {
+            drop.push(key);
+        } else {
+            kept = kept.saturating_add(1);
+            kept_bytes = kept_bytes.saturating_add(bytes);
         }
     }
+    for key in drop {
+        view.tiles.remove(&key);
+    }
     let mut queue: Vec<RenderRequest> = wanted
         .iter()
         .map(|&(page, wanted_scale, wanted_tag, tile)| RenderRequest {

crates/ui-egui/tests/view.rs

@@ -1149,17 +1149,15 @@ fn compare_page_with_raster(h: &mut Harness<'static, PdfCraftApp>, pdf: &[u8], z
     (sum as f64 / n as f64, worst)
 }
 
-#[test]
-fn zooming_a_tiled_page_shows_its_earlier_tiles_until_new_ones_arrive() {
-    let _gpu = gpu();
-    // A 1200 pt square page of fine lines (0.5 pt every 1.7 pt): tiled at 300 %.
+/// A 1200 pt square page of fine lines (0.5 pt every 1.7 pt): tiled at 300 %, whole-page at 150 %.
+fn fine_grid_pdf() -> Vec<u8> {
     let mut content = String::from("0 g\n");
     for i in 0..670 {
         let v = 30.0 + 1.7 * i as f32;
         content.push_str(&format!("{v} 30 0.5 1140 re 30 {v} 1140 0.5 re\n"));
     }
     content.push_str("f\n");
-    let pdf = format!(
+    format!(
         "%PDF-1.7
 1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj
@@ -1171,15 +1169,21 @@ trailer << /Root 1 0 R >>
 %%EOF",
         content.len()
     )
-    .into_bytes();
-    let mut h = Harness::builder().with_size(egui::vec2(1000.0, 800.0)).build_eframe(move |_cc| {
+    .into_bytes()
+}
+
+fn grid_harness(pdf: Vec<u8>, zoom: &str) -> Harness<'static, PdfCraftApp> {
+    Harness::builder().with_size(egui::vec2(1000.0, 800.0)).build_eframe(move |_cc| {
         let mut app = PdfCraftApp::new();
         app.open_bytes("grid.pdf", None, pdf.clone()).expect("opens");
         app.set_option("left", "closed").unwrap();
         app.set_option("panel", "none").unwrap();
-        app.set_option("zoom", "300").unwrap();
+        app.set_option("zoom", zoom).unwrap();
         app
-    });
+    })
+}
+
+fn settle(h: &mut Harness<'static, PdfCraftApp>) {
     for _ in 0..400 {
         h.run_steps(2);
         if !h.state().render_pending() {
@@ -1187,26 +1191,58 @@ trailer << /Root 1 0 R >>
         }
         std::thread::sleep(std::time::Duration::from_millis(5));
     }
-    // Detail in the middle of the window: the summed difference between neighbouring pixels.
-    let detail = |h: &mut Harness<'static, PdfCraftApp>| {
-        let img = h.render().expect("renders");
-        let (cx, cy) = (img.width() / 2, img.height() / 2);
-        let mut sum = 0u64;
-        for y in cy - 100..cy + 100 {
-            for x in cx - 150..cx + 150 {
-                sum += u64::from(img.get_pixel(x, y)[0].abs_diff(img.get_pixel(x + 1, y)[0]));
-            }
+}
+
+/// Detail in the middle of the window: the summed difference between neighbouring pixels.
+fn centre_detail(h: &mut Harness<'static, PdfCraftApp>) -> u64 {
+    let img = h.render().expect("renders");
+    let (cx, cy) = (img.width() / 2, img.height() / 2);
+    let mut sum = 0u64;
+    for y in cy - 100..cy + 100 {
+        for x in cx - 150..cx + 150 {
+            sum += u64::from(img.get_pixel(x, y)[0].abs_diff(img.get_pixel(x + 1, y)[0]));
         }
-        sum
-    };
-    let sharp = detail(&mut h);
+    }
+    sum
+}
+
+#[test]
+fn zooming_a_tiled_page_shows_its_earlier_tiles_until_new_ones_arrive() {
+    let _gpu = gpu();
+    let mut h = grid_harness(fine_grid_pdf(), "300");
+    settle(&mut h);
+    let sharp = centre_detail(&mut h);
     // The first frame at a new zoom has no tiles for it yet (they are requested at its end).
     h.state_mut().set_option("zoom", "310").unwrap();
     h.step();
-    let first = detail(&mut h);
+    let first = centre_detail(&mut h);
     assert!(first * 10 > sharp * 7, "the first frame after zooming lost its detail: {first} against {sharp}");
 }
 
+/// #2196: zooming away from a tiled page and back must not re-blur an area already rendered
+/// sharp. The tiles of the zoom just left used to be dropped while the page was shown whole,
+/// so returning to it was a cold miss and every tile cost a full page interpret again.
+#[test]
+fn returning_to_a_visited_zoom_reuses_its_cached_tiles() {
+    let _gpu = gpu();
+    let mut h = grid_harness(fine_grid_pdf(), "300");
+    settle(&mut h);
+    let before = h.state().views[0].tiles_debug(0);
+    assert!(!before.is_empty(), "the 300 % page should be tiled, but it cached no tiles");
+    // Zoom right out, where the 1200 pt page fits in one raster and is not tiled at all, so
+    // none of its tiles is demanded...
+    h.state_mut().set_option("zoom", "50").unwrap();
+    settle(&mut h);
+    // ...then back to the first zoom, which was already rendered sharp.
+    h.state_mut().set_option("zoom", "300").unwrap();
+    h.step();
+    let after = h.state().views[0].tiles_debug(0);
+    // Every tile rendered sharp before the round trip is still cached: zooming away must not
+    // drop them, so returning shows them at once instead of re-interpreting the page per tile.
+    let lost: Vec<_> = before.iter().copied().filter(|k| !after.contains(k)).collect();
+    assert!(lost.is_empty(), "returning to a visited zoom dropped its cached tiles {lost:?} (had {before:?}, now {after:?})");
+}
+
 #[test]
 fn sidebar_collapse_and_resize_preserves_scroll_position() {
     use pdfcraft_ui_egui::canvas::Fit;

parity/acrobat-features.toml

@@ -842,8 +842,8 @@ milestone = "M3"
 status = "partial"
 commands = []
 tools = []
-evidence = ["crates/render/src/raster.rs::tiles_match_full_render", "crates/render/src/raster.rs::effective_scale_never_exceeds_the_caps"]
-notes = "capped at 8192 px per side (M3.3)"
+evidence = ["crates/render/src/raster.rs::tiles_match_full_render", "crates/render/src/raster.rs::effective_scale_never_exceeds_the_caps", "crates/ui-egui/tests/view.rs::returning_to_a_visited_zoom_reuses_its_cached_tiles", "crates/ui-egui/tests/view.rs::zooming_a_tiled_page_shows_its_earlier_tiles_until_new_ones_arrive"]
+notes = "capped at 8192 px per side (M3.3); tiles of a zoom already rendered stay cached while their page is on screen, so zooming away and back shows the area sharp instead of re-rendering it tile by tile (#2196)"
 
 [[feature]]
 id = "view.layout-continuous"