From b661ba4584e510f9077852c8af609322ad7cdf35 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:53:42 +0800 Subject: [PATCH 1/2] fix(forms): draw fields the file leaves to the viewer, for display only (#3470) A form with /NeedAppearances true (ISO 32000-2 Table 224: the writer "has not provided appearance streams for all visible widget annotations") showed its text fields empty: PdfCraft only read the flag when writing, and the display copy that draws missing appearances skips widgets. Filled forms from tools that leave drawing to the viewer looked blank, while Acrobat, Firefox and MuPDF show the values. pdfcraft_forms::with_field_appearances returns a copy with text and choice widgets drawn from their values (every one when the flag is set, otherwise those with no /AP /N), with the generator a typed value uses. The engine's display copy (page view, page images, Print as Image, CLI render) applies it after the comment appearances. The document itself is not changed, so saving writes the file as it was. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/engine/src/lib.rs | 19 +++++++---- crates/engine/src/tests.rs | 60 +++++++++++++++++++++++++++++++++++ crates/forms/README.md | 4 +++ crates/forms/src/lib.rs | 28 +++++++++++++++++ crates/forms/src/tests.rs | 61 ++++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +-- 6 files changed, 168 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8f39248ab..3caa0c828 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -876,12 +876,13 @@ fn is_placeholder_title(title: &str) -> bool { address || t == "untitled" } -/// The working file plus appearances for comments that have none. Signature artwork -/// is preserved; validation badges belong to the viewer and never enter these bytes, -/// which image/text exports and Print as Image also consume. +/// The working file plus appearances for comments that have none, and for form fields the file +/// leaves to the viewer (#3470). Signature artwork is preserved; validation badges belong to the +/// viewer and never enter these bytes, which image/text exports and Print as Image also consume. fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { let Some(e) = editor else { return bytes.clone() }; - let Some(display) = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances) else { + let comments = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances); + let Some(display) = with_field_appearances(comments, &e.cos) else { return bytes.clone(); }; match write_incremental(&display, &SaveOptions::default()) { @@ -893,6 +894,12 @@ fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc, doc: &pdfcraft_cos::Document) -> Option { + pdfcraft_forms::with_field_appearances(comments.as_ref().unwrap_or(doc)).or(comments) +} + /// The reader shows the same pages in `a` and `b`: count, displayed size and rotation. fn same_pages(a: &[pdfcraft_render::PageInfo], b: &[pdfcraft_render::PageInfo]) -> bool { a.len() == b.len() @@ -900,14 +907,14 @@ fn same_pages(a: &[pdfcraft_render::PageInfo], b: &[pdfcraft_render::PageInfo]) } /// The copy PDFCraft displays when opening `bytes`: with appearances drawn, in memory only, for -/// comments that have none. Render paths outside the app, such as the CLI, use this so their page +/// comments that have none and form fields the file leaves to the viewer. Render paths outside the app, such as the CLI, use this so their page /// images agree with the app and independent viewers. It never changes the source document, and an /// unparseable file, a write failure or a parser panic falls back to the original bytes. pub fn display_bytes_for(bytes: Arc>, password: Option<&str>) -> Arc> { guard(|| { pdfcraft_cos::Document::open_with_password(bytes.clone(), password) .ok() - .and_then(|cos| pdfcraft_annot::with_missing_appearances(&cos)) + .and_then(|cos| with_field_appearances(pdfcraft_annot::with_missing_appearances(&cos), &cos)) .and_then(|display| write_incremental(&display, &SaveOptions::default()).ok()) .map(Arc::new) }) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f125c41da..d199eeb70 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6106,3 +6106,63 @@ fn undo_never_makes_an_unsaved_document_clean() { s.undo(new).unwrap(); assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); } + +/// #3470: a filled form whose writer left the appearances to the viewer (`/NeedAppearances`): +/// field A has an empty appearance, B none. The page as displayed (and printed) shows both +/// values, and a comment without an appearance beside them still gets its own; the file itself +/// is not changed, so saving writes it as it was. +#[test] +fn fields_left_to_the_viewer_are_displayed_from_their_values() { + let pdf = |need: bool| { + let flag = if need { "/NeedAppearances true " } else { "" }; + format!( + "%PDF-1.7\n\ + 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm 5 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 400 200] /Annots [6 0 R 7 0 R 10 0 R] >> endobj\n\ + 10 0 obj << /Type /Annot /Subtype /Square /Rect [360 20 390 50] /C [1 0 0] /BS << /W 4 >> >> endobj\n\ + 5 0 obj << /Fields [6 0 R 7 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 8 0 R >> >> >> endobj\n\ + 6 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (a) /V (MMMMMMMM) /DA (/Helv 12 Tf 0 g) /Rect [50 120 350 140] /F 4 /P 3 0 R /AP << /N 9 0 R >> >> endobj\n\ + 7 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (b) /V (MMMMMMMM) /DA (/Helv 12 Tf 0 g) /Rect [50 60 350 80] /F 4 /P 3 0 R >> endobj\n\ + 8 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> endobj\n\ + 9 0 obj << /Type /XObject /Subtype /Form /BBox [0 0 300 20] /Length 4 >> stream\nq\nQ\nendstream endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF" + ) + .into_bytes() + }; + let red = |s: &Session, id: DocId| { + let doc = s.get(id).unwrap(); + let mut r = pdfcraft_render::PageRenderer::new(doc.display.clone(), doc.config.clone()); + let page = r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }); + page.rgba.as_chunks::<4>().0.iter().filter(|p| p[0] > 200 && p[1] < 80 && p[2] < 80).count() + }; + // Dark pixels inside a field's box (user-space rectangle, page drawn at scale 1). + let ink = |s: &Session, id: DocId, y0: u32, y1: u32| { + let doc = s.get(id).unwrap(); + let mut r = pdfcraft_render::PageRenderer::new(doc.display.clone(), doc.config.clone()); + let page = r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }); + let (px, _) = page.rgba.as_chunks::<4>(); + (200 - y1..200 - y0) + .flat_map(|y| (55..345).map(move |x| (x, y))) + .filter(|&(x, y)| px.get((y * page.width + x) as usize).is_some_and(|p| p[0] < 128)) + .count() + }; + let mut s = Session::new(); + let id = s.open("form.pdf", None, Arc::new(pdf(true)), None).unwrap(); + let (a, b) = (ink(&s, id, 122, 138), ink(&s, id, 62, 78)); + assert!(a > 100 && b > 100, "both values are shown: a {a}, b {b}"); + assert!(red(&s, id) > 100, "the comment is drawn too"); + // The file is saved as it was: B still has no appearance, A its empty one. + let saved = pdfcraft_cos::Document::open(s.save_bytes(id).unwrap()).unwrap(); + let fields = pdfcraft_forms::fields(&saved); + let widget = |name: &str| saved.get(fields.iter().find(|f| f.name == name).unwrap().widgets[0].obj); + assert!(widget("b").as_dict().is_some_and(|d| !d.contains(b"AP")), "B has no appearance"); + let n = widget("a").as_dict().and_then(|d| d.get(b"AP").and_then(|ap| saved.resolve(ap).as_dict().and_then(|ap| ap.reference(b"N")))).unwrap(); + let pdfcraft_cos::Object::Stream(n) = &*saved.get(n) else { panic!("A's appearance is a stream") }; + assert_eq!(n.decoded().unwrap().trim_ascii_end(), b"q\nQ", "A keeps its empty appearance"); + + // Without the flag, the empty appearance is the file's own, and only B is drawn. + let id = s.open("form.pdf", None, Arc::new(pdf(false)), None).unwrap(); + let (a, b) = (ink(&s, id, 122, 138), ink(&s, id, 62, 78)); + assert!(a == 0 && b > 100, "a {a}, b {b}"); +} diff --git a/crates/forms/README.md b/crates/forms/README.md index 66fb919fe..a05b69b94 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -42,6 +42,10 @@ delete_field(&mut doc, "email")?; that have them. Not in a document that declares PDF/A, which forbids that flag (`declares_pdfa`). `undrawn_fields` lists the marked fields; the engine reports each edit that draws one, and flattening refuses to make that `?` permanent. +- Displaying a form that leaves its appearances to the viewer (`/NeedAppearances`, or a text or + choice widget with no `/AP /N`): `with_field_appearances` returns a copy with those widgets + drawn from their values, as typing them would draw them; the engine shows and prints that copy + and saves the file as it was (#3470). - Check boxes and radio buttons switch `/V` and each widget's `/AS`. Widgets with no appearances get PdfCraft's own (a drawn check mark or dot; no symbol font needed). - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 1dfa76726..8d78fb899 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1232,6 +1232,34 @@ pub fn need_appearances(doc: &Document) -> bool { acroform(doc).is_some_and(|af| matches!(af.get(b"NeedAppearances").map(|o| doc.resolve(o)).as_deref(), Some(Object::Bool(true)))) } +/// A copy of `doc` in which text and choice fields are drawn from their values, for displaying +/// the document (`None` when no widget needs it): every such widget when the form sets +/// `/NeedAppearances` (ISO 32000-2 Table 224: the writer left the appearances to the viewer), +/// and otherwise each widget without a normal appearance (`/AP /N`). Drawn as typing the value +/// would draw it (#3470). `doc` is not changed: saving writes the file as it was. +pub fn with_field_appearances(doc: &Document) -> Option { + let all = need_appearances(doc); + let mut copy: Option = None; + for f in fields(doc).iter().filter(|f| matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List)) { + for w in &f.widgets { + let widget = doc.get(w.obj); + let drawn = widget.as_dict().and_then(|d| d.get(b"AP")).is_some_and(|ap| doc.resolve(ap).as_dict().is_some_and(|ap| ap.contains(b"N"))); + if drawn && !all { + continue; + } + let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + let target = copy.get_or_insert_with(|| doc.clone()); + let ap = target.add(Object::Stream(stream)); + let mut fresh = Dict::new(); + fresh.set(b"N".to_vec(), Object::Ref(ap)); + let apd = merged_appearance(target, w.obj, &fresh); + // Fails only when the widget isn't a dictionary; that widget then keeps its own look. + target.update_dict(w.obj, |d| d.set(b"AP".to_vec(), Object::Dict(apd))).ok(); + } + } + copy +} + /// Whether the document's XMP metadata declares PDF/A conformance (`pdfaid:part`). pub fn declares_pdfa(doc: &Document) -> bool { let Some(root) = doc.root() else { return false }; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd..987a74f2c 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2277,3 +2277,64 @@ fn document_scripts_read_each_name_tree_node_once() { ]); assert_eq!(document_scripts_named(&doc), [("init".to_string(), "var ready = true;".to_string())]); } + +/// #3470: a form whose writer left the appearances to the viewer. Field A has an empty one, +/// B none, C its own (`(old C)`), D is a combo without one, E a check box with states, F a list +/// box without one, and G only a down appearance (`/D`, no `/N`). +fn left_to_the_viewer(need_appearances: bool) -> Document { + let flag = if need_appearances { "/NeedAppearances true " } else { "" }; + document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R 8 0 R 9 0 R 10 0 R 13 0 R 14 0 R] >>".into(), + format!("<< /Fields [6 0 R 7 0 R 8 0 R 9 0 R 10 0 R 13 0 R 14 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>"), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (a) /V (Value A) /DA (/Helv 12 Tf 0 g) /Rect [50 700 350 720] /P 3 0 R /AP << /N 11 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (b) /V (Value B) /DA (/Helv 12 Tf 0 g) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (c) /V (Value C) /DA (/Helv 12 Tf 0 g) /Rect [50 600 350 620] /P 3 0 R /AP << /N 12 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Ch /Ff 131072 /T (d) /Opt [(Red) (Blue)] /V (Blue) /DA (/Helv 12 Tf 0 g) /Rect [50 550 350 570] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (e) /V /Yes /AS /Yes /Rect [400 700 415 715] /P 3 0 R /AP << /N << /Yes 11 0 R /Off 11 0 R >> >> >>".into(), + "<< /Length 4 >>\nstream\nq\nQ\nendstream".into(), + "<< /Length 18 >>\nstream\nBT (old C) Tj ET\nendstream".into(), + "<< /Type /Annot /Subtype /Widget /FT /Ch /T (f) /Opt [(Green) (Grey)] /V (Green) /DA (/Helv 12 Tf 0 g) /Rect [50 450 350 520] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (g) /V (Value G) /DA (/Helv 12 Tf 0 g) /Rect [50 400 350 420] /P 3 0 R /AP << /D 11 0 R >> >>".into(), + ]) +} + +/// #3470: with `/NeedAppearances`, every text and choice widget of the display copy is drawn +/// from its value, its own appearance or not; buttons keep their states, and the document +/// itself is untouched. +#[test] +fn need_appearances_draws_every_text_and_choice_field_in_the_display_copy() { + let doc = left_to_the_viewer(true); + let copy = with_field_appearances(&doc).expect("fields to draw"); + let all = fields(©); + for (name, value) in + [("a", "(Value A) Tj"), ("b", "(Value B) Tj"), ("c", "(Value C) Tj"), ("d", "(Blue) Tj"), ("f", "(Green) Tj"), ("g", "(Value G) Tj")] + { + let shown = ap(©, &field(&all, name).widgets[0]); + assert!(shown.contains(value), "{name}: {shown}"); + } + let e = |d: &Document| d.get(field(&fields(d), "e").widgets[0].obj).as_dict().and_then(|w| w.get(b"AP").cloned()); + assert_eq!(e(©), e(&doc), "the check box keeps its states"); + let original = fields(&doc); + assert_eq!(ap(&doc, &field(&original, "a").widgets[0]).trim_end(), "q\nQ", "the document keeps its own appearances"); + assert!(doc.get(field(&original, "b").widgets[0].obj).as_dict().is_some_and(|w| !w.contains(b"AP"))); +} + +/// #3470: without `/NeedAppearances`, only widgets with no normal appearance are drawn; an +/// appearance the file has, even an empty one, is the file's. A form with all of them needs no +/// copy. +#[test] +fn without_need_appearances_only_missing_appearances_are_drawn() { + let doc = left_to_the_viewer(false); + let copy = with_field_appearances(&doc).expect("b, d, f and g have none"); + let all = fields(©); + assert_eq!(ap(©, &field(&all, "a").widgets[0]).trim_end(), "q\nQ"); + assert!(ap(©, &field(&all, "b").widgets[0]).contains("(Value B) Tj")); + assert_eq!(ap(©, &field(&all, "c").widgets[0]).trim_end(), "BT (old C) Tj ET"); + assert!(ap(©, &field(&all, "d").widgets[0]).contains("(Blue) Tj")); + assert!(ap(©, &field(&all, "f").widgets[0]).contains("(Green) Tj")); + assert!(ap(©, &field(&all, "g").widgets[0]).contains("(Value G) Tj"), "a down appearance alone is no normal one"); + assert!(with_field_appearances(©).is_none(), "every widget has one now"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index e19efb0a0..35812170b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -806,8 +806,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", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source"] -notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered 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", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy"] +notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only. Not cached: a form with 1000 such fields adds about 80 ms to each refresh (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" From da351cda8f0bef0265da4bedf7d4144cb8020cc3 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:26:31 +0800 Subject: [PATCH 2/2] fix(forms): cache field appearances drawn for display, redraw only what changed (#3470) Review follow-up (CodeRabbit on #3475). A form edit refreshes the display copy on every keystroke, and with /NeedAppearances that redrew every field of the form: 68 ms per edit on a 1000-field form against 7 ms on main. FieldAppearanceCache keeps each widget's drawing with the objects it was drawn from: the widget and its field ancestors, the form dictionary and the fonts of its /DR, each with the objects its entries refer to. An entry is used only while each of them is still the same object (Arc::ptr_eq); the document replaces an object it edits, so a changed input is a miss, never a stale hit. The engine keeps one per editor. A field edit on that form now takes 13 ms. The repair note on a failed display write now names field appearances too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q --- crates/engine/src/lib.rs | 10 ++- crates/engine/src/tests.rs | 5 ++ crates/forms/README.md | 4 +- crates/forms/src/lib.rs | 127 ++++++++++++++++++++++++++++++++++- crates/forms/src/tests.rs | 107 +++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 6 files changed, 251 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3caa0c828..af80271b9 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -316,6 +316,10 @@ struct Editor { /// runs (content edits leave every annotation dictionary alone). Cleared on any refresh /// that may change annotations; see [`AppearanceCache`]. appearances: AppearanceCache, + /// Display appearances drawn for form fields the file leaves to the viewer (#3470), reused + /// for each widget while the objects it was drawn from are unchanged; see + /// [`pdfcraft_forms::FieldAppearanceCache`]. + field_appearances: pdfcraft_forms::FieldAppearanceCache, /// The text blocks last computed for one page, reused while the document is unchanged. text_blocks: TextBlocksCache, } @@ -882,13 +886,14 @@ fn is_placeholder_title(title: &str) -> bool { fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { let Some(e) = editor else { return bytes.clone() }; let comments = pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances); - let Some(display) = with_field_appearances(comments, &e.cos) else { + let fields = pdfcraft_forms::with_field_appearances_cached(comments.as_ref().unwrap_or(&e.cos), &mut e.field_appearances); + let Some(display) = fields.or(comments) else { return bytes.clone(); }; match write_incremental(&display, &SaveOptions::default()) { Ok(written) => Arc::new(written), Err(err) => { - e.cos.note_repair(format!("missing comment appearances could not be drawn ({err})")); + e.cos.note_repair(format!("missing comment or field appearances could not be drawn ({err})")); bytes.clone() } } @@ -3576,6 +3581,7 @@ impl Session { redo: Vec::new(), keys, appearances: AppearanceCache::default(), + field_appearances: pdfcraft_forms::FieldAppearanceCache::default(), text_blocks: TextBlocksCache::default(), }), None, diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d199eeb70..46dbe4ce3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6160,6 +6160,11 @@ fn fields_left_to_the_viewer_are_displayed_from_their_values() { let n = widget("a").as_dict().and_then(|d| d.get(b"AP").and_then(|ap| saved.resolve(ap).as_dict().and_then(|ap| ap.reference(b"N")))).unwrap(); let pdfcraft_cos::Object::Stream(n) = &*saved.get(n) else { panic!("A's appearance is a stream") }; assert_eq!(n.decoded().unwrap().trim_ascii_end(), b"q\nQ", "A keeps its empty appearance"); + // Typing in one field redraws that field only, not the whole form, on each keystroke. + let builds = |s: &Session| s.get(id).unwrap().editor.as_ref().unwrap().field_appearances.builds(); + let before = builds(&s); + s.apply(id, Edit::SetFieldValue { name: "a".into(), value: pdfcraft_forms::FieldValue::Text("Typed".into()) }).unwrap(); + assert_eq!(builds(&s), before + 1); // Without the flag, the empty appearance is the file's own, and only B is drawn. let id = s.open("form.pdf", None, Arc::new(pdf(false)), None).unwrap(); diff --git a/crates/forms/README.md b/crates/forms/README.md index a05b69b94..96f1f01f9 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -45,7 +45,9 @@ delete_field(&mut doc, "email")?; - Displaying a form that leaves its appearances to the viewer (`/NeedAppearances`, or a text or choice widget with no `/AP /N`): `with_field_appearances` returns a copy with those widgets drawn from their values, as typing them would draw them; the engine shows and prints that copy - and saves the file as it was (#3470). + and saves the file as it was (#3470). `with_field_appearances_cached` keeps each widget's + drawing while the objects it was drawn from are the same (`FieldAppearanceCache`), so typing in + one field redraws that field, not the form. - Check boxes and radio buttons switch `/V` and each widget's `/AS`. Widgets with no appearances get PdfCraft's own (a drawn check mark or dot; no symbol font needed). - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 8d78fb899..8a27d3ebd 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1238,6 +1238,12 @@ pub fn need_appearances(doc: &Document) -> bool { /// and otherwise each widget without a normal appearance (`/AP /N`). Drawn as typing the value /// would draw it (#3470). `doc` is not changed: saving writes the file as it was. pub fn with_field_appearances(doc: &Document) -> Option { + with_field_appearances_cached(doc, &mut FieldAppearanceCache::default()) +} + +/// [`with_field_appearances`], reusing the appearances `cache` drew for widgets whose inputs are +/// the same objects as then (see [`FieldAppearanceCache`]). +pub fn with_field_appearances_cached(doc: &Document, cache: &mut FieldAppearanceCache) -> Option { let all = need_appearances(doc); let mut copy: Option = None; for f in fields(doc).iter().filter(|f| matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List)) { @@ -1247,7 +1253,15 @@ pub fn with_field_appearances(doc: &Document) -> Option { if drawn && !all { continue; } - let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + let inputs = appearance_inputs(doc, w); + let stream = match cache.get(w.obj, &inputs) { + Some(stream) => stream, + None => { + let (stream, _) = appearance::field_appearance_drawn(doc, f, w, &f.value, true); + cache.insert(w.obj, inputs, &stream); + stream + } + }; let target = copy.get_or_insert_with(|| doc.clone()); let ap = target.add(Object::Stream(stream)); let mut fresh = Dict::new(); @@ -1260,6 +1274,117 @@ pub fn with_field_appearances(doc: &Document) -> Option { copy } +/// The objects a widget's drawn appearance is built from, as the document holds them: the widget +/// and the fields above it (value, `/DA`, `/Q`, flags, options, `/MK`, formatting actions), the +/// form dictionary (`/DA`) and the fonts of its `/DR`, each with the objects its entries refer to +/// (an indirect `/MK` or `/V`). Not the page: a field's rotation is its own `/MK /R`. +fn appearance_inputs(doc: &Document, w: &Widget) -> Vec> { + // The objects a dictionary's entries refer to. The tree links and the drawn appearance itself + // are not inputs, and `/Kids` and `/Fields` can be long. + let referred = |d: &Dict, inputs: &mut Vec>| { + for (k, v) in d.iter() { + if !matches!(&k[..], b"Kids" | b"Fields" | b"P" | b"Parent" | b"AP") + && let Some(r) = v.as_ref() + { + inputs.push(doc.get(r)); + } + } + }; + let mut inputs = Vec::new(); + let mut seen = std::collections::HashSet::new(); + let mut node = Some(w.obj); + // The /Parent chain; the seen-set and the cap end a cycle or an absurd depth. + while let Some(r) = node.filter(|r| seen.insert(*r) && seen.len() <= 64) { + let obj = doc.get(r); + if let Some(d) = obj.as_dict() { + node = d.reference(b"Parent"); + referred(d, &mut inputs); + } else { + node = None; + } + inputs.push(obj); + } + let Some(root) = doc.root() else { return inputs }; + let catalog = doc.get(root); + let form = match catalog.as_dict().and_then(|c| c.reference(b"AcroForm")) { + Some(r) => doc.get(r), + None => catalog, + }; + if let Some(f) = form.as_dict() { + referred(f, &mut inputs); + // The fonts of /DR. An indirect /DR or /Font is listed through the entries that refer to + // it; a direct one is part of the object that holds it. `held` only finds them: a copy + // of a direct object is a new one each time, never an input. + let held = |o: &Object| o.as_ref().map_or_else(|| std::sync::Arc::new(o.clone()), |r| doc.get(r)); + if let Some(dr) = f.get(b"DR").map(held) + && let Some(dr) = dr.as_dict() + { + referred(dr, &mut inputs); + if let Some(font) = dr.get(b"Font").map(held) + && let Some(fonts) = font.as_dict() + { + referred(fonts, &mut inputs); + } + } + } + inputs.push(form); + inputs +} + +/// Field appearances drawn for the display copy ([`with_field_appearances_cached`]), so a refresh +/// redraws only the widgets whose inputs changed: a form that leaves every appearance to the +/// viewer otherwise redrew all of its fields on each keystroke in any one of them (#3470). An +/// entry holds the input objects it was drawn from and is used only while each is still the same +/// object (`Arc::ptr_eq`); the document replaces an object it edits, so a changed input is a +/// miss, never a stale hit. +#[derive(Clone, Debug, Default)] +pub struct FieldAppearanceCache { + entries: std::collections::HashMap>, pdfcraft_cos::Stream)>, + bytes: usize, + hits: u64, + builds: u64, +} + +impl FieldAppearanceCache { + /// Cached appearance data restarts past this instead of growing without bound. + const MAX_BYTES: usize = 64 << 20; + + /// Forget every entry. + pub fn clear(&mut self) { + *self = Self::default(); + } + + fn get(&mut self, w: ObjRef, inputs: &[std::sync::Arc]) -> Option { + let (from, stream) = self.entries.get(&w)?; + if from.len() != inputs.len() || !from.iter().zip(inputs).all(|(a, b)| std::sync::Arc::ptr_eq(a, b)) { + return None; + } + self.hits += 1; + Some(stream.clone()) + } + + fn insert(&mut self, w: ObjRef, inputs: Vec>, stream: &pdfcraft_cos::Stream) { + self.builds += 1; + if self.bytes.saturating_add(stream.raw.len()) > Self::MAX_BYTES { + self.clear(); + } + self.bytes = self.bytes.saturating_add(stream.raw.len()); + if let Some((_, old)) = self.entries.insert(w, (inputs, stream.clone())) { + self.bytes = self.bytes.saturating_sub(old.raw.len()); + } + } + + /// Cache hits so far (reused appearances); for tests. + pub fn hits(&self) -> u64 { + self.hits + } + + /// Appearances drawn so far; for tests. + pub fn builds(&self) -> u64 { + self.builds + } +} + /// Whether the document's XMP metadata declares PDF/A conformance (`pdfaid:part`). pub fn declares_pdfa(doc: &Document) -> bool { let Some(root) = doc.root() else { return false }; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 987a74f2c..7a4bd8695 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2338,3 +2338,110 @@ fn without_need_appearances_only_missing_appearances_are_drawn() { assert!(ap(©, &field(&all, "g").widgets[0]).contains("(Value G) Tj"), "a down appearance alone is no normal one"); assert!(with_field_appearances(©).is_none(), "every widget has one now"); } + +/// #3470: refreshing the display copy redraws only the widgets whose inputs changed. Typing in +/// one field of a form that leaves every appearance to the viewer otherwise redrew all of them. +#[test] +fn the_field_appearance_cache_redraws_only_what_changed() { + let mut doc = left_to_the_viewer(true); + let mut cache = FieldAppearanceCache::default(); + with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + let drawn = cache.builds(); + assert_eq!(drawn, 6, "a, b, c, d, f and g"); + with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + assert_eq!((cache.builds(), cache.hits()), (drawn, drawn), "nothing changed, nothing redrawn"); + + set_value(&mut doc, "b", &FieldValue::Text("Typed".into())).unwrap(); + let copy = with_field_appearances_cached(&doc, &mut cache).expect("fields to draw"); + assert_eq!(cache.builds(), drawn + 1, "only b is redrawn"); + assert!(ap(©, &field(&fields(©), "b").widgets[0]).contains("(Typed) Tj"), "and shows its new value"); + assert!(ap(©, &field(&fields(©), "a").widgets[0]).contains("(Value A) Tj"), "a reuses its drawing"); +} + +/// #3470: each kind of input a field's drawing depends on invalidates its cached appearance when +/// it changes, wherever it lives: the parent field's value, the form's `/DA`, an indirect `/MK` +/// and a `/DR` font. Field H is a parent with one kid widget; I is a plain field. +#[test] +fn the_field_appearance_cache_sees_every_input() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R] >>".into(), + "<< /Fields [5 0 R 7 0 R] /NeedAppearances true /DA (/Helv 12 Tf 0 g) /DR << /Font << /Helv 8 0 R >> >> >>".into(), + "<< /FT /Tx /T (h) /V (Kid value) /Kids [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /Rect [50 700 350 720] /P 3 0 R /MK 9 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (i) /V (Plain) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + "<< /BG [1 1 1] >>".into(), + ]); + let r = |n: u32| pdfcraft_cos::ObjRef::new(n, 0); + let mut cache = FieldAppearanceCache::default(); + let mut redraw = |doc: &Document| { + let before = cache.builds(); + let copy = with_field_appearances_cached(doc, &mut cache).expect("fields to draw"); + (cache.builds() - before, copy) + }; + assert_eq!(redraw(&doc).0, 2); + assert_eq!(redraw(&doc).0, 0, "unchanged"); + + doc.update_dict(r(5), |d| d.set(b"V".to_vec(), Object::String(PdfString::text("New value")))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 1, "the parent's value: only its kid"); + assert!(ap(©, &field(&fields(©), "h").widgets[0]).contains("(New value) Tj")); + + doc.update_dict(r(9), |d| d.set(b"BG".to_vec(), Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)]))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 1, "the kid's indirect /MK"); + assert!(ap(©, &field(&fields(©), "h").widgets[0]).contains("1 0 0 rg"), "the new background"); + + doc.update_dict(r(4), |d| d.set(b"DA".to_vec(), Object::String(PdfString::text("/Helv 9 Tf 0 g")))).unwrap(); + let (n, copy) = redraw(&doc); + assert_eq!(n, 2, "the form's /DA: both fields"); + assert!(ap(©, &field(&fields(©), "i").widgets[0]).contains("/Helv 9 Tf")); + + doc.update_dict(r(8), |d| d.set(b"BaseFont".to_vec(), Object::Name(b"Courier".to_vec()))).unwrap(); + assert_eq!(redraw(&doc).0, 2, "a /DR font: both fields"); +} + +/// #3470: an indirect `/DR` and an indirect `/Font` inside it are inputs too, with a font written +/// directly into the `/Font` dictionary, and so is the form's `/DA` when it is an indirect string. +#[test] +fn the_field_appearance_cache_sees_indirect_resources() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R] >>".into(), + "<< /Fields [5 0 R] /NeedAppearances true /DA 8 0 R /DR 6 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (i) /V (Plain) /Rect [50 650 350 670] /P 3 0 R >>".into(), + "<< /Font 7 0 R >>".into(), + "<< /Helv << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> >>".into(), + "(/Helv 12 Tf 0 g)".into(), + ]); + let r = |n: u32| pdfcraft_cos::ObjRef::new(n, 0); + let mut cache = FieldAppearanceCache::default(); + let mut redraw = |doc: &Document| { + let before = cache.builds(); + with_field_appearances_cached(doc, &mut cache).expect("a field to draw"); + cache.builds() - before + }; + assert_eq!(redraw(&doc), 1); + assert_eq!(redraw(&doc), 0); + let courier = || { + let mut f = Dict::new(); + f.set(b"Type".to_vec(), Object::Name(b"Font".to_vec())); + f.set(b"Subtype".to_vec(), Object::Name(b"Type1".to_vec())); + f.set(b"BaseFont".to_vec(), Object::Name(b"Courier".to_vec())); + Object::Dict(f) + }; + doc.update_dict(r(7), |d| d.set(b"Helv".to_vec(), courier())).unwrap(); + assert_eq!(redraw(&doc), 1, "the indirect /Font dictionary"); + doc.update_dict(r(6), |d| { + let mut fonts = Dict::new(); + fonts.set(b"Helv".to_vec(), courier()); + d.set(b"Font".to_vec(), Object::Dict(fonts)); + }) + .unwrap(); + assert_eq!(redraw(&doc), 1, "the indirect /DR"); + doc.set(r(8), Object::String(PdfString::text("/Helv 9 Tf 0 g"))); + assert_eq!(redraw(&doc), 1, "the form's indirect /DA"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 35812170b..872018003 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -806,8 +806,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", "crates/engine/src/tests.rs::comments_without_appearances_are_drawn_but_not_saved", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy"] -notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only. Not cached: a form with 1000 such fields adds about 80 ms to each refresh (a read-only document still gets the renderer's Highlight fallback only); Link borders without /AP and printing are not covered 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", "crates/engine/src/tests.rs::cli_display_adds_missing_appearances_without_changing_the_source", "crates/engine/src/tests.rs::fields_left_to_the_viewer_are_displayed_from_their_values", "crates/forms/src/tests.rs::need_appearances_draws_every_text_and_choice_field_in_the_display_copy", "crates/forms/src/tests.rs::the_field_appearance_cache_sees_every_input"] +notes = "comments without /AP (FreeText, Ink, notes, stamps, shapes, markup) are drawn from their dictionaries in the page view, page_render and page images, including the CLI render command, never saved; text and choice fields are drawn from their values when the form sets /NeedAppearances (every one) or a widget has no /N (#3470), also display-only and cached per widget (a field edit on a 1000-field /NeedAppearances form takes ~13 ms against ~7 ms on a form that needs none) (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"