Merge PR #3009: fix(ui): the header, footer and watermark previews show the page under the marks (#2837)

crates/ui-egui/src/marks_ui.rs

@@ -1,6 +1,7 @@
 //! Add Header and Footer, Add Watermark and Add Background dialogs (Acrobat: Edit a PDF ▸ …;
-//! dialog snapshots d13, d14, d12d). Each has the shared Page Range Options and a schematic
-//! preview of the current page. "Update" opens the same dialog in replace mode.
+//! dialog snapshots d13, d14, d12d). Each has the shared Page Range Options and a preview of a
+//! page the range selects, with the marks laid out on it. "Update" opens the same dialog in
+//! replace mode.
 
 use egui::{Align, Color32, CornerRadius, Layout, Rect, Stroke, pos2, vec2};
 use pdfcraft_engine::{Background, Edit, FontFamily, HeaderFooter, MarkKind, Watermark};
@@ -124,14 +125,30 @@ fn color_button(ui: &mut egui::Ui, c: &mut [f64; 3]) {
     }
 }
 
-/// A schematic of the current page with the marks laid out (no rendering needed).
-fn preview(ui: &mut egui::Ui, t: &Tokens, size: (f64, f64), draw: impl FnOnce(&egui::Painter, Rect, f32)) {
+/// The page (displayed size `size`) with the marks laid out on it. `picture` is its thumbnail,
+/// rendered off the UI thread like the pages panel's; until it arrives the page is a blank sheet.
+/// `tint` multiplies the page, which is how a background behind its content looks.
+fn preview(
+    ui: &mut egui::Ui,
+    t: &Tokens,
+    size: (f64, f64),
+    picture: Option<egui::TextureId>,
+    tint: Color32,
+    draw: impl FnOnce(&egui::Painter, Rect, f32),
+) {
     let (w, h) = (size.0.max(1.0), size.1.max(1.0));
     let s = (190.0 / w.max(h)) as f32;
     let (r, _) = ui.allocate_exact_size(vec2(210.0, 210.0), egui::Sense::hover());
     let page = Rect::from_center_size(r.center(), vec2(w as f32 * s, h as f32 * s));
     let painter = ui.painter_at(r);
-    painter.rect_filled(page, CornerRadius::ZERO, Color32::WHITE);
+    match picture {
+        Some(tex) => {
+            painter.image(tex, page, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), tint);
+        }
+        None => {
+            painter.rect_filled(page, CornerRadius::ZERO, tint);
+        }
+    }
     painter.rect_stroke(page, CornerRadius::ZERO, Stroke::new(1.0, t.border), egui::StrokeKind::Outside);
     draw(&painter, page, s);
 }
@@ -141,13 +158,26 @@ fn rgb32(c: [f64; 3]) -> Color32 {
     Color32::from_rgb(b(c[0]), b(c[1]), b(c[2]))
 }
 
+/// The tint of a page over a background of `color` at `opacity`: the colour shows where the page
+/// is white and the content stays on top. A non-finite opacity is opaque, as the engine has it.
+fn behind(color: [f64; 3], opacity: f64) -> Color32 {
+    let a = if opacity.is_finite() { opacity.clamp(0.0, 1.0) } else { 1.0 };
+    rgb32(color.map(|c| 1.0 - a * (1.0 - c)))
+}
+
+/// The page (0-based) the preview shows: the current one when the range selects it, otherwise the
+/// first page the range selects (the current one again when it selects none).
+fn preview_page(range: &PageRange, current: usize, count: usize) -> usize {
+    let pages = range.pages(count);
+    if pages.contains(&current) { current } else { pages.first().copied().unwrap_or(current) }
+}
+
 /// The dialog for `kind`; returns (apply, cancel).
 pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: MarkKind) -> (bool, bool) {
     let Some((i, id)) = app.active_ids() else { return (false, true) };
     let current = app.views[i].current;
     let Some(doc) = app.session.get(id) else { return (false, true) };
     let count = doc.info.pages.len();
-    let page = doc.info.pages.get(current).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0));
     let units = app.units;
     let d = &mut app.marks_draft;
     let mut browse = false;
@@ -313,13 +343,22 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M
     ui.label(egui::RichText::new(tl!("Page Range Options")).font(theme::semibold(12.5)));
     d.range.ui(ui, count);
     ui.add_space(8.0);
-    // Preview of the current page.
+    // The preview: the current page, or the first one the range selects if it leaves that out,
+    // drawn from its thumbnail at its displayed size (after /Rotate), the frame the engine lays
+    // the marks out in.
+    let target = preview_page(&d.range, 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 picture = app.views.get_mut(i).and_then(|view| {
+        view.need_thumbnail(target, true);
+        view.thumb_id(target)
+    });
+    let tint = if kind == MarkKind::Background && !d.use_file { behind(d.bg.color, d.bg.opacity) } else { Color32::WHITE };
     let hf = d.hf.clone();
     let size_h = page.1.max(1.0);
-    let (wm, bg, fit) = (d.wm.clone(), d.bg.clone(), d.fit);
+    let (wm, fit) = (d.wm.clone(), d.fit);
     let file_name = d.use_file.then(|| d.file.as_ref().map(|f| f.0.clone())).flatten();
     ui.horizontal(|ui| {
-        preview(ui, t, page, |p, r, s| match kind {
+        preview(ui, t, page, picture, tint, |p, r, s| match kind {
             MarkKind::HeaderFooter => {
                 if hf.shrink {
                     // The band the page content shrinks into, centred across.
@@ -382,15 +421,14 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M
                     Color32::GRAY,
                 );
             }
-            MarkKind::Background => {
-                p.rect_filled(r, CornerRadius::ZERO, rgb32(bg.color).gamma_multiply(bg.opacity as f32));
-            }
+            // A colour is the page's tint.
+            MarkKind::Background => {}
         });
         ui.vertical(|ui| {
             ui.label(
                 egui::RichText::new(crate::i18n::fmt(
                     tl!("Preview: page {p} of {n}"),
-                    &[("p", &(current + 1).to_string()), ("n", &count.to_string())],
+                    &[("p", &(target + 1).to_string()), ("n", &count.to_string())],
                 ))
                 .small()
                 .color(t.text_faint),
@@ -546,6 +584,27 @@ mod tests {
         assert_eq!(r.pages(38), [0]);
     }
 
+    #[test]
+    fn the_preview_shows_a_page_the_range_selects() {
+        let all = PageRange::default();
+        assert_eq!(preview_page(&all, 2, 5), 2, "the current page when the range selects it");
+        let even = PageRange { all: true, subset: Subset::Even, ..PageRange::default() };
+        assert_eq!(preview_page(&even, 0, 5), 1, "otherwise the first page the range selects");
+        let later = PageRange { all: false, from: 3, to: 4, subset: Subset::All };
+        assert_eq!(preview_page(&later, 0, 5), 2);
+        let none = PageRange { all: false, from: 4, to: 2, subset: Subset::All };
+        assert_eq!(preview_page(&none, 1, 5), 1, "the current page when the range selects none");
+        assert_eq!(preview_page(&all, 0, 0), 0, "no pages at all");
+    }
+
+    #[test]
+    fn a_background_tints_the_page_it_is_behind() {
+        assert_eq!(behind([1.0, 0.0, 0.0], 1.0), Color32::from_rgb(255, 0, 0));
+        assert_eq!(behind([1.0, 0.0, 0.0], 0.0), Color32::WHITE);
+        assert_eq!(behind([0.0, 0.0, 0.0], 0.5), Color32::from_rgb(128, 128, 128));
+        assert_eq!(behind([0.2, 0.4, 0.6], f64::NAN), behind([0.2, 0.4, 0.6], 1.0), "opaque, as the engine has it");
+    }
+
     #[test]
     fn the_watermark_edit_carries_the_offset() {
         let mut d = MarksDraft::default();

crates/ui-egui/tests/it/control.rs

@@ -1672,15 +1672,19 @@ fn control_open_replies_pending_while_the_worker_opens_and_focuses_once_it_has()
     assert_eq!(first["pending"], true, "{first}");
 
     // The worker completes the open on later frames; the tab shows, and only then does a
-    // repeated open focus it instead of reporting another pending open.
+    // repeated open focus it instead of reporting another pending open. The worker is a thread
+    // of its own, so the wait is bounded by time: a busy runner can draw 30 frames before it has
+    // read the file.
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
     let mut documents = 0;
-    for _ in 0..30 {
+    while std::time::Instant::now() < deadline {
         h.run_steps(1);
         let state = ok(&mut h, &c, "ui.state", json!({}));
         documents = state["documents"].as_array().map(|d| d.len()).unwrap_or_default();
         if documents == 1 {
             break;
         }
+        std::thread::sleep(std::time::Duration::from_millis(5));
     }
     assert_eq!(documents, 1, "the pending open completed into a tab");
 

crates/ui-egui/tests/it/forms.rs

@@ -500,8 +500,13 @@ fn wait_for_file(h: &mut Harness<'static, PdfCraftApp>, path: &std::path::Path)
     h.run_steps(1);
 }
 
+/// A fresh folder of its own for one test. The tests of this binary share one process id and run
+/// in parallel, and two of them name their folder "same-frame": a number taken per call keeps one
+/// test from clearing the folder another is saving into.
 fn scratch(name: &str) -> std::path::PathBuf {
-    let dir = std::env::temp_dir().join(format!("pdfcraft-forms-{name}-{}", std::process::id()));
+    static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
+    let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+    let dir = std::env::temp_dir().join(format!("pdfcraft-forms-{name}-{}-{n}", std::process::id()));
     let _ = std::fs::remove_dir_all(&dir);
     std::fs::create_dir_all(&dir).unwrap();
     dir

crates/ui-egui/tests/it/marks.rs

@@ -15,10 +15,14 @@ trailer << /Root 1 0 R >>
 %%EOF";
 
 fn harness() -> Harness<'static, PdfCraftApp> {
-    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| {
+    harness_with(FIXTURE.to_vec())
+}
+
+fn harness_with(pdf: Vec<u8>) -> Harness<'static, PdfCraftApp> {
+    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| {
         let mut app = PdfCraftApp::new();
         app.set_option("language", "en").unwrap();
-        app.open_bytes("doc.pdf", None, FIXTURE.to_vec()).unwrap();
+        app.open_bytes("doc.pdf", None, pdf.clone()).unwrap();
         app
     });
     h.run_steps(4);
@@ -139,6 +143,128 @@ fn backgrounds_and_watermarks_from_a_file() {
     assert_eq!(s.session.get(s.views[0].id).unwrap().can_undo(), Some("Add background"));
 }
 
+/// Two 300 × 400 pages with content (a dark bar), the second turned a quarter (/Rotate 90), so it
+/// is shown landscape, 400 × 300.
+fn content_fixture() -> Vec<u8> {
+    let bar = "0 0 0 rg 40 300 220 60 re f";
+    format!(
+        "%PDF-1.7
+1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj
+2 0 obj << /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /MediaBox [0 0 300 400] >> endobj
+3 0 obj << /Type /Page /Parent 2 0 R /Contents 5 0 R >> endobj
+4 0 obj << /Type /Page /Parent 2 0 R /Rotate 90 /Contents 5 0 R >> endobj
+5 0 obj << /Length {} >>
+stream
+{bar}
+endstream
+endobj
+trailer << /Root 1 0 R >>
+%%EOF",
+        bar.len()
+    )
+    .into_bytes()
+}
+
+/// The pictures of page `page` (its thumbnail) painted last frame: paint order, bounds, tint.
+fn page_pictures(h: &Harness<'static, PdfCraftApp>, page: usize) -> Vec<(usize, egui::Rect, egui::Color32)> {
+    let Some(tex) = h.state().views[0].thumb(page).map(|t| t.id()) else { return Vec::new() };
+    h.output()
+        .shapes
+        .iter()
+        .enumerate()
+        .filter_map(|(k, s)| match &s.shape {
+            egui::Shape::Mesh(m) if m.texture_id == tex => Some((k, m.calc_bounds(), m.vertices.first()?.color)),
+            _ => None,
+        })
+        .collect()
+}
+
+/// The preview's picture of `page`, once its thumbnail has arrived (it renders off the UI thread).
+fn wait_for_picture(h: &mut Harness<'static, PdfCraftApp>, page: usize) -> (usize, egui::Rect, egui::Color32) {
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
+    loop {
+        h.step();
+        if let [one] = page_pictures(h, page)[..] {
+            return one;
+        }
+        assert!(std::time::Instant::now() < deadline, "the preview never showed page {}'s picture", page + 1);
+        std::thread::sleep(std::time::Duration::from_millis(5));
+    }
+}
+
+/// A text painted on the page last frame (the dialog's text boxes show it too): its paint order
+/// and its laid-out rectangle.
+fn painted(h: &Harness<'static, PdfCraftApp>, text: &str, page: egui::Rect) -> (usize, egui::Rect) {
+    h.output()
+        .shapes
+        .iter()
+        .enumerate()
+        .find_map(|(k, s)| match &s.shape {
+            egui::Shape::Text(t) if t.galley.text() == text => {
+                Some((k, t.galley.rect.translate(t.pos.to_vec2()))).filter(|(_, r)| page.contains(r.center()))
+            }
+            _ => None,
+        })
+        .unwrap_or_else(|| panic!("{text:?} is painted on the page"))
+}
+
+fn near(a: f32, b: f32, what: &str) {
+    assert!((a - b).abs() < 1.0, "{what}: {a} against {b}");
+}
+
+#[test]
+fn the_preview_shows_the_page_the_range_selects_under_the_marks() {
+    use pdfcraft_ui_egui::marks::{PageRange, Subset};
+    let mut h = harness_with(content_fixture());
+    assert!(h.state_mut().execute("edit.header_footer"));
+    h.run_steps(2);
+    {
+        let d = &mut h.state_mut().marks_draft;
+        d.hf.text[1] = "TOP".into();
+        d.hf.text[5] = "END".into();
+        // Page 2 alone, not the current page: the preview follows the range (#2837).
+        d.range = PageRange { all: false, from: 2, to: 2, subset: Subset::All };
+    }
+    let (at, page, tint) = wait_for_picture(&mut h, 1);
+    h.get_by_label("Preview: page 2 of 2");
+    assert_eq!(tint, egui::Color32::WHITE);
+    assert!((page.width() / page.height() - 4.0 / 3.0).abs() < 0.01, "page 2 is shown turned, as displayed: {page:?}");
+    // The marks are drawn over the page, where the engine puts them on its displayed box
+    // (default margins: 36 pt top and bottom, 72 pt left and right).
+    let s = page.width() / 400.0;
+    let (top_at, top) = painted(&h, "TOP", page);
+    let (end_at, end) = painted(&h, "END", page);
+    assert!(at < top_at && at < end_at, "the page is under the marks");
+    near(top.center().x, page.center().x, "centre header, across");
+    near(top.center().y, page.top() + 36.0 * s, "centre header, down");
+    near(end.right(), page.right() - 72.0 * s, "right footer, across");
+    near(end.center().y, page.bottom() - 36.0 * s, "right footer, down");
+    h.get_by_label("Cancel").click();
+    h.run_steps(2);
+
+    // The watermark sits on the page's centre.
+    assert!(h.state_mut().execute("edit.watermark"));
+    h.run_steps(2);
+    {
+        let d = &mut h.state_mut().marks_draft;
+        d.wm.text = "DRAFT".into();
+        d.wm.rotation = 0.0;
+    }
+    let (at, page, _) = wait_for_picture(&mut h, 1);
+    let (mark_at, mark) = painted(&h, "DRAFT", page);
+    assert!(at < mark_at, "the page is under the watermark");
+    near(mark.center().x, page.center().x, "watermark, across");
+    near(mark.center().y, page.center().y, "watermark, down");
+    h.get_by_label("Cancel").click();
+    h.run_steps(2);
+
+    // A background colour shows where the page is white: it tints the page's picture.
+    assert!(h.state_mut().execute("edit.background"));
+    h.run_steps(2);
+    let (_, _, tint) = wait_for_picture(&mut h, 1);
+    assert_eq!(tint, egui::Color32::from_rgb(255, 255, 217), "the default pale yellow, opaque");
+}
+
 /// Pages from 1 to 1 stays one page: the dialog used to turn "to" back into the last page on
 /// every frame while both were 1, so only page 1 could not be chosen alone. The field starts on
 /// the last page.

parity/acrobat-features.toml

@@ -2400,8 +2400,8 @@ milestone = "M7"
 status = "shipped"
 commands = ["edit.header_footer"]
 tools = ["doc_header_footer"]
-evidence = ["crates/ui-egui/tests/it/marks.rs::header_and_footer_dialog_inserts_tokens_and_applies_to_a_subset", "crates/edit/src/tests.rs::header_and_footer_are_drawn_in_display_space_and_wrap_the_original_content", "crates/engine/src/tests.rs::headers_footers_watermarks_and_backgrounds_show_update_and_remove"]
-notes = "Standard faces only: Helvetica, Times or Courier (#3754; no bold, italic or embedded fonts yet); drawn in display space on rotated pages; artifact-tagged"
+evidence = ["crates/ui-egui/tests/it/marks.rs::header_and_footer_dialog_inserts_tokens_and_applies_to_a_subset", "crates/edit/src/tests.rs::header_and_footer_are_drawn_in_display_space_and_wrap_the_original_content", "crates/engine/src/tests.rs::headers_footers_watermarks_and_backgrounds_show_update_and_remove", "crates/ui-egui/tests/it/marks.rs::the_preview_shows_the_page_the_range_selects_under_the_marks"]
+notes = "Standard faces only: Helvetica, Times or Courier (#3754; no bold, italic or embedded fonts yet); drawn in display space on rotated pages; artifact-tagged; the dialog previews the text over the page's own thumbnail (the current page, or the first one the range selects), in the interface font and with tokens unexpanded"
 
 [[feature]]
 id = "edit.header-footer-update-remove"
@@ -2460,8 +2460,8 @@ milestone = "M7"
 status = "shipped"
 commands = ["edit.watermark"]
 tools = ["doc_watermark"]
-evidence = ["crates/edit/src/tests.rs::watermarks_rotate_fade_and_can_go_behind", "crates/ui-egui/tests/it/marks.rs::watermark_and_background_dialogs", "crates/automation/tests/it/automation.rs::headers_watermarks_and_backgrounds_through_tools"]
-notes = "Helvetica; fit-to-page or a size"
+evidence = ["crates/edit/src/tests.rs::watermarks_rotate_fade_and_can_go_behind", "crates/ui-egui/tests/it/marks.rs::watermark_and_background_dialogs", "crates/automation/tests/it/automation.rs::headers_watermarks_and_backgrounds_through_tools", "crates/ui-egui/tests/it/marks.rs::the_preview_shows_the_page_the_range_selects_under_the_marks"]
+notes = "Helvetica; fit-to-page or a size; the dialog previews it over the page's own thumbnail, always on top (a watermark behind the page is not drawn under its content there)"
 
 [[feature]]
 id = "edit.watermark-add-file"
@@ -2473,7 +2473,7 @@ status = "shipped"
 commands = ["edit.watermark"]
 tools = ["doc_watermark"]
 evidence = ["crates/engine/src/tests.rs::backgrounds_and_watermarks_from_files"]
-notes = "an image or a page of a PDF, scaled relative to the page, rotated, with opacity, behind or on top"
+notes = "an image or a page of a PDF, scaled relative to the page, rotated, with opacity, behind or on top; the dialog's preview shows its frame and file name over the page, not the picture itself"
 
 [[feature]]
 id = "edit.watermark-update-remove"
@@ -2508,8 +2508,8 @@ milestone = "M7"
 status = "shipped"
 commands = ["edit.background"]
 tools = ["doc_background"]
-evidence = ["crates/ui-egui/tests/it/marks.rs::watermark_and_background_dialogs", "crates/engine/src/tests.rs::headers_footers_watermarks_and_backgrounds_show_update_and_remove", "crates/engine/src/tests.rs::backgrounds_and_watermarks_from_files", "crates/ui-egui/tests/it/marks.rs::backgrounds_and_watermarks_from_a_file", "crates/organize/src/tests.rs::a_page_becomes_a_form_xobject_upright"]
-notes = "colour, or a picture from a file (an image or a page of a PDF) scaled relative to the page"
+evidence = ["crates/ui-egui/tests/it/marks.rs::watermark_and_background_dialogs", "crates/engine/src/tests.rs::headers_footers_watermarks_and_backgrounds_show_update_and_remove", "crates/engine/src/tests.rs::backgrounds_and_watermarks_from_files", "crates/ui-egui/tests/it/marks.rs::backgrounds_and_watermarks_from_a_file", "crates/organize/src/tests.rs::a_page_becomes_a_form_xobject_upright", "crates/ui-egui/tests/it/marks.rs::the_preview_shows_the_page_the_range_selects_under_the_marks"]
+notes = "colour, or a picture from a file (an image or a page of a PDF) scaled relative to the page; the dialog previews a colour as a tint of the page's own thumbnail (opaque pictures on the page are tinted there too, not in the result), a file as its frame and name"
 
 [[feature]]
 id = "edit.background-update-remove"