Merge branch 'fix/background-hidden-by-page-backdrop' of https://github.com/alehel/pdfcraft into fix/background-hidden-by-page-backdrop

crates/automation/src/lib.rs

@@ -1182,9 +1182,10 @@ impl Automation {
         };
         let doc = self.doc(a)?;
         let found: Vec<Value> = pages
-            .into_iter()
-            .filter_map(|p| {
-                let kind = match doc.page_backdrop(p)? {
+            .iter()
+            .zip(doc.page_backdrops(&pages))
+            .filter_map(|(p, found)| {
+                let kind = match found? {
                     pdfcraft_engine::Backdrop::Fill => "fill",
                     pdfcraft_engine::Backdrop::Image => "image",
                 };

crates/edit/src/backdrop.rs

@@ -25,7 +25,8 @@ const SCAN_BYTES: usize = 64 * 1024;
 const SCAN_OPS: usize = 4096;
 /// How far (in points) a backdrop may fall short of the crop box and still count as covering it.
 const COVER_EPS: f64 = 1.0;
-/// Components at least this close to white (per channel) count as white.
+/// Components at least this close to white (per channel) count as white: a near-white fill
+/// (`0.99 g`) is a backdrop too, and replacing it changes the page by at most 2%.
 const WHITE: f64 = 0.98;
 
 /// What paints a page's backdrop.
@@ -57,15 +58,26 @@ struct State {
     opaque: bool,
 }
 
-/// The backdrop `page` (0-based) paints, if any.
+/// The backdrop `page` (0-based) paints, if any. Finding the page walks the page tree, so to
+/// check many pages use [`page_backdrops`], which walks it once.
 pub fn page_backdrop(doc: &Document, page: usize) -> Result<Option<Backdrop>, EditError> {
-    Ok(scan(doc, page)?.map(|(b, _)| b))
+    let all = pdfcraft_model::pages(doc);
+    let p = all.get(page).ok_or(EditError::NoSuchPage(page))?;
+    Ok(scan(doc, p).map(|(b, _)| b))
+}
+
+/// The backdrop each of `pages` (0-based) paints, in the same order, walking the page tree once.
+/// A page past the end has none, and doesn't hide what the others paint: a page count taken
+/// elsewhere (the viewer's) can disagree with the page tree's on a malformed document.
+pub fn page_backdrops(doc: &Document, pages: &[usize]) -> Vec<Option<Backdrop>> {
+    let all = pdfcraft_model::pages(doc);
+    pages.iter().map(|&p| all.get(p).and_then(|page| scan(doc, page)).map(|(b, _)| b)).collect()
 }
 
 /// Replace `page`'s white backdrop fill, if it has one (see the module doc). Returns whether it
-/// did. A picture backdrop is left alone.
-pub(crate) fn replace(doc: &mut Document, page: usize) -> Result<bool, EditError> {
-    let Some((Backdrop::Fill, Some(found))) = scan(doc, page)? else { return Ok(false) };
+/// did; when it did, `page` no longer matches the document. A picture backdrop is left alone.
+pub(crate) fn replace(doc: &mut Document, page: &pdfcraft_model::Page) -> Result<bool, EditError> {
+    let Some((Backdrop::Fill, Some(found))) = scan(doc, page) else { return Ok(false) };
     let edited = found.pieces.splice([(found.op.clone(), b"n".to_vec())]);
     let piece = found.pieces.piece_of(found.op.start);
     let (Some(&(slot, original)), Some(bytes)) = (found.refs.get(piece), edited.get(piece)) else { return Ok(false) };
@@ -80,11 +92,10 @@ pub(crate) fn replace(doc: &mut Document, page: usize) -> Result<bool, EditError
     let mut dict = Dict::new();
     dict.set(b"PCBackdrop".to_vec(), Object::Dict(note));
     let copy = doc.add(Object::Stream(Stream::flate(dict, bytes)));
-    let p = page_of(doc, page)?;
-    let mut list = crate::contents(doc, &p)?;
+    let mut list = crate::contents(doc, page)?;
     let Some(entry) = list.get_mut(slot) else { return Ok(false) };
     *entry = Object::Ref(copy);
-    doc.update_dict(p.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?;
+    doc.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?;
     Ok(true)
 }
 
@@ -124,20 +135,13 @@ fn unchanged_copy_of(doc: &Document, entry: &Object) -> Option<ObjRef> {
     (copy.decoded().ok()? == expect).then_some(*original)
 }
 
-fn page_of(doc: &Document, page: usize) -> Result<pdfcraft_model::Page, EditError> {
-    pdfcraft_model::pages(doc).into_iter().nth(page).ok_or(EditError::NoSuchPage(page))
-}
-
 /// Find the backdrop, and for a fill, where it is.
-fn scan(doc: &Document, page: usize) -> Result<Option<(Backdrop, Option<Found>)>, EditError> {
-    let p = page_of(doc, page)?;
+fn scan(doc: &Document, p: &pdfcraft_model::Page) -> Option<(Backdrop, Option<Found>)> {
     let crop = p.crop(doc);
-    let list: Vec<Object> = match p.dict.get(b"Contents") {
-        None => return Ok(None),
-        Some(c) => match &*doc.resolve(c) {
-            Object::Array(a) => a.clone(),
-            _ => vec![c.clone()],
-        },
+    let c = p.dict.get(b"Contents")?;
+    let list: Vec<Object> = match &*doc.resolve(c) {
+        Object::Array(a) => a.clone(),
+        _ => vec![c.clone()],
     };
     // The page's own content: untagged streams (marks and the q/Q wrapper carry /PCMark). Read
     // only as far as a backdrop could be.
@@ -148,7 +152,7 @@ fn scan(doc: &Document, page: usize) -> Result<Option<(Backdrop, Option<Found>)>
         if total >= SCAN_BYTES {
             break;
         }
-        let Object::Ref(r) = o else { return Ok(None) };
+        let Object::Ref(r) = o else { return None };
         let obj = doc.get(*r);
         let Object::Stream(s) = &*obj else { continue };
         if s.dict.get(b"PCMark").is_some() {
@@ -164,10 +168,10 @@ fn scan(doc: &Document, page: usize) -> Result<Option<(Backdrop, Option<Found>)>
     let parsed = pdfcraft_content::parse(head);
     let res = resources(doc, &p.dict);
     let found = walk(doc, &res, crop, &parsed.ops);
-    Ok(found.map(|(b, op)| match (b, op) {
+    found.map(|(b, op)| match (b, op) {
         (Backdrop::Fill, Some(op)) => (b, Some(Found { refs, pieces, op })),
         _ => (b, None),
-    }))
+    })
 }
 
 fn resources(doc: &Document, page: &Dict) -> Dict {

crates/edit/src/backdrop_tests.rs

@@ -231,3 +231,71 @@ fn pages_without_content_or_with_odd_contents_have_no_backdrop() {
     assert_eq!(page_backdrop(&doc, 1).unwrap(), None);
     assert!(matches!(page_backdrop(&doc, 7), Err(EditError::NoSuchPage(7))));
 }
+
+/// `pages` 300 × 200 pages in a flat tree; even pages paint a white backdrop, odd ones don't.
+fn many_pages(pages: usize) -> Document {
+    let kids: Vec<String> = (0..pages).map(|i| format!("{} 0 R", i + 5)).collect();
+    let mut objs = vec![
+        "<< /Type /Catalog /Pages 2 0 R >>".to_string(),
+        format!("<< /Type /Pages /Kids [{}] /Count {pages} /MediaBox [0 0 300 200] >>", kids.join(" ")),
+        stream("", &format!("1 g 0 0 300 200 re f 0 g {TEXT}")),
+        stream("", TEXT),
+    ];
+    objs.extend((0..pages).map(|i| format!("<< /Type /Page /Parent 2 0 R /Contents {} 0 R >>", 3 + i % 2)));
+    build(&objs)
+}
+
+#[test]
+fn checking_many_pages_walks_the_page_tree_once() {
+    // The Add Background dialog checks every page of a long document. Looking each page up on
+    // its own walked the whole tree per page, so checking them all grew with the square of the
+    // page count (minutes at 20,000 pages); one walk keeps it in proportion.
+    const PAGES: usize = 4000;
+    let doc = many_pages(PAGES);
+    let all: Vec<usize> = (0..PAGES).collect();
+    let start = std::time::Instant::now();
+    let found = page_backdrops(&doc, &all);
+    let took = start.elapsed();
+    assert_eq!(found.len(), PAGES);
+    for (i, f) in found.iter().enumerate() {
+        assert_eq!(*f, (i % 2 == 0).then_some(Backdrop::Fill), "page {i}");
+    }
+    assert!(took < std::time::Duration::from_secs(5), "checking {PAGES} pages took {took:?}");
+}
+
+#[test]
+fn a_batch_answers_in_the_order_asked_and_a_missing_page_hides_nothing() {
+    let doc = many_pages(5);
+    assert_eq!(page_backdrops(&doc, &[3, 0, 0]), [None, Some(Backdrop::Fill), Some(Backdrop::Fill)]);
+    assert_eq!(page_backdrops(&doc, &[]), []);
+    // The viewer can count more pages than the page tree holds: the others still answer.
+    assert_eq!(page_backdrops(&doc, &[2, 5, usize::MAX]), [Some(Backdrop::Fill), None, None]);
+    assert_eq!(page_backdrop(&doc, 4).unwrap(), Some(Backdrop::Fill));
+    assert!(matches!(page_backdrop(&doc, 5), Err(EditError::NoSuchPage(5))));
+}
+
+#[test]
+fn a_background_over_many_pages_replaces_each_backdrop_and_removing_restores_them() {
+    // Pages share their content streams, so each replaced page gets its own copy and the rest
+    // are checked against the document as it is after the earlier ones.
+    let mut doc = many_pages(4);
+    let all = [0, 1, 2, 3];
+    let before: Vec<Object> = pdfcraft_model::pages(&doc).iter().filter_map(|p| p.dict.get(b"Contents").cloned()).collect();
+    add_background(&mut doc, &all, &red(), true).unwrap();
+    assert_eq!(page_backdrops(&doc, &all), [None; 4]);
+    // Update: the backdrops are gone already, so nothing is copied again.
+    add_background(&mut doc, &all, &red(), true).unwrap();
+    assert_eq!(page_backdrops(&doc, &all), [None; 4]);
+    remove_marks(&mut doc, &all, MarkKind::Background).unwrap();
+    let after: Vec<Object> = pdfcraft_model::pages(&doc).iter().filter_map(|p| p.dict.get(b"Contents").cloned()).collect();
+    let refs = |list: &[Object]| -> Vec<Vec<Object>> {
+        list.iter()
+            .map(|c| match &*doc.resolve(c) {
+                Object::Array(a) => a.clone(),
+                _ => vec![c.clone()],
+            })
+            .collect()
+    };
+    assert_eq!(refs(&after), refs(&before), "every page points at its original stream again");
+    assert_eq!(page_backdrops(&doc, &all), [Some(Backdrop::Fill), None, Some(Backdrop::Fill), None]);
+}

crates/edit/src/lib.rs

@@ -599,10 +599,10 @@ pub fn add_background(doc: &mut Document, pages: &[usize], bg: &Background, repl
     }
     let opacity = if bg.opacity.is_finite() { bg.opacity.clamp(0.0, 1.0) } else { 1.0 };
     for &i in pages {
-        if bg.replace_backdrop {
-            backdrop::replace(doc, i)?;
+        let mut page = page_list(doc)[i].clone();
+        if bg.replace_backdrop && backdrop::replace(doc, &page)? {
+            page = page_list(doc)[i].clone();
         }
-        let page = page_list(doc)[i].clone();
         let (w, h) = page.display_size(doc);
         let mut content = begin(MarkKind::Background, "Background", page.view_matrix(doc));
         match &bg.source {
@@ -659,7 +659,7 @@ pub fn marks_present(doc: &Document) -> Vec<MarkKind> {
 }
 
 pub mod backdrop;
-pub use backdrop::{Backdrop, page_backdrop};
+pub use backdrop::{Backdrop, page_backdrop, page_backdrops};
 pub mod objects;
 pub use objects::{EditableObject, MAX_MOVE_OBJECTS, ObjectKind, ObjectTarget, editable_objects, move_objects};
 

crates/engine/src/lib.rs

@@ -504,6 +504,15 @@ impl Document {
         self.editor.as_ref().and_then(|e| pdfcraft_edit::page_backdrop(&e.cos, page).ok().flatten())
     }
 
+    /// [`Self::page_backdrop`] for each of `pages`, in order, walking the page tree once: the
+    /// way to check many pages. A page out of range has none.
+    pub fn page_backdrops(&self, pages: &[usize]) -> Vec<Option<pdfcraft_edit::Backdrop>> {
+        match &self.editor {
+            Some(e) => pdfcraft_edit::page_backdrops(&e.cos, pages),
+            None => vec![None; pages.len()],
+        }
+    }
+
     /// Edit a PDF: the images `page` draws (0-based).
     pub fn page_images(&self, page: usize) -> Vec<pdfcraft_edit::PageImage> {
         self.editor.as_ref().and_then(|e| pdfcraft_edit::page_images(&e.cos, page).ok()).unwrap_or_default()

crates/ui-egui/src/marks_ui.rs

@@ -105,14 +105,26 @@ impl BackdropCheck {
             self.key = Some(key);
             self.pages = vec![None; count];
         }
-        let mut budget = BACKDROP_PAGES_PER_FRAME;
+        // One batch a frame: finding a page walks the page tree, so the batch shares one walk.
+        let mut batch = Vec::with_capacity(BACKDROP_PAGES_PER_FRAME);
+        for &p in pages {
+            if batch.len() >= BACKDROP_PAGES_PER_FRAME {
+                break;
+            }
+            if matches!(self.pages.get(p), Some(None)) && !batch.contains(&p) {
+                batch.push(p);
+            }
+        }
+        if !batch.is_empty() {
+            for (p, found) in batch.iter().zip(doc.page_backdrops(&batch)) {
+                if let Some(slot) = self.pages.get_mut(*p) {
+                    *slot = Some(found);
+                }
+            }
+        }
         let mut n = BackdropCount::default();
         for &p in pages {
-            let Some(slot) = self.pages.get_mut(p) else { continue };
-            if slot.is_none() && budget > 0 {
-                *slot = Some(doc.page_backdrop(p));
-                budget -= 1;
-            }
+            let Some(slot) = self.pages.get(p) else { continue };
             match slot {
                 None => n.pending += 1,
                 Some(Some(Backdrop::Fill)) => n.fills += 1,