fix(ui): Update's preview parses its unmarked copy once and keeps a page's failure to itself

The page shown was drawn through a fresh renderer each time, which parsed
the whole unmarked document on the UI thread; UnmarkedPages now parses it
once and keeps it. An error from rendering a page ahead (the pool retiring
its parser, a cancelled request) no longer marks that page failed: only
drawing it when shown decides, and a page that has a picture keeps it. On
the web, where the pool renders on the UI thread, nothing renders ahead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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();

crates/engine/src/text_preview.rs

@@ -27,6 +27,12 @@ impl TextEditPreview {
 pub struct UnmarkedPages {
     bytes: Arc<Vec<u8>>,
     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<PageRenderer>,
+    /// 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<RenderedPage, String> {
-        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<RenderedPage, String> {
+        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,
+            }))
         })?
     }
 

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<usize, (u64, egui::TextureHandle)>,
-    /// 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<usize>,
     /// 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())
     }
 }