diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index b1abddfdca..dfdb522158 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -307,7 +307,7 @@ impl PdfCraftApp { /// `true` while any open document still waits for page renders (used by headless capture). pub fn render_pending(&self) -> bool { - self.views.iter().any(|v| v.render_pending()) || self.unmarked_page.pending() + self.views.iter().any(|v| v.render_pending()) } } diff --git a/crates/ui-egui/src/dialogs/marks_ui.rs b/crates/ui-egui/src/dialogs/marks_ui.rs index 2efc10ecce..d9a5b8815d 100644 --- a/crates/ui-egui/src/dialogs/marks_ui.rs +++ b/crates/ui-egui/src/dialogs/marks_ui.rs @@ -3,6 +3,8 @@ //! page the range selects, with the marks laid out on it. "Update" opens the same dialog in //! replace mode. +use std::collections::{HashMap, HashSet}; + use egui::{Align, Color32, CornerRadius, Layout, Rect, Stroke, pos2, vec2}; use pdfcraft_engine::{Background, Edit, FontFamily, HeaderFooter, MarkKind, Watermark}; @@ -186,31 +188,35 @@ fn behind(color: [f64; 3], opacity: f64) -> Color32 { /// In Update, the pages previewed without the marks the dialog replaces. Their thumbnails still /// have them, so the new marks would be drawn over the old ones, and a new background colour /// would tint the old one. The document without them is made once per dialog (it costs about a -/// save), and each page shown is rendered from it off the UI thread, like the thumbnails. +/// save). The page shown is drawn from it straight away, at the preview's small size, so neither +/// a blank sheet nor the old marks show while it renders; the pages a step goes to next render +/// ahead, off the UI thread, so stepping rarely waits. #[derive(Default)] pub(crate) struct UnmarkedPage { /// The document, its edit generation and the kind of mark taken away. key: Option<(pdfcraft_engine::DocId, u64, MarkKind)>, + pages: Option, renderer: Option, - /// The page and scale last asked for. - asked: Option<(usize, u64)>, - /// The page and scale the texture shows. - texture: Option<(usize, u64, egui::TextureHandle)>, + /// The pages rendered, with the scale (×1000) they were rendered at: the page shown and the + /// pages next to it. + textures: HashMap, + /// Pages that failed to render: their thumbnails show instead, the other pages still don't. + failed: HashSet, + /// The pages asked of the renderer ahead, and not back yet. Nothing waits for them: a step + /// is a frame, which takes what has come back first and draws the rest itself. + ahead: Vec<(usize, u64)>, } impl UnmarkedPage { - pub(crate) fn pending(&self) -> bool { - self.renderer.is_some() && self.asked.is_some() && self.texture.as_ref().map(|(p, t, _)| (*p, *t)) != self.asked - } - /// The picture to preview `page` on in place of its thumbnail, or `None` for the thumbnail: - /// not Update, no marks of `kind` in the document, the pages can't be prepared, or `page` is - /// still rendering (its thumbnail shows until then, rather than a blank sheet). + /// not Update, no marks of `kind` in the document, the pages can't be prepared, or `page` + /// fails to render. `near` are the pages a step goes to from it, rendered ahead. fn picture( &mut self, ctx: &egui::Context, (doc, id): (&pdfcraft_engine::Document, pdfcraft_engine::DocId), page: usize, + near: &[usize], kind: MarkKind, replace: bool, ) -> Option { @@ -218,34 +224,57 @@ impl UnmarkedPage { if self.key != key { *self = Self { key, ..Default::default() }; if replace { - self.renderer = doc.without_marks(kind).ok().flatten().map(|p| p.renderer()); + self.pages = doc.without_marks(kind).ok().flatten(); + self.renderer = self.pages.as_ref().map(|p| p.renderer()); } } - let renderer = self.renderer.as_ref()?; + let (pages, renderer) = (self.pages.as_ref()?, self.renderer.as_ref()?); // At the preview's size in pixels. - let p = doc.info.pages.get(page)?; - let scale = PREVIEW_SIDE * ctx.pixels_per_point() / p.width.max(p.height).max(1.0); - let asked = (page, (scale * 1000.0) as u64); - if self.asked != Some(asked) { - renderer.set_queue(vec![pdfcraft_render::RenderRequest { page, scale, tag: asked.1, ..Default::default() }]); - self.asked = Some(asked); - } - if let Some(out) = renderer.try_recv() { + let scale = |page: usize| { + let p = doc.info.pages.get(page)?; + let s = PREVIEW_SIDE * ctx.pixels_per_point() / p.width.max(p.height).max(1.0); + Some((s, (s * 1000.0) as u64)) + }; + let load = |ctx: &egui::Context, out: pdfcraft_render::RenderedPage| { + let image = crate::canvas::texture_image([out.width as usize, out.height as usize], out.rgba); + ctx.load_texture("marks-preview-unmarked", image, egui::TextureOptions::LINEAR) + }; + while let Some(out) = renderer.try_recv() { + let at = (out.request.page, out.request.tag); + self.ahead.retain(|a| *a != at); if out.error.is_some() { - // The thumbnails, as before, rather than no page at all. - self.renderer = None; - self.texture = None; - return None; - } - if (out.request.page, out.request.tag) == asked { - let image = crate::canvas::texture_image([out.width as usize, out.height as usize], out.rgba); - self.texture = Some((page, asked.1, ctx.load_texture("marks-preview-unmarked", image, egui::TextureOptions::LINEAR))); + self.failed.insert(at.0); + } else { + self.textures.insert(at.0, (at.1, load(ctx, out))); } } - if self.pending() { - ctx.request_repaint_after(std::time::Duration::from_millis(16)); + let (s, tag) = scale(page)?; + if !self.failed.contains(&page) && self.textures.get(&page).is_none_or(|(t, _)| *t != tag) { + match pages.render_page(page, s) { + Ok(out) => { + self.textures.insert(page, (tag, load(ctx, out))); + } + Err(_) => { + self.failed.insert(page); + } + } } - self.texture.as_ref().filter(|(p, ..)| *p == page).map(|(.., t)| t.id()) + // Ahead: the pages a step goes to, not rendered yet at their scale. + let wanted: Vec<(usize, u64)> = near + .iter() + .filter(|p| !self.failed.contains(p)) + .filter_map(|&p| scale(p).map(|(_, t)| (p, t))) + .filter(|(p, t)| self.textures.get(p).is_none_or(|(have, _)| have != t)) + .collect(); + if !wanted.is_empty() && wanted != self.ahead { + let requests = wanted + .iter() + .filter_map(|&(p, _)| scale(p).map(|(s, tag)| pdfcraft_render::RenderRequest { page: p, scale: s, tag, ..Default::default() })); + renderer.set_queue(requests.collect()); + self.ahead = wanted; + } + self.textures.retain(|p, _| *p == page || near.contains(p)); + self.textures.get(&page).filter(|_| !self.failed.contains(&page)).map(|(_, t)| t.id()) } } @@ -450,7 +479,8 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M // frame the engine lays the marks out in. Update draws it without the marks it replaces. let target = shown_preview(&d.range, &mut d.preview, current, count); let page = doc.info.pages.get(target).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0)); - let unmarked = app.unmarked_page.picture(ui.ctx(), (doc, id), target, kind, d.replace); + let near: Vec = [false, true].into_iter().filter_map(|forward| step_preview(&d.range, target, count, forward)).collect(); + let unmarked = app.unmarked_page.picture(ui.ctx(), (doc, id), target, &near, kind, d.replace); let picture = unmarked.or_else(|| { app.views.get_mut(i).and_then(|view| { view.need_thumbnail(target, true); diff --git a/crates/ui-egui/tests/it/marks.rs b/crates/ui-egui/tests/it/marks.rs index e0d429efa6..f66bf39864 100644 --- a/crates/ui-egui/tests/it/marks.rs +++ b/crates/ui-egui/tests/it/marks.rs @@ -386,8 +386,6 @@ fn update_previews_the_page_without_the_marks_it_replaces() { let mut app = PdfCraftApp::new(); app.set_option("language", "en").unwrap(); app.open_bytes("doc.pdf", None, pdf.clone()).unwrap(); - // The Pages panel keeps both thumbnails, for stepping to page 2 in Update below. - app.set_option("panel", "pages").unwrap(); app }); h.run_steps(4); @@ -427,30 +425,26 @@ fn update_previews_the_page_without_the_marks_it_replaces() { assert!(h.state_mut().execute("edit.background.update")); h.run_steps(2); h.state_mut().marks_draft.bg.color = [0.6, 0.8, 1.0]; - let white = |i: &egui::ColorImage| at(i, 0.5, 0.75) == egui::Color32::WHITE; - let (tint, page) = wait_for_preview(&mut h, &uploads, "the page without its background", white); + h.step(); + // Update shows the page without the marks from the frame it opens: neither a blank sheet nor + // the old marks while it renders. + let shown = |h: &Harness<'static, PdfCraftApp>| preview_picture(h, &uploads).expect("a page is previewed"); + let (tint, page) = shown(&h); assert_eq!(tint, egui::Color32::from_rgb(153, 204, 255), "the new colour tints the page"); assert_eq!(at(&page, 0.5, 0.75), egui::Color32::WHITE, "the page is previewed without the yellow background"); assert!(count(&page, red) > 200 && count(&page, blue) > 40 && count(&page, dark) > 1000, "the watermark, header and page content stay"); - // Stepping to page 2 shows its thumbnail until it renders without the background, not a - // blank sheet: the pages are prepared once, and each renders on its own. - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); - while h.state().views[0].thumb(1).is_none() { - assert!(std::time::Instant::now() < deadline, "page 2's thumbnail never loaded"); - h.step(); - std::thread::sleep(std::time::Duration::from_millis(5)); - } + // So does a step to page 2: the pages are prepared once, and each is drawn as it is shown. h.state_mut().marks_draft.preview = Some(1); h.step(); - assert!(preview_picture(&h, &uploads).is_some(), "a page shows while page 2 renders"); - wait_for_preview(&mut h, &uploads, "page 2 without its background", white); + assert_eq!(at(&shown(&h).1, 0.5, 0.75), egui::Color32::WHITE, "page 2 is previewed without its background at once"); h.get_by_label("Cancel").click(); h.run_steps(2); assert!(h.state_mut().execute("edit.watermark.update")); h.run_steps(2); h.state_mut().marks_draft.wm.text = "NEW".into(); - let (_, page) = wait_for_preview(&mut h, &uploads, "the page without its watermark", |i| count(i, red) == 0); + h.step(); + let (_, page) = shown(&h); assert_eq!(count(&page, red), 0, "the new watermark isn't previewed over the old one"); assert!(at(&page, 0.5, 0.75) == yellow && count(&page, blue) > 40, "the background and header stay"); h.get_by_label("Cancel").click(); @@ -458,7 +452,7 @@ fn update_previews_the_page_without_the_marks_it_replaces() { assert!(h.state_mut().execute("edit.header_footer.update")); h.run_steps(2); - let (_, page) = wait_for_preview(&mut h, &uploads, "the page without its header", |i| count(i, blue) == 0); + let (_, page) = shown(&h); assert_eq!(count(&page, blue), 0, "the new header isn't previewed over the old one"); assert!(at(&page, 0.5, 0.75) == yellow && count(&page, red) > 200, "the background and watermark stay"); }