diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index b570c0e79..2abfdce55 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -1094,6 +1094,30 @@ pub fn set_appearance(doc: &mut Document, r: ObjRef) -> Result<(), AnnotError> { Ok(()) } +/// A copy of `doc` in which every annotation without a normal appearance (`/AP /N`) has the one +/// [`appearance::build`] draws from its dictionary, for displaying the document (`None` when no +/// annotation needs one). Many files carry comments without appearances (FreeText, Ink, notes, +/// stamps, …) that viewers draw from the dictionary. `doc` is not changed: saving writes the file +/// as it was. Links, form fields and pop-ups keep their own handling. +pub fn with_missing_appearances(doc: &Document) -> Option { + let mut copy: Option = None; + for page in page_refs(doc).ok()? { + for r in annots(doc, page).iter().filter_map(Object::as_ref) { + let d = annot_dict(doc, r); + if matches!(d.name(b"Subtype"), None | Some(b"Link" | b"Widget" | b"Popup")) { + continue; + } + let has_normal = d.get(b"AP").is_some_and(|ap| doc.resolve(ap).as_dict().is_some_and(|ap| ap.contains(b"N"))); + if has_normal || appearance::build(&d).is_none() { + continue; + } + // Fails only when `r` isn't a dictionary, which `build` above has just ruled out. + set_appearance(copy.get_or_insert_with(|| doc.clone()), r).ok(); + } + } + copy +} + /// Format a number for content streams and DA strings. pub(crate) fn n(v: f64) -> String { let s = format!("{:.3}", if v.abs() < 5e-4 { 0.0 } else { v }); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 555419885..3664e7cab 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -679,3 +679,59 @@ fn the_eraser_cuts_strokes_and_removes_empty_drawings() { erase_ink(&mut doc, 0, i, &all[21..], 8.0, &meta("e")).unwrap(); assert!(!list(&doc, 0).iter().any(|d| d.name(b"Subtype") == Some(b"Ink"))); } + +/// A one-page document from object bodies (object 1 is the catalog, 3 the page). +fn document_of(objs: &[&[u8]]) -> Document { + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + Document::open(Arc::new(out)).expect("opens") +} + +#[test] +fn comments_without_appearances_get_one_for_display_only() { + let doc = document_of(&[ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", + b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R 7 0 R 8 0 R 9 0 R 10 0 R] >>", + b"<< /Type /Annot /Subtype /FreeText /Rect [48 582 285 632] /Contents (FreeText annotation with valid /DA string.) /DA (/Helv 10 Tf 0 0 1 rg) /Border [0 0 1] /C [.85 .47 .02] >>", + b"<< /Type /Annot /Subtype /Ink /Rect [70 392 275 472] /InkList [[80 402 120 452 160 412 210 462 260 417]] /Border [0 0 2] /C [.49 .18 .07] >>", + b"<< /Type /Annot /Subtype /Text /Rect [400 717 420 737] /C [1 1 0] /Contents (A note) >>", + b"<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /C [1 0 0] /AP << /N 11 0 R >> >>", + b"<< /Type /Annot /Subtype /Link /Rect [48 717 308 741] /Border [0 0 1] /C [0 0 1] >>", + b"<< /Type /Annot /Subtype /Widget /FT /Tx /T (f) /Rect [10 100 100 120] >>", + b"<< /Type /Annot /Subtype /Sound /Rect [10 200 30 220] >>", + b"<< /Type /XObject /Subtype /Form /BBox [10 10 50 50] /Length 0 >>\nstream\n\nendstream", + ]); + let display = with_missing_appearances(&doc).expect("three comments need an appearance"); + let has_ap = |doc: &Document| list(doc, 0).iter().map(|d| d.get(b"AP").is_some()).collect::>(); + assert_eq!(has_ap(&doc), [false, false, false, true, false, false, false], "the document itself is untouched"); + assert_eq!(has_ap(&display), [true, true, true, true, false, false, false], "FreeText, Ink and Text get one; the rest don't"); + let shown = list(&display, 0); + let free_text = ap_content(&display, &shown[0]); + assert!(free_text.contains("re f") && free_text.contains("Tj"), "background box and text: {free_text}"); + assert_eq!(shown[3].get(b"AP"), list(&doc, 0)[3].get(b"AP"), "an existing appearance is kept"); + // The display copy survives a save and reopen (what the renderer reads). + assert_eq!(has_ap(&reopen(&display)), [true, true, true, true, false, false, false]); + // Nothing to add: no copy. + let complete = document_of(&[ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", + b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R] >>", + b"<< /Type /Annot /Subtype /Link /Rect [48 717 308 741] >>", + b"<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /C [1 0 0] /AP << /N 6 0 R >> >>", + b"<< /Type /XObject /Subtype /Form /BBox [10 10 50 50] /Length 0 >>\nstream\n\nendstream", + ]); + assert!(with_missing_appearances(&complete).is_none()); +} diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 9ddff4bf1..bda8922c5 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -1343,10 +1343,10 @@ impl Automation { let doc = self.session.get(id).ok_or_else(|| failed("no such document"))?; let fresh = || { let config = RenderConfig { password: doc.password.as_deref().map(Arc::from), ..Default::default() }; - (doc.bytes.clone(), PageRenderer::new(doc.bytes.clone(), config)) + (doc.display.clone(), PageRenderer::new(doc.display.clone(), config)) }; let entry = self.renderers.entry(id).or_insert_with(fresh); - if !Arc::ptr_eq(&entry.0, &doc.bytes) { + if !Arc::ptr_eq(&entry.0, &doc.display) { *entry = fresh(); } Ok(&mut entry.1) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index ca90c0099..5360531ad 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -25,7 +25,7 @@ pub struct ExportSource { impl Document { /// The current state, for exporting on another thread. pub fn export_source(&self) -> ExportSource { - ExportSource { bytes: self.bytes.clone(), config: self.config.clone(), pages: self.info.pages.len() } + ExportSource { bytes: self.display.clone(), config: self.config.clone(), pages: self.info.pages.len() } } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 68de16b03..2dca9559f 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -194,8 +194,11 @@ pub struct Document { pub id: DocId, pub name: String, pub path: Option, - /// The working file: what Save writes and what is displayed. + /// The working file: what Save writes. pub bytes: Arc>, + /// What is displayed (the page view, `page_render`, page images): `bytes`, plus appearances + /// drawn for display only for comments that have none ([`display_bytes`]). Never saved. + pub display: Arc>, pub info: DocInfo, pub renderer: RenderPool, /// The password the document was opened with (needed to read attachments, etc.). @@ -474,6 +477,15 @@ impl Document { } } +/// What is displayed: the working file, plus (in memory only, never saved) the appearances +/// PdfCraft draws for comments that have none ([`pdfcraft_annot::with_missing_appearances`]). +/// The working file is unchanged, and so is everything that reads it (saving, signatures, …). +fn display_bytes(editor: Option<&Editor>, bytes: &Arc>) -> Arc> { + let Some(display) = editor.and_then(|e| pdfcraft_annot::with_missing_appearances(&e.cos)) else { return bytes.clone() }; + // Writing fails only as saving the document would; the page then shows as it always did. + write_incremental(&display, &SaveOptions::default()).map(Arc::new).unwrap_or_else(|_| bytes.clone()) +} + fn render_threads() -> usize { std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(2, 8) - 1 } @@ -482,7 +494,7 @@ fn render_threads() -> usize { fn use_layer_choices(doc: &mut Document) { let overrides: Vec<(i32, i32, bool)> = doc.info.layers.iter().map(|l| (l.id.0 as i32, l.id.1 as i32, l.visible)).collect(); doc.config.layers = Arc::new(overrides); - doc.renderer = RenderPool::new(doc.bytes.clone(), render_threads(), doc.config.clone()); + doc.renderer = RenderPool::new(doc.display.clone(), render_threads(), doc.config.clone()); } /// Apply a set-layer-visibility action to `layers`, one change at a time, so a toggle flips the @@ -2171,7 +2183,6 @@ impl Session { xfa: Option, ) -> Result { let config = RenderConfig { password: render_password.as_deref().map(Arc::from), ..Default::default() }; - let renderer = RenderPool::new(bytes.clone(), render_threads(), config.clone()); let (editor, read_only_reason) = match cos { Ok(Ok(cos)) => { let keys = Keys { render: render_password.clone(), reopen: password.map(str::to_owned) }; @@ -2180,6 +2191,8 @@ impl Session { Ok(Err(e)) => (None, Some(e.to_string())), Err(_) => (None, Some("the document structure could not be read for editing".into())), }; + let display = display_bytes(editor.as_ref(), &bytes); + let renderer = RenderPool::new(display.clone(), render_threads(), config.clone()); let mut form = editor.as_ref().map(|e| pdfcraft_forms::fields(&e.cos)).unwrap_or_default(); if let Some(e) = editor.as_ref() { xfa::mark_script_buttons(&e.cos, &mut form); @@ -2196,6 +2209,7 @@ impl Session { name, path, bytes, + display, info, renderer, password: render_password, @@ -2414,8 +2428,9 @@ impl Session { if !doc.signatures.is_empty() { doc.signatures = signatures_of(&editor.cos, &bytes, &doc.trust, &doc.sig_cache); } - doc.bytes = bytes.clone(); - doc.renderer = RenderPool::new(bytes, render_threads(), doc.config.clone()); + doc.display = display_bytes(Some(editor), &bytes); + doc.renderer = RenderPool::new(doc.display.clone(), render_threads(), doc.config.clone()); + doc.bytes = bytes; Ok(()) } @@ -2450,8 +2465,9 @@ impl Session { doc.added = pdfcraft_edit::list_added(&editor.cos); doc.links = pdfcraft_annot::links::list(&editor.cos); doc.signatures = signatures_of(&editor.cos, &bytes, &doc.trust, &doc.sig_cache); - doc.bytes = bytes.clone(); - doc.renderer = RenderPool::new(bytes, render_threads(), doc.config.clone()); + doc.display = display_bytes(Some(editor), &bytes); + doc.renderer = RenderPool::new(doc.display.clone(), render_threads(), doc.config.clone()); + doc.bytes = bytes; Ok(()) } @@ -2839,7 +2855,7 @@ impl Session { return false; } doc.config.hide_comments = hide; - doc.renderer = RenderPool::new(doc.bytes.clone(), render_threads(), doc.config.clone()); + doc.renderer = RenderPool::new(doc.display.clone(), render_threads(), doc.config.clone()); doc.generation += 1; true } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6a03dc547..a58152299 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2042,3 +2042,69 @@ fn runaway_xfa_calculations_at_open_end_quickly_with_a_report() { assert!(started.elapsed() < std::time::Duration::from_secs(2)); assert!(s.take_js_output(id).errors.is_empty()); } + +/// The page view's raster of `page` at 1 px/pt (the document's own render pool). +fn shown(s: &Session, id: DocId, page: usize) -> pdfcraft_render::RenderedPage { + let doc = s.get(id).unwrap(); + doc.renderer.set_queue(vec![pdfcraft_render::RenderRequest { page, scale: 1.0, ..Default::default() }]); + let start = std::time::Instant::now(); + loop { + if let Some(p) = doc.renderer.try_recv() { + assert!(p.error.is_none(), "{:?}", p.error); + return p; + } + assert!(start.elapsed() < std::time::Duration::from_secs(20), "no render"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } +} + +/// #260 (page 7): comments without an appearance stream, here a FreeText box (`/C` background, +/// blue `/DA` text) and an Ink stroke, weren't drawn. The page view now draws them from +/// appearances made for display only; the working file and what Save writes stay as they were. +#[test] +fn comments_without_appearances_are_drawn_but_not_saved() { + let objs: Vec<&[u8]> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>", + b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R] >>", + b"<< /Type /Annot /Subtype /FreeText /Rect [20 200 180 260] /Contents (Box) /DA (/Helv 10 Tf 0 0 1 rg) /Border [0 0 1] /C [.85 .47 .02] >>", + b"<< /Type /Annot /Subtype /Ink /Rect [20 20 180 120] /InkList [[30 30 100 110 170 30]] /Border [0 0 4] /C [0 .6 0] >>", + ]; + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + bytes.extend_from_slice(o); + bytes.extend_from_slice(b"\nendobj\n"); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let original = Arc::new(bytes); + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("comments.pdf", None, original.clone(), None).expect("opens"); + let check = |s: &Session| { + let p = shown(s, id, 0); + let px = |x: u32, y: u32| p.rgba[((y * p.width + x) * 4) as usize..][..4].to_vec(); + // y down: the FreeText box spans rows 40..100, the Ink apex is at (100, 190). + assert_eq!(px(100, 90), vec![217, 120, 5, 255], "the FreeText background"); + // On the apex row: Ink is drawn as a curve through its points (#374), level at the apex. + assert_eq!(px(100, 190), vec![0, 153, 0, 255], "the Ink stroke"); + }; + check(&s); + let doc = s.get(id).unwrap(); + assert!(Arc::ptr_eq(&doc.bytes, &original), "the working file is the file as opened"); + assert!(!Arc::ptr_eq(&doc.display, &doc.bytes) && Arc::ptr_eq(&doc.export_source().bytes, &doc.display), "page images show it too"); + // After an edit (a scoped refresh) they are still drawn, and Save writes no appearances. + s.apply(id, rect_comment(0, [150.0, 270.0, 190.0, 290.0])).unwrap(); + check(&s); + let saved = s.save_bytes(id).unwrap(); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + for r in [pdfcraft_cos::ObjRef::new(4, 0), pdfcraft_cos::ObjRef::new(5, 0)] { + assert!(reopened.get(r).as_dict().is_some_and(|d| !d.contains(b"AP")), "{r:?} is saved without /AP"); + } +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 5998819e5..c790b121b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -770,8 +770,8 @@ milestone = "M2" status = "shipped" commands = [] tools = [] -evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn"] -notes = "a Highlight without /AP is drawn from /QuadPoints, /C and /CA; other comment types without /AP are not drawn yet" +evidence = ["crates/render/src/raster.rs::widget_appearance_states", "crates/render/src/raster.rs::highlights_without_an_appearance_are_drawn", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved"] +notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, never saved (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered yet" [[feature]] id = "view.render-error-isolation"