diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c58ba90129..b21f3d335c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3023,7 +3023,7 @@ fn update_previews_on_the_page_without_the_marks_it_replaces() { let dark = |p: &pdfcraft_render::RenderedPage| p.rgba.as_chunks::<4>().0.iter().filter(|[r, g, b, _]| *r < 100 && *g < 100 && *b < 100).count(); assert_eq!(pixel(&s, id, 0, 5, 5)[..3], [255, 255, 0], "the page has the yellow background"); - let unmarked = doc.without_marks(MarkKind::Background).unwrap().expect("the pages have a background"); + let mut unmarked = doc.without_marks(MarkKind::Background).unwrap().expect("the pages have a background"); let page = unmarked.render_page(0, 1.0).unwrap(); assert_eq!(px(&page, 5, 5), [255, 255, 255], "without the background the page is white"); assert!(reds(&page) > 50, "the watermark stays"); @@ -3034,6 +3034,7 @@ fn update_previews_on_the_page_without_the_marks_it_replaces() { assert_eq!(px(&page, 5, 5), [255, 255, 0], "the background stays"); assert_eq!(px(&unmarked.render_page(1, 1.0).unwrap(), 5, 5), [255, 255, 255], "made once, for every page: page 2 is white too"); + assert_eq!(unmarked.parser_builds(), 1, "parsed once for both pages"); assert!(doc.without_marks(MarkKind::HeaderFooter).unwrap().is_none(), "the document has no header"); assert!(unmarked.render_page(9, 1.0).is_err(), "no page 10"); let doc = s.get(id).unwrap(); diff --git a/crates/engine/src/text_preview.rs b/crates/engine/src/text_preview.rs index 5f39945a14..f2e60dac07 100644 --- a/crates/engine/src/text_preview.rs +++ b/crates/engine/src/text_preview.rs @@ -27,6 +27,12 @@ impl TextEditPreview { pub struct UnmarkedPages { bytes: Arc>, config: RenderConfig, + /// Parsed on the first [`Self::render_page`], once (a copy that can't be parsed isn't tried + /// again), and kept: parsing costs about as much as reading the document. + parsed: Option, + /// How many times it was parsed. + #[cfg(test)] + builds: u64, } impl UnmarkedPages { @@ -34,8 +40,38 @@ impl UnmarkedPages { RenderPool::new(self.bytes.clone(), 1, self.config.clone()) } - pub fn render_page(&self, page: usize, scale: f32) -> Result { - render_one(&self.bytes, &self.config, page, scale) + /// `page` at `scale`, on this thread. + pub fn render_page(&mut self, page: usize, scale: f32) -> Result { + if !scale.is_finite() || scale <= 0.0 { + return Err("preview scale must be finite and positive".into()); + } + if self.parsed.is_none() { + self.parsed = Some(PageRenderer::new(self.bytes.clone(), self.config.clone())); + #[cfg(test)] + { + self.builds += 1; + } + } + let Some(renderer) = self.parsed.as_mut() else { return Err("the document could not be parsed".into()) }; + let out = guard(|| renderer.render(RenderRequest { page, scale, ..Default::default() })); + let out = match out { + Ok(out) => out, + Err(e) => { + // A panic may have left it half-way: parse afresh next time. + self.parsed = None; + return Err(e); + } + }; + if let Some(e) = &out.error { + return Err(e.clone()); + } + Ok(out) + } + + /// How many times [`Self::render_page`] has parsed the document. + #[cfg(test)] + pub(crate) fn parser_builds(&self) -> u64 { + self.builds } } @@ -129,7 +165,13 @@ impl Document { return Ok(None); } let bytes = Arc::new(pdfcraft_cos::write_incremental(&cos, &pdfcraft_cos::SaveOptions::default()).map_err(|e| e.to_string())?); - Ok(Some(UnmarkedPages { bytes, config: self.config.clone() })) + Ok(Some(UnmarkedPages { + bytes, + config: self.config.clone(), + parsed: None, + #[cfg(test)] + builds: 0, + })) })? } diff --git a/crates/ui-egui/src/dialogs/marks_ui.rs b/crates/ui-egui/src/dialogs/marks_ui.rs index d9a5b8815d..a3162d0851 100644 --- a/crates/ui-egui/src/dialogs/marks_ui.rs +++ b/crates/ui-egui/src/dialogs/marks_ui.rs @@ -190,7 +190,8 @@ fn behind(color: [f64; 3], opacity: f64) -> Color32 { /// would tint the old one. The document without them is made once per dialog (it costs about a /// 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. +/// ahead, off the UI thread (not on the web, where the pool renders on it), so stepping rarely +/// waits. #[derive(Default)] pub(crate) struct UnmarkedPage { /// The document, its edit generation and the kind of mark taken away. @@ -200,7 +201,8 @@ pub(crate) struct UnmarkedPage { /// 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. + /// Pages that failed to render when shown, so they aren't tried every frame: 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. @@ -228,7 +230,10 @@ impl UnmarkedPage { self.renderer = self.pages.as_ref().map(|p| p.renderer()); } } - let (pages, renderer) = (self.pages.as_ref()?, self.renderer.as_ref()?); + // On the web the render pool renders inside `try_recv`, on this thread: rendering ahead + // there would draw the pages next to it in the same frame too. + let near = if cfg!(target_arch = "wasm32") { &[][..] } else { near }; + let renderer = self.renderer.as_ref()?; // At the preview's size in pixels. let scale = |page: usize| { let p = doc.info.pages.get(page)?; @@ -242,14 +247,17 @@ impl UnmarkedPage { 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() { - self.failed.insert(at.0); - } else { + // An error rendering ahead needn't be the page's (the pool retires its parser, a + // request is cancelled): the page is drawn here when it is shown, and that decides. + if out.error.is_none() { self.textures.insert(at.0, (at.1, load(ctx, out))); } } let (s, tag) = scale(page)?; - if !self.failed.contains(&page) && self.textures.get(&page).is_none_or(|(t, _)| *t != tag) { + if !self.failed.contains(&page) + && self.textures.get(&page).is_none_or(|(t, _)| *t != tag) + && let Some(pages) = self.pages.as_mut() + { match pages.render_page(page, s) { Ok(out) => { self.textures.insert(page, (tag, load(ctx, out))); @@ -274,7 +282,8 @@ impl UnmarkedPage { 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()) + // A picture rendered at another scale beats the thumbnail with the old marks. + self.textures.get(&page).map(|(_, t)| t.id()) } }