M5: comments without an appearance read style values given by reference, and a callout without /RD is drawn (view.render-annotation-appearances)

Any value may be an indirect object (ISO 32000-2 §7.3.10). The appearance
builder read only direct values, so a border style dictionary given as
`/BS 5 0 R` (Table 168) was ignored and the border drawn 1 pt solid. The
display copy, a redraw, the properties and a new width now read the
annotation with its style references followed (bounded in depth and in
copied values; links such as /P and /Popup and streams stay references).
A new width keeps an indirect /BS's other entries instead of dropping them.

/RD is optional (Table 177): a FreeText callout without it now has its
text box at /Rect and is drawn, where it was not drawn at all.

PDFBOX-4515 (annotations-2.0-without-AP).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn
This commit is contained in:
mtlcl410andClaude Opus 5.5 committed 2026-11-07 11:56:24 +08:00
1 parent f42fda7342
commit e69b433156
6 files changed
+198 -9

No files matched your search

+6 -1
View File
@@ -5,6 +5,7 @@
//! `/BBox = /Rect` (identity matrix), except note icons, which are drawn in a 20 × 20 box.
//! It returns `None` for anything it cannot draw faithfully (cloudy borders, unknown line
//! endings, indirect geometry), so callers never replace an appearance with a worse one.
//! Callers that hold the document follow references first (`draw::with_direct_values`).
//!
//! The note icons are PdfCraft's own drawings (AGENTS.md §1). Text boxes use the standard
//! Helvetica font with WinAnsi encoding; line breaking uses [`text_width`], an approximation of
@@ -1019,7 +1020,11 @@ fn draw_free_text(context: &AppearanceContext<'_>, mut drawing: AppearanceDrawin
let mut rect = rect;
if d.contains(b"CL") {
let cl = nums(d, b"CL").filter(|l| l.len() == 4 || l.len() == 6)?;
let rd = nums(d, b"RD").filter(|r| r.len() == 4 && r.iter().all(|x| *x >= 0.0))?;
// `/RD` is optional (Table 177; null is absent, §7.3.9): without it the text box is `/Rect`.
let rd = match d.get(b"RD") {
None | Some(Object::Null) => vec![0.0; 4],
Some(_) => nums(d, b"RD").filter(|r| r.len() == 4 && r.iter().all(|x| *x >= 0.0))?,
};
rect = [full[0] + rd[0], full[1] + rd[1], full[2] - rd[2], full[3] - rd[3]];
if rect[2] - rect[0] < 1.0 || rect[3] - rect[1] < 1.0 {
return None;
+82 -3
View File
@@ -40,7 +40,7 @@ pub(crate) fn draw_appearance(doc: &mut Document, r: ObjRef) -> Result<(), Annot
/// Draw the appearance for an annotation dictionary: the expensive half of [`set_appearance`].
/// Pure from the dictionary and the objects it points at, so [`AppearanceCache`] can reuse it.
fn build_appearance(doc: &Document, d: &Dict) -> Option<Stream> {
let mut stream = appearance::build(d)?;
let mut stream = appearance::build(&with_direct_values(doc, d))?;
// Picture-stamp restyling must retain the placement's page-axis correction.
if d.get(b"PCPicture").is_some() {
let matrix = d.get(b"AP").and_then(|ap| {
@@ -56,10 +56,89 @@ fn build_appearance(doc: &Document, d: &Dict) -> Option<Stream> {
Some(stream)
}
/// Entries that link the annotation to other objects rather than describe how it looks: left as
/// references by [`with_direct_values`].
const LINK_KEYS: [&[u8]; 5] = [b"P", b"Popup", b"IRT", b"Parent", b"AP"];
/// How deep inside an entry [`with_direct_values`] follows references: `/InkList` is an array of
/// arrays, `/Border` may end in a dash array.
const INLINE_DEPTH: usize = 4;
/// Values [`with_direct_values`] copies out of referenced arrays and dictionaries for one
/// annotation, so a file can't have a large object copied for every entry that points at it.
const INLINE_BUDGET: usize = 1 << 16;
/// `d` with the references among the values [`appearance::build`] reads followed. Any value may be
/// given by reference (ISO 32000-2 §7.3.10): a border style dictionary (`/BS 5 0 R`, Table 168), a
/// colour, `/RD`, `/CL`, a `/W` inside `/BS`. The builder reads only direct values, so it draws
/// from this copy. The entries in [`LINK_KEYS`] and streams stay references; past
/// [`INLINE_DEPTH`] levels or [`INLINE_BUDGET`] copied values a reference is left as it is, which
/// the builder treats as unreadable. `d` itself when nothing is a reference.
pub(crate) fn with_direct_values<'d>(doc: &Document, d: &'d Dict) -> std::borrow::Cow<'d, Dict> {
let mut budget = INLINE_BUDGET;
let mut out: Option<Dict> = None;
for (key, value) in d.iter() {
if LINK_KEYS.contains(&key.as_slice()) {
continue;
}
if let Some(direct) = direct_value(doc, value, INLINE_DEPTH, &mut budget) {
out.get_or_insert_with(|| d.clone()).set(key.clone(), direct);
}
}
out.map_or(std::borrow::Cow::Borrowed(d), std::borrow::Cow::Owned)
}
/// `o` with references followed `depth` levels into arrays and dictionaries, or `None` when there
/// is nothing to follow (or it may not be: a stream, an over-budget object, a chain of references).
fn direct_value(doc: &Document, o: &Object, depth: usize, budget: &mut usize) -> Option<Object> {
let held;
let (base, followed) = match o {
Object::Ref(_) => {
held = doc.resolve(o);
let cost = match &*held {
Object::Stream(_) | Object::Ref(_) => return None,
Object::Array(a) => a.len().max(1),
Object::Dict(d) => d.len().max(1),
_ => 1,
};
*budget = budget.checked_sub(cost)?;
(&*held, true)
}
other => (other, false),
};
let Some(next) = depth.checked_sub(1) else { return followed.then(|| base.clone()) };
match base {
Object::Array(items) => {
let mut changed: Option<Vec<Object>> = None;
for (i, item) in items.iter().enumerate() {
if let Some(direct) = direct_value(doc, item, next, budget) {
let copy = changed.get_or_insert_with(|| items.clone());
if let Some(slot) = copy.get_mut(i) {
*slot = direct;
}
}
}
changed.map(Object::Array).or_else(|| followed.then(|| base.clone()))
}
Object::Dict(entries) => {
let mut changed: Option<Dict> = None;
for (key, value) in entries.iter() {
if let Some(direct) = direct_value(doc, value, next, budget) {
changed.get_or_insert_with(|| entries.clone()).set(key.clone(), direct);
}
}
changed.map(Object::Dict).or_else(|| followed.then(|| base.clone()))
}
_ => followed.then(|| base.clone()),
}
}
/// Store a built appearance on the annotation: the cheap half of [`set_appearance`].
fn apply_appearance(doc: &mut Document, r: ObjRef, d: &Dict, stream: Stream) -> Result<(), AnnotError> {
// Characters the standard fonts can't draw came out as `?`: mark the annotation with the first.
let missing = appearance::undrawable_text(d).map(|(_, c)| c);
// Read like the builder read it, references followed.
let direct = with_direct_values(doc, d);
let missing = appearance::undrawable_text(&direct).map(|(_, c)| c);
let ap = doc.add(Object::Stream(stream));
// A copy (a shared /AP is left alone) that keeps unknown entries; the old down and rollover
// appearances would show the previous look on press or hover, so they go with the old /N.
@@ -71,7 +150,7 @@ fn apply_appearance(doc: &mut Document, r: ObjRef, d: &Dict, stream: Stream) ->
// The appearance's box is mapped onto `/Rect`, so any other `/Rect` (a large one, or one with
// no width, as LaTeX `pdfcomment` writes) would stretch the icon or shrink it to nothing.
let icon_rect = matches!(d.name(b"Subtype"), Some(b"Text" | b"FileAttachment"))
.then(|| d.get(b"Rect")?.as_array()?.iter().map(|o| o.as_f64()).collect::<Option<Vec<f64>>>())
.then(|| direct.get(b"Rect")?.as_array()?.iter().map(|o| o.as_f64()).collect::<Option<Vec<f64>>>())
.flatten()
.filter(|q| q.len() == 4 && finite(q))
.map(|q| {
+3
View File
@@ -238,6 +238,9 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option<Props> {
let c: Vec<f64> = d.get(b"C").and_then(|o| o.as_array().map(|a| a.iter().filter_map(|x| x.as_f64()).collect())).unwrap_or_default();
let color = if subtype == "FreeText" { Some(appearance::parse_da(d).0) } else { (c.len() == 3).then(|| [c[0], c[1], c[2]]) };
let width = matches!(subtype.as_str(), "Square" | "Circle" | "Line" | "Ink" | "Polygon" | "PolyLine" | "FreeText").then(|| {
// A border style given by reference (`/BS 5 0 R`) reads as the display draws it.
let direct = crate::draw::with_direct_values(doc, d);
let d = direct.as_ref();
if subtype == "FreeText" {
if !d.contains(b"BS") && !d.contains(b"Border") { 0.0 } else { appearance::border_width(d) }
} else {
+7 -3
View File
@@ -730,8 +730,12 @@ pub fn set_style(doc: &mut Document, page: usize, index: usize, change: StyleCha
// So does a text box inside an oval or a circle, which goes around it again.
let shape = if free_text { BorderShape::of(&d) } else { BorderShape::Rectangle };
let typewriter = free_text && (d.name(b"IT") == Some(b"FreeTextTypeWriter") || shape != BorderShape::Rectangle) && !d.contains(b"CL");
// A border style given by reference (`/BS 5 0 R`, ISO 32000-2 §7.3.10): its width and other
// entries, which a new width keeps.
let direct = crate::draw::with_direct_values(doc, &d);
let (old_width, old_bs) = (border_width_of(&direct), direct.get(b"BS").and_then(Object::as_dict).cloned());
let regrow = width.filter(|_| typewriter).and_then(|w| {
let by = w.max(0.0) - if d.contains(b"BS") || d.contains(b"Border") { border_width_of(&d) } else { 0.0 };
let by = w.max(0.0) - if d.contains(b"BS") || d.contains(b"Border") { old_width } else { 0.0 };
let rect = grow(text_box_of(&d)?, by);
(by != 0.0 && finite(&rect) && rect[2] - rect[0] >= 1.0 && rect[3] - rect[1] >= 1.0).then_some((rect, w.max(0.0)))
});
@@ -755,9 +759,9 @@ pub fn set_style(doc: &mut Document, page: usize, index: usize, change: StyleCha
set_part_keys(d, appearance::PartOpacity { line: o, fill: o, text: o });
}
if let Some(w) = width {
let old_w = border_width_of(d);
let old_w = old_width;
let w = w.max(0.0);
let mut bs = d.get(b"BS").and_then(|b| b.as_dict()).cloned().unwrap_or_default();
let mut bs = old_bs.unwrap_or_default();
bs.set(b"W".to_vec(), Object::Real(w));
d.set(b"BS".to_vec(), Object::Dict(bs));
d.remove(b"Border");
+98
View File
@@ -1461,6 +1461,104 @@ fn malformed_optional_interior_color_does_not_hide_shape_strokes() {
assert!(list(&doc, 0)[0].get(b"AP").is_none(), "display generation leaves the source document unchanged");
}
/// Any value may be given by reference (ISO 32000-2 §7.3.10). A Square whose border style
/// dictionary (Table 168), colour and dash array are indirect objects is drawn with them, not
/// with the 1 pt solid default; a reference cycle inside `/BS` is not followed for ever.
#[test]
fn indirect_border_style_and_colour_are_drawn() {
let doc = document_of(&[
b"<< /Type /Catalog /Pages 2 0 R >>",
b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>",
b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>",
b"<< /Type /Annot /Subtype /Square /P 3 0 R /Rect [20 20 180 180] /BS 5 0 R /C 6 0 R >>",
b"<< /Type /Border /W 7 0 R /S /D /D 8 0 R /Self 5 0 R >>",
b"[0 0 1]",
b"6",
b"[4 2]",
]);
let display = with_missing_appearances(&doc).expect("the square gets a display appearance");
let shown = list(&display, 0).remove(0);
let content = ap_content(&display, &shown);
assert!(content.contains("0 0 1 RG\n6 w\n[4 2] 0 d\n"), "the indirect /BS, /W, /D and /C are used: {content}");
assert!(content.contains("23 23 154 154 re"), "the 6 pt border is inset by half its width: {content}");
assert_eq!(shown.get(b"BS"), Some(&Object::Ref(ObjRef::new(5, 0))), "the display copy keeps the reference");
assert_eq!(shown.get(b"P"), Some(&Object::Ref(ObjRef::new(3, 0))));
let props = props(&doc, 0, 0).expect("props");
assert_eq!(props.width, Some(6.0), "the properties show the width drawn");
}
/// Following references copies a bounded number of values: an indirect array past the budget
/// stays a reference (and the comment is not drawn from it), a chain that never ends is not
/// followed, and links to other objects are never copied.
#[test]
fn direct_values_are_bounded() {
let mut doc = fixture();
let huge = doc.add(Object::Array(vec![Object::Int(0); (1 << 16) + 1]));
let chain = doc.add(Object::Null);
doc.set(chain, Object::Ref(chain));
let mut d = Dict::new();
d.set(b"Subtype".to_vec(), Object::name("Square"));
d.set(b"Rect".to_vec(), Object::Ref(doc.add(num_array(&[0.0, 0.0, 10.0, 10.0]))));
d.set(b"Vertices".to_vec(), Object::Ref(huge));
d.set(b"IC".to_vec(), Object::Ref(chain));
d.set(b"Popup".to_vec(), Object::Ref(huge));
let direct = crate::draw::with_direct_values(&doc, &d);
assert_eq!(direct.get(b"Rect").and_then(Object::as_array).map(Vec::len), Some(4), "a small array is followed");
assert_eq!(direct.get(b"Vertices"), Some(&Object::Ref(huge)), "over the budget: left as a reference");
assert_eq!(direct.get(b"IC"), Some(&Object::Ref(chain)), "a chain of references is left alone");
assert_eq!(direct.get(b"Popup"), Some(&Object::Ref(huge)), "a link is not copied");
let mut plain = Dict::new();
plain.set(b"W".to_vec(), Object::Int(2));
assert!(matches!(crate::draw::with_direct_values(&doc, &plain), std::borrow::Cow::Borrowed(_)), "nothing to follow: no copy");
}
/// A new width for a comment whose border style is an indirect object (§7.3.10) keeps that
/// dictionary's other entries (a dashed style stays dashed) and leaves the shared object alone.
#[test]
fn new_width_keeps_an_indirect_border_style() {
let mut doc = document_of(&[
b"<< /Type /Catalog /Pages 2 0 R >>",
b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>",
b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>",
b"<< /Type /Annot /Subtype /Square /Rect [20 20 180 180] /BS 5 0 R /C [1 0 0] >>",
b"<< /W 6 /S /D /D [4 2] >>",
]);
set_style(&mut doc, 0, 0, StyleChange::new(None, None, Some(3.0), None), &meta("")).unwrap();
let doc = reopen(&doc);
let d = list(&doc, 0).remove(0);
let bs = d.get(b"BS").and_then(Object::as_dict).expect("a direct /BS");
assert_eq!(bs.get(b"W").and_then(Object::as_f64), Some(3.0));
assert_eq!(bs.name(b"S"), Some(b"D".as_slice()), "the dashed style is kept");
assert!(ap_content(&doc, &d).contains("3 w\n[4 2] 0 d\n"));
let shared = doc.get(ObjRef::new(5, 0));
assert_eq!(shared.as_dict().and_then(|b| b.get(b"W")).and_then(Object::as_f64), Some(6.0), "the shared object is unchanged");
}
/// `/RD` is optional (ISO 32000-2 Table 177): a callout without it has its text box at `/Rect`
/// and is drawn, box, text and leader line. A callout line and `/RD` given by reference are read too.
#[test]
fn callout_without_rd_is_drawn() {
let doc = document_of(&[
b"<< /Type /Catalog /Pages 2 0 R >>",
b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 400 400] >>",
b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>",
b"<< /Type /Annot /Subtype /FreeText /IT /FreeTextCallout /Rect [100 100 300 160] /CL [20 20 60 60 100 130] /LE /OpenArrow /C [1 1 0] /DA (0 g /Helv 12 Tf) /Contents (callout) >>",
b"<< /Type /Annot /Subtype /FreeText /IT /FreeTextCallout /Rect [10 200 300 300] /CL 7 0 R /RD 8 0 R /C [1 1 0] /DA (0 g /Helv 12 Tf) /Contents (indirect) >>",
b"<< /Type /Annot /Subtype /FreeText /IT /FreeTextCallout /Rect [10 300 300 390] /CL [5 305 10 350] /RD [1 2] /C [1 1 0] /DA (0 g /Helv 12 Tf) /Contents (bad) >>",
b"[15 205 40 250 100 250]",
b"[90 0 0 0]",
]);
let display = with_missing_appearances(&doc).expect("the callouts get a display appearance");
let shown = list(&display, 0);
let plain = ap_content(&display, &shown[0]);
assert!(plain.contains("1 1 0 rg\n100 100 200 60 re f"), "the yellow box fills /Rect: {plain}");
assert!(plain.contains("20 20 m\n60 60 l\n100 130 l\nS"), "the leader line: {plain}");
assert!(plain.contains("(callout) Tj"), "the text: {plain}");
let indirect = ap_content(&display, &shown[1]);
assert!(indirect.contains("100 200 200 100 re f") && indirect.contains("15 205 m"), "indirect /CL and /RD: {indirect}");
assert!(shown[2].get(b"AP").is_none(), "a malformed /RD is still not guessed at");
}
/// #3175: viewers draw a note without an appearance as a fixed-size icon at the upper-left corner
/// of its `/Rect`. The display copy must not stretch the icon over a large `/Rect` or shrink it
/// to nothing in one with no width.
+2 -2
View File
@@ -818,8 +818,8 @@ milestone = "M2"
status = "shipped"
commands = []
tools = []
evidence = ["crates/render/src/raster/tests.rs::widget_appearance_states", "crates/render/src/raster/tests.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", "crates/engine/src/tests.rs::buttons_and_combs_left_to_the_viewer_are_displayed", "crates/forms/src/tests.rs::check_boxes_without_an_appearance_show_their_caption_symbol", "crates/forms/src/tests.rs::push_buttons_without_an_appearance_show_their_caption_and_frame", "crates/forms/src/tests.rs::need_appearances_draws_check_boxes_and_radio_buttons_from_their_captions", "crates/engine/src/tests.rs::need_appearances_buttons_are_displayed_from_their_captions", "crates/render/src/raster/tests.rs::invisible_unknown_annotations_and_popups_paint_nothing", "crates/print/src/tests.rs::invisible_annotations_of_unknown_types_do_not_print", "crates/engine/src/tests.rs::stamps_without_appearances_draw_the_stamp_they_name", "crates/automation/tests/it/automation.rs::page_render_draws_stamps_without_an_appearance"]
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 stamp from another program draws the stamp its /Name names (the fourteen standard names of ISO 32000-2 Table 184, Draft when there is none) or, for any other name, a grey frame with both diagonals; 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); check boxes and radio buttons with no appearance for their /AS are drawn display-only from /MK (background, border, and when on the /MK /CA ZapfDingbats symbol in the /DA colour, a check or a dot by default), and when the form sets /NeedAppearances so is every one that has an /MK, over the appearance the file carries (ISO 32000-2 Table 224; one without /MK keeps its own), and push buttons with none from /MK and /BS with their caption; Link borders without /AP and printing are not covered yet; an annotation of a non-standard type with the Invisible flag (ISO 32000-2 Table 167) is neither drawn nor printed, and a popup's own /AP is never painted on the page (its parent's text shows in the popup window, §12.5.6.14)"
evidence = ["crates/render/src/raster/tests.rs::widget_appearance_states", "crates/render/src/raster/tests.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", "crates/engine/src/tests.rs::buttons_and_combs_left_to_the_viewer_are_displayed", "crates/forms/src/tests.rs::check_boxes_without_an_appearance_show_their_caption_symbol", "crates/forms/src/tests.rs::push_buttons_without_an_appearance_show_their_caption_and_frame", "crates/forms/src/tests.rs::need_appearances_draws_check_boxes_and_radio_buttons_from_their_captions", "crates/engine/src/tests.rs::need_appearances_buttons_are_displayed_from_their_captions", "crates/render/src/raster/tests.rs::invisible_unknown_annotations_and_popups_paint_nothing", "crates/print/src/tests.rs::invisible_annotations_of_unknown_types_do_not_print", "crates/engine/src/tests.rs::stamps_without_appearances_draw_the_stamp_they_name", "crates/automation/tests/it/automation.rs::page_render_draws_stamps_without_an_appearance", "crates/annot/src/tests.rs::indirect_border_style_and_colour_are_drawn", "crates/annot/src/tests.rs::callout_without_rd_is_drawn"]
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, with style values given by reference (a /BS, colour, /RD or /CL as an indirect object, ISO 32000-2 §7.3.10) followed and a callout without /RD (optional, Table 177) boxed at its /Rect; a stamp from another program draws the stamp its /Name names (the fourteen standard names of ISO 32000-2 Table 184, Draft when there is none) or, for any other name, a grey frame with both diagonals; 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); check boxes and radio buttons with no appearance for their /AS are drawn display-only from /MK (background, border, and when on the /MK /CA ZapfDingbats symbol in the /DA colour, a check or a dot by default), and when the form sets /NeedAppearances so is every one that has an /MK, over the appearance the file carries (ISO 32000-2 Table 224; one without /MK keeps its own), and push buttons with none from /MK and /BS with their caption; Link borders without /AP and printing are not covered yet; an annotation of a non-standard type with the Invisible flag (ISO 32000-2 Table 167) is neither drawn nor printed, and a popup's own /AP is never painted on the page (its parent's text shows in the popup window, §12.5.6.14)"
[[feature]]
id = "view.render-error-isolation"