diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index e8700d8137..d015040ce6 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/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 = 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 = 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 = wanted .iter() .map(|&(page, wanted_scale, wanted_tag, tile)| RenderRequest { diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 9fc490262d..65f4b31324 100644 --- a/crates/ui-egui/tests/view.rs +++ b/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 { 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, 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; diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 69fd3b0b95..f1dc35ff62 100644 --- a/parity/acrobat-features.toml +++ b/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"