Comments without an appearance stream are drawn (display only, never saved) (#371)

* M5: comments without an appearance stream are drawn (display only, never saved)

#260 (page 7): a FreeText box with /DA and an orange /C background, a
Text note and an Ink stroke, all without /AP, were not drawn; Acrobat,
MuPDF, PDFium and Poppler draw them. The renderer only draws /AP (plus
the Highlight fallback of #274), and pdfcraft-annot, which already
draws all of these for PdfCraft's own comments, can't be used from the
renderer (both are L3).

pdfcraft_annot::with_missing_appearances returns a copy of the document
in which every annotation without /AP /N that appearance::build can
draw (FreeText, Ink, Text, FileAttachment, stamps, shapes, lines,
markup, …) has the appearance build draws from its dictionary; links,
form fields and pop-ups are left alone, as is anything build declines.
The engine keeps that copy, written as an in-memory incremental update,
in a new Document::display: the page view's render pool, page_render
and page images (export_source) read it. Document::bytes, what Save,
signatures and the inspector use, is unchanged. display is rebuilt
wherever the renderer was (open, refresh, scoped refresh) and is bytes
itself when nothing is missing or the document can't be edited.

Tests: annot tests::comments_without_appearances_get_one_for_display_only
(FreeText, Ink and Text get /AP /N in the copy, an existing /AP is kept,
Link, Widget and an unknown subtype aren't touched, the original is
untouched, none means no copy) and engine
tests::comments_without_appearances_are_drawn_but_not_saved (the page
view draws the FreeText background and the Ink stroke, before and after
an edit; the working bytes stay the opened file; Save writes no /AP).
On main the page view shows white where the FreeText box is. Parity:
view.render-annotation-appearances cites the test and notes what's left
(Link borders without /AP, printing).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* Test the Ink stroke on its apex row: Ink is now a curve through its points (#374)

Co-Authored-By: Claude Opus 5.5 <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Brandon Thomas <[email protected]>

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<Document> {
+    let mut copy: Option<Document> = 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 });

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::<Vec<_>>();
+    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());
+}

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)

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

crates/engine/src/lib.rs

@@ -194,8 +194,11 @@ pub struct Document {
     pub id: DocId,
     pub name: String,
     pub path: Option<String>,
-    /// The working file: what Save writes and what is displayed.
+    /// The working file: what Save writes.
     pub bytes: Arc<Vec<u8>>,
+    /// 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<Vec<u8>>,
     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<Vec<u8>>) -> Arc<Vec<u8>> {
+    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<XfaLayout>,
     ) -> Result<DocId, OpenError> {
         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
     }

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");
+    }
+}

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"