From 6e0d0fde3f7592567bf05fd78e34321aed1209df Mon Sep 17 00:00:00 2001 From: Josias Eli Rodriguez <339499413+chamber074@users.noreply.github.com> Date: Sat, 7 Nov 2026 03:04:51 -0600 Subject: [PATCH 1/2] redact: treat unusable /QuadPoints as absent in every reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A quad counts only when all 8 entries are present, numeric and finite. Anything else contributes nothing — never folded, shifted or zero-filled into a box the document does not describe. With no usable quad the existing /Rect fallback runs, and with neither the mark stays unreadable. The rule lives in one helper, geom::quad_points::usable_quads, and redact, render-inspect, annot summaries and annot appearance drawing all read through it, so the outline shown is the area apply removes. Redact also follows indirect numbers in /QuadPoints now, which it used to drop. --- Cargo.lock | 3 + crates/annot/Cargo.toml | 2 + crates/annot/src/appearance.rs | 16 ++- crates/annot/src/info.rs | 11 +- crates/annot/src/tests.rs | 65 ++++++++++ crates/geom/src/lib.rs | 1 + crates/geom/src/quad_points.rs | 74 +++++++++++ crates/redact/Cargo.toml | 2 + crates/redact/src/lib.rs | 19 +-- crates/redact/src/tests.rs | 156 +++++++++++++++++++++++ crates/render/Cargo.toml | 4 +- crates/render/src/inspect/annotations.rs | 6 +- crates/render/src/inspect/tests.rs | 20 +++ 13 files changed, 363 insertions(+), 16 deletions(-) create mode 100644 crates/geom/src/quad_points.rs diff --git a/Cargo.lock b/Cargo.lock index 8617c45522..78fb437353 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4267,6 +4267,7 @@ dependencies = [ "hayro-syntax", "pdfcraft-cos", "pdfcraft-fonts", + "pdfcraft-geom", "pdfcraft-model", "thiserror 2.0.21", ] @@ -4627,6 +4628,7 @@ dependencies = [ "pdfcraft-filters", "pdfcraft-fonts", "pdfcraft-forms", + "pdfcraft-geom", "pdfcraft-model", "pdfcraft-render", "pdfcraft-testkit", @@ -4649,6 +4651,7 @@ dependencies = [ "lopdf", "pdfcraft-cos", "pdfcraft-fonts", + "pdfcraft-geom", "serde", "skrifa 0.44.0", "thiserror 2.0.21", diff --git a/crates/annot/Cargo.toml b/crates/annot/Cargo.toml index 8a31381493..e91c9ed896 100644 --- a/crates/annot/Cargo.toml +++ b/crates/annot/Cargo.toml @@ -11,6 +11,8 @@ workspace = true [dependencies] pdfcraft-cos = { workspace = true } pdfcraft-fonts = { workspace = true } +# The usable-quad rule `/QuadPoints` readers share. +pdfcraft-geom = { workspace = true } pdfcraft-model = { workspace = true } thiserror = { workspace = true } diff --git a/crates/annot/src/appearance.rs b/crates/annot/src/appearance.rs index 7148e9fc2e..c2b61eeba3 100644 --- a/crates/annot/src/appearance.rs +++ b/crates/annot/src/appearance.rs @@ -166,6 +166,14 @@ fn nums(d: &Dict, key: &[u8]) -> Option> { d.get(key)?.as_array()?.iter().map(|o| o.as_f64()).collect() } +/// The usable `/QuadPoints` quads, by the rule redaction apply reads them with +/// ([`pdfcraft_geom::quad_points::usable_quads`]): a malformed or non-finite quad is not drawn (it +/// would write `NaN` into the stream) and the others still are. `None` when none is usable. +fn quads(d: &Dict) -> Option> { + let q = pdfcraft_geom::quad_points::usable_quads(d.get(b"QuadPoints")?.as_array()?.iter().map(Object::as_f64)); + (!q.is_empty()).then_some(q) +} + /// `/C`-style colour arrays: none (transparent), gray, RGB or CMYK. fn color(d: &Dict, key: &[u8]) -> Option> { let c = match d.get(key) { @@ -565,9 +573,9 @@ fn draw_markup(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) let subtype = context.subtype; let stroke = context.stroke; let c = &mut drawing.content; - let q = nums(d, b"QuadPoints").filter(|q| !q.is_empty() && q.len() % 8 == 0)?; + let q = quads(d)?; let col = stroke?; - for quad in q.as_chunks::<8>().0.iter() { + for quad in q.iter() { let p = |i: usize| (quad[2 * i], quad[2 * i + 1]); let (p1, p2, p3, p4) = (p(0), p(1), p(2), p(3)); let h = (p1.0 - p3.0).hypot(p1.1 - p3.1); @@ -612,10 +620,10 @@ fn draw_redaction(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawi let stroke = context.stroke; let c = &mut drawing.content; // While marked: the outline of each area (the fill comes when applied). - let q = nums(d, b"QuadPoints").filter(|q| !q.is_empty() && q.len() % 8 == 0)?; + let q = quads(d)?; let col = stroke.unwrap_or([0.89, 0.13, 0.13]); c.push_str(&format!("{}1 w\n", rg_stroke(col))); - for quad in q.as_chunks::<8>().0.iter() { + for quad in q.iter() { c.push_str(&format!( "{} {} m {} {} l {} {} l {} {} l h S\n", n(quad[0]), diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index ba181d7504..6696ad4b0f 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -117,7 +117,16 @@ pub fn summaries(doc: &Document) -> Vec { color = Some([f(i - 3), f(i - 2), f(i - 1)]); } } - let quads = nums(b"QuadPoints").as_chunks::<8>().0.to_vec(); + // Only usable quads, by the rule redaction apply reads them with, so a mark's outline + // is the area apply removes (and with none, the `/Rect` both fall back to). + let quads: Vec<[f32; 8]> = d + .get(b"QuadPoints") + .map(|o| doc.resolve(o)) + .map(|a| { + let entries = a.as_array().into_iter().flatten().map(|x| doc.resolve(x).as_f64()); + pdfcraft_geom::quad_points::usable_quads(entries).into_iter().map(|q| q.map(|v| v as f32)).collect() + }) + .unwrap_or_default(); // Keep in step with `pdfcraft_render::inspect`. let points = match subtype.as_str() { "Line" => Some(nums(b"L")).filter(|l| l.len() == 4).map(|l| l.as_chunks::<2>().0.to_vec()).unwrap_or_default(), diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index ff7f8925f0..d9983b8b41 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2779,3 +2779,68 @@ fn indirect_typewriter_names_keep_outer_whitespace_in_summaries() { check(&doc); check(&reopen(&doc)); } + +const QUAD: [f64; 8] = [20.0, 110.0, 40.0, 110.0, 20.0, 95.0, 40.0, 95.0]; + +fn reals(v: &[f64]) -> Vec { + v.iter().map(|x| Object::Real(*x)).collect() +} + +/// The appearance a `subtype` with `/QuadPoints` `quads` draws, as text. +fn quad_appearance(subtype: &str, quads: Vec) -> Option { + let mut d = Dict::new(); + d.set(b"Subtype".to_vec(), Object::name(subtype)); + d.set(b"Rect".to_vec(), Object::Array([20, 95, 40, 110].map(Object::Int).to_vec())); + d.set(b"C".to_vec(), Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)])); + d.set(b"QuadPoints".to_vec(), Object::Array(quads)); + appearance::build(&d).map(|s| String::from_utf8_lossy(&s.decoded().unwrap()).into_owned()) +} + +/// Redaction outlines and text markup draw the quads redaction apply would use, no more: a NaN +/// quad wrote `NaN` into the stream, and one non-number or a trailing partial quad left the other, +/// usable quads undrawn. +#[test] +fn appearances_draw_only_usable_quads() { + let mut one_nan = QUAD; + one_nan[3] = f64::NAN; + for subtype in ["Redact", "Highlight"] { + let clean = quad_appearance(subtype, reals(&QUAD)).unwrap(); + assert!(!clean.contains("NaN"), "{clean}"); + let mut name_first = reals(&[QUAD, QUAD].concat()); + name_first[2] = Object::name("oops"); + let same = [ + reals(&[one_nan, QUAD].concat()), + name_first, + reals(&[&QUAD[..], &QUAD[..4]].concat()), + reals(&[QUAD, [f64::INFINITY; 8]].concat()), + ]; + for (i, quads) in same.into_iter().enumerate() { + assert_eq!(quad_appearance(subtype, quads).as_deref(), Some(clean.as_str()), "{subtype} case {i}"); + } + // No usable quad draws nothing, as with no /QuadPoints. + assert_eq!(quad_appearance(subtype, reals(&[f64::NAN; 8])), None, "{subtype}"); + } +} + +/// The comment list reads `/QuadPoints` by the rule redaction apply does: unusable quads are left +/// out, not read as zeros, and an indirect number is followed. +#[test] +fn summaries_list_only_usable_quads() { + let mut doc = fixture(); + let forty = Object::Ref(doc.add(Object::Int(40))); + let page = page_refs(&doc).unwrap()[0]; + let mut entries = annots(&doc, page); + let Object::Dict(annotation) = &mut entries[0] else { panic!("fixture annotation is inline") }; + // A non-number in the first quad, an indirect 40 in the second, NaN in the third, and four + // entries of a fourth. + let mut quads = reals(&[QUAD, QUAD, QUAD, QUAD].concat()); + quads[1] = Object::name("oops"); + quads[10] = forty; + quads[17] = Object::Real(f64::NAN); + quads.truncate(28); + annotation.set(b"Subtype".to_vec(), Object::name("Redact")); + annotation.set(b"QuadPoints".to_vec(), Object::Array(quads)); + set_annots(&mut doc, page, entries).unwrap(); + let summary = summaries(&doc).into_iter().find(|a| a.page == 0 && a.index == 0).unwrap(); + assert_eq!(summary.quads, vec![QUAD.map(|v| v as f32)]); +} diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs index 40f38498af..94ac7cdcc6 100644 --- a/crates/geom/src/lib.rs +++ b/crates/geom/src/lib.rs @@ -6,6 +6,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] pub mod perspective; +pub mod quad_points; /// An axis-aligned rectangle in PDF user space (points). #[derive(Clone, Copy, Debug, PartialEq, Default)] diff --git a/crates/geom/src/quad_points.rs b/crates/geom/src/quad_points.rs new file mode 100644 index 0000000000..5acba5d0dc --- /dev/null +++ b/crates/geom/src/quad_points.rs @@ -0,0 +1,74 @@ +//! The quadrilaterals of a `/QuadPoints` array (ISO 32000-2 §12.5.6.10), read by one rule. +//! +//! Redaction apply, the comment summaries and outlines, the inspector and generated appearances +//! all read `/QuadPoints`; they go through [`usable_quads`] so the area a user sees marked is the +//! area apply removes. + +/// The usable quadrilaterals of a `/QuadPoints` array, given its entries in order (`None` for an +/// entry that is not a number). +/// +/// A quadrilateral is usable only when all eight of its entries are present, numbers, and within +/// the range of a PDF real (finite as an `f32`, Annex C), so the readers that keep `f32` see the +/// same set as those that keep `f64`. An unusable one is skipped whole: the ones after it keep +/// their coordinates, and it never becomes a folded box or zeros. A trailing partial group is +/// dropped. +pub fn usable_quads>>(entries: I) -> Vec<[f64; 8]> { + let entries: Vec> = entries.into_iter().collect(); + entries + .as_chunks::<8>() + .0 + .iter() + .filter_map(|q| { + let mut quad = [0.0; 8]; + for (slot, v) in quad.iter_mut().zip(q) { + // NaN and infinities stay non-finite as `f32`; so does a value past its range. + *slot = v.filter(|v| (*v as f32).is_finite())?; + } + Some(quad) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::usable_quads; + + const A: [f64; 8] = [10.0, 20.0, 50.0, 20.0, 10.0, 40.0, 50.0, 40.0]; + const B: [f64; 8] = [60.0, 70.0, 90.0, 70.0, 60.0, 80.0, 90.0, 80.0]; + + fn some(v: &[f64]) -> Vec> { + v.iter().copied().map(Some).collect() + } + + #[test] + fn finite_quads_are_all_usable() { + assert_eq!(usable_quads(some(&[A, B].concat())), vec![A, B]); + assert!(usable_quads(Vec::new()).is_empty()); + } + + #[test] + fn an_all_nan_quad_is_unusable() { + assert!(usable_quads(some(&[f64::NAN; 8])).is_empty()); + } + + #[test] + fn one_non_finite_entry_makes_its_quad_unusable() { + for bad in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY, 1e300] { + let mut q = A; + q[3] = bad; + assert_eq!(usable_quads(some(&[q, B].concat())), vec![B], "{bad}"); + } + } + + #[test] + fn a_non_number_drops_its_quad_without_shifting_the_next() { + let mut entries = some(&[A, B].concat()); + entries[2] = None; + assert_eq!(usable_quads(entries), vec![B]); + } + + #[test] + fn a_trailing_partial_group_is_dropped() { + assert_eq!(usable_quads(some(&[&A[..], &B[..4]].concat())), vec![A]); + } +} diff --git a/crates/redact/Cargo.toml b/crates/redact/Cargo.toml index 149d8d8295..c55d108440 100644 --- a/crates/redact/Cargo.toml +++ b/crates/redact/Cargo.toml @@ -13,6 +13,8 @@ aho-corasick = "1" pdfcraft-cos = { workspace = true } pdfcraft-content = { workspace = true } pdfcraft-fonts = { workspace = true } +# The usable-quad rule `/QuadPoints` readers share. +pdfcraft-geom = { workspace = true } pdfcraft-model = { workspace = true } pdfcraft-forms = { workspace = true } pdfcraft-edit = { workspace = true } diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 6d323004ad..e3b8e4f354 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -184,6 +184,14 @@ pub(crate) fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { (v.len() == 4 && v.iter().all(|x| x.is_finite())).then(|| [v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3])]) } +/// A mark's usable `/QuadPoints` quads ([`pdfcraft_geom::quad_points::usable_quads`]), the set the +/// comment outlines show. Each one is finite, so the box it folds to is a real one. +fn quad_points(doc: &Document, d: &Dict) -> Vec<[f64; 8]> { + let Some(q) = d.get(b"QuadPoints").map(|q| doc.resolve(q)) else { return Vec::new() }; + let Some(a) = q.as_array() else { return Vec::new() }; + pdfcraft_geom::quad_points::usable_quads(a.iter().map(|x| doc.resolve(x).as_f64())) +} + fn color(doc: &Document, d: &Dict, key: &[u8]) -> Option { let v: Vec = doc.resolve(d.get(key)?).as_array()?.iter().filter_map(Object::as_f64).collect(); match v.len() { @@ -228,13 +236,7 @@ pub(crate) fn marks_checked(doc: &Document) -> (Vec, Vec) { if d.name(b"Subtype") != Some(b"Redact") { continue; } - let quads: Vec = d - .get(b"QuadPoints") - .and_then(|q| doc.resolve(q).as_array().map(|a| a.iter().filter_map(Object::as_f64).collect())) - .unwrap_or_default(); - let mut rects: Vec<[f64; 4]> = quads - .as_chunks::<8>() - .0 + let mut rects: Vec<[f64; 4]> = quad_points(doc, d) .iter() .map(|q| { let xs = [q[0], q[2], q[4], q[6]]; @@ -246,8 +248,9 @@ pub(crate) fn marks_checked(doc: &Document) -> (Vec, Vec) { ys.iter().copied().fold(f64::MIN, f64::max), ] }) - .filter(|r| r.iter().all(|v| v.is_finite())) .collect(); + // No usable quad (all absent, malformed or non-finite): the mark's `/Rect`, else it + // is unreadable and apply refuses it. if rects.is_empty() && let Some(r) = rect_of(doc, d.get(b"Rect")) { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 8978e0e7d6..51c51eca62 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2258,6 +2258,162 @@ fn an_unreadable_mark_on_another_page_does_not_stop_the_selected_page() { assert!(!content(&doc, 0).contains("1234")); } +/// The quad over "1234" in `AB1234CD` (x 20–40) and the one over "CD" (x 40–50). +const OVER_1234: [f64; 8] = [20.0, 110.0, 40.0, 110.0, 20.0, 95.0, 40.0, 95.0]; +const OVER_CD: [f64; 8] = [40.0, 110.0, 50.0, 110.0, 40.0, 95.0, 50.0, 95.0]; + +/// `AB1234CD` with one mark whose `/QuadPoints` is `quads`, whose `/Rect` is `rect` (removed when +/// `None`) and which has a fill (`/IC`) only when `fill` is set. +fn quad_mark(quads: Vec, rect: Option<[i64; 4]>, fill: bool) -> (Document, ObjRef) { + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (AB1234CD) Tj ET", "", vec![]); + mark(&mut doc, 0, &[[20.0, 95.0, 40.0, 110.0]], ""); + let m = marks(&doc).swap_remove(0); + doc.update_dict(m.obj, |d| { + d.set(b"QuadPoints".to_vec(), Object::Array(quads)); + if let Some(r) = rect { + d.set(b"Rect".to_vec(), Object::Array(r.iter().map(|v| Object::Int(*v)).collect())); + } else { + d.remove(b"Rect"); + } + if !fill { + d.remove(b"IC"); + } + }) + .unwrap(); + (doc, m.obj) +} + +fn reals(v: &[f64]) -> Vec { + v.iter().map(|x| Object::Real(*x)).collect() +} + +/// An all-NaN quad folded to `[MAX, MAX, MIN, MIN]`: a finite, inverted box that passed as a +/// readable area. Without a fill the mark was applied and removed nothing; with one the overlay +/// failed. With no `/Rect` either, the mark has no area and apply refuses it. +#[test] +fn an_all_nan_quad_without_a_rect_is_refused_not_applied_as_nothing() { + for fill in [false, true] { + let (mut doc, _) = quad_mark(reals(&[f64::NAN; 8]), None, fill); + assert!(marks(&doc).is_empty(), "fill {fill}: the NaN quad is no area"); + assert_eq!(apply(&mut doc, None), Err(RedactError::UnreadableMark(1)), "fill {fill}"); + assert!(content(&doc, 0).contains("(AB1234CD) Tj"), "fill {fill}: nothing changed"); + } +} + +/// With a usable `/Rect`, an all-NaN mark redacts exactly the `/Rect`. +#[test] +fn an_all_nan_quad_falls_back_to_the_rect() { + for fill in [false, true] { + let (mut doc, _) = quad_mark(reals(&[f64::NAN; 8]), Some([20, 95, 40, 110]), fill); + assert_eq!(marks(&doc)[0].rects, vec![[20.0, 95.0, 40.0, 110.0]], "fill {fill}"); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.marks, r.glyphs), (1, 4), "fill {fill}"); + assert!(!content(&doc, 0).contains("1234"), "fill {fill}"); + } +} + +/// One NaN among finite entries made `f64::min`/`max` ignore it and fold the other three corners: +/// a quad the document never described. Such a quad is unusable. +#[test] +fn a_quad_with_one_nan_entry_is_unusable() { + let mut q = OVER_1234; + q[0] = f64::NAN; + let (mut doc, _) = quad_mark(reals(&q), None, false); + assert!(marks(&doc).is_empty()); + assert_eq!(apply(&mut doc, None), Err(RedactError::UnreadableMark(1))); + // Next to a usable quad, only that one applies. + let (mut doc, _) = quad_mark(reals(&[q, OVER_CD].concat()), None, false); + assert_eq!(marks(&doc)[0].rects, vec![[40.0, 95.0, 50.0, 110.0]]); + assert_eq!(apply(&mut doc, None).map(|r| r.glyphs), Ok(2)); +} + +/// A name in the middle of `/QuadPoints` was dropped, which moved every later coordinate into the +/// wrong slot and folded the first quad to a box over most of the page. +#[test] +fn a_non_number_entry_drops_its_quad_without_shifting_the_rest() { + let mut quads = reals(&[OVER_1234, OVER_CD].concat()); + quads[2] = Object::name("oops"); + let (mut doc, _) = quad_mark(quads, None, false); + assert_eq!(marks(&doc)[0].rects, vec![[40.0, 95.0, 50.0, 110.0]]); + assert_eq!(apply(&mut doc, None).map(|r| r.glyphs), Ok(2)); +} + +/// Twelve entries are one quad and half of another: only the whole one applies. +#[test] +fn a_trailing_partial_quad_is_ignored() { + let (mut doc, _) = quad_mark(reals(&[&OVER_1234[..], &OVER_CD[..4]].concat()), None, false); + assert_eq!(marks(&doc)[0].rects, vec![[20.0, 95.0, 40.0, 110.0]]); + assert_eq!(apply(&mut doc, None).map(|r| r.glyphs), Ok(4)); +} + +/// Finite quads redact what they always did, one box each, and the `/Rect` is not consulted. +#[test] +fn finite_quads_redact_their_areas_as_before() { + let (mut doc, _) = quad_mark(reals(&[OVER_1234, OVER_CD].concat()), Some([0, 0, 1, 1]), true); + assert_eq!(marks(&doc)[0].rects, vec![[20.0, 95.0, 40.0, 110.0], [40.0, 95.0, 50.0, 110.0]]); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.marks, r.glyphs), (1, 6)); + assert!(!content(&doc, 0).contains("1234")); +} + +/// The boxes a reader's quads outline, or its `/Rect` when it has none (as the comment outlines +/// fall back to it). +fn outline(quads: &[[f32; 8]], rect: [f32; 4]) -> Vec<[f32; 4]> { + if quads.is_empty() { + return vec![rect]; + } + quads + .iter() + .map(|q| { + let (xs, ys) = ([q[0], q[2], q[4], q[6]], [q[1], q[3], q[5], q[7]]); + let (lo, hi) = (|v: [f32; 4]| v.into_iter().fold(f32::MAX, f32::min), |v: [f32; 4]| v.into_iter().fold(f32::MIN, f32::max)); + [lo(xs), lo(ys), hi(xs), hi(ys)] + }) + .collect() +} + +/// The outline the comment list draws (`pdfcraft_annot::summaries`, and the inspector's listing +/// of the saved file) is the area apply removes, for malformed `/QuadPoints` too. The readers +/// used to disagree: the summaries and the inspector read non-numbers as zeros, apply dropped +/// them. +#[test] +fn the_outline_shown_is_the_area_apply_removes() { + // (entries, whether a saved file can say them: PDF syntax has no NaN). `forty` is an indirect + // number, which every reader follows. + let cases = |forty: ObjRef| -> Vec<(Vec, bool)> { + let mut name_inside = reals(&[OVER_1234, OVER_CD].concat()); + name_inside[2] = Object::name("oops"); + let mut by_reference = reals(&OVER_1234); + by_reference[2] = Object::Ref(forty); + let mut one_nan = OVER_1234; + one_nan[5] = f64::NAN; + vec![ + (reals(&[OVER_1234, OVER_CD].concat()), true), + (name_inside, true), + (reals(&[&OVER_1234[..], &OVER_CD[..4]].concat()), true), + (by_reference, true), + (vec![Object::name("oops"); 8], true), + (reals(&[one_nan, OVER_CD].concat()), false), + (reals(&[f64::NAN; 8]), false), + ] + }; + for i in 0.. { + let (mut doc, obj) = quad_mark(Vec::new(), Some([20, 95, 40, 110]), true); + let forty = doc.add(Object::Int(40)); + let Some((quads, saved)) = cases(forty).into_iter().nth(i) else { break }; + doc.update_dict(obj, |d| d.set(b"QuadPoints".to_vec(), Object::Array(quads))).unwrap(); + let applied: Vec<[f32; 4]> = marks(&doc)[0].rects.iter().map(|r| r.map(|v| v as f32)).collect(); + let summary = pdfcraft_annot::summaries(&doc).into_iter().find(|s| s.subtype == "Redact").unwrap(); + assert_eq!(outline(&summary.quads, summary.rect), applied, "case {i}: comment summaries"); + if saved { + let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap(); + let info = pdfcraft_render::inspect(Arc::new(bytes), None).unwrap(); + let listed = info.annotations.iter().find(|a| a.subtype == "Redact").unwrap(); + assert_eq!(outline(&listed.quads, listed.rect), applied, "case {i}: inspector"); + } + } +} + /// A document with something in every hidden-information category. fn hidden_fixture() -> Document { let content = b"BT /F1 10 Tf 10 100 Td (Visible) Tj 3 Tr (Hidden) Tj 0 Tr ET BT /F1 10 Tf 500 500 Td (Offpage) Tj ET /OC /L1 BDC BT /F1 10 Tf 10 50 Td (Layer) Tj ET EMC"; diff --git a/crates/render/Cargo.toml b/crates/render/Cargo.toml index 71467dfeb2..65c94852e6 100644 --- a/crates/render/Cargo.toml +++ b/crates/render/Cargo.toml @@ -11,7 +11,9 @@ workspace = true [dependencies] pdfcraft-cos = { workspace = true } pdfcraft-fonts = { workspace = true } -hayro = { workspace = true } +# The usable-quad rule `/QuadPoints` readers share. +pdfcraft-geom = { workspace = true } +hayro ={ workspace = true } hayro-syntax = { workspace = true } # Picks the face of an installed font collection (the version pdfcraft-fonts already uses). skrifa = "0.44" diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 603aa88110..7c4543b733 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -138,10 +138,12 @@ impl<'a> Inspector<'a> { color = Some([f(i - 3), f(i - 2), f(i - 1)]); } } + // Only usable quads, by the rule redaction apply reads them with (keep in step with + // `pdfcraft_annot::summaries`): a malformed one is not drawn as zeros. let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) { Ok(Object::Array(q)) => { - let v: Vec = q.iter().map(|o| o.as_float().unwrap_or(0.0)).collect(); - v.as_chunks::<8>().0.to_vec() + let entries = q.iter().map(|o| self.resolve(o).as_float().ok().map(f64::from)); + pdfcraft_geom::quad_points::usable_quads(entries).into_iter().map(|quad| quad.map(|v| v as f32)).collect() } _ => Vec::new(), }; diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 7221e97d5a..907401f8bb 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -805,6 +805,26 @@ impl Structure for CountingReads<'_> { } } +/// `/QuadPoints` is read by the rule redaction apply uses (`pdfcraft_geom::quad_points`): a quad +/// with a non-number is left out rather than drawn with a zero, an indirect number is followed, +/// and a trailing partial quad is dropped. +#[test] +fn only_usable_quads_are_listed() { + let pdf = "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 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 200 200] /Annots [5 0 R] >> endobj\n\ + 4 0 obj 40 endobj\n\ + 5 0 obj << /Type /Annot /Subtype /Redact /Rect [20 95 40 110] /QuadPoints \ + [20 /oops 40 110 20 95 40 95 20 110 4 0 R 110 20 95 40 95 20 110 40 110] >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF"; + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let page = PageInfo { width: 200.0, height: 200.0, crop: [0.0, 0.0, 200.0, 200.0], rotation: 0, label: String::new() }; + let mut info = DocInfo { pages: vec![page], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + let mark = info.annotations.iter().find(|a| a.subtype == "Redact").expect("listed"); + assert_eq!(mark.quads, vec![[20.0, 110.0, 40.0, 110.0, 20.0, 95.0, 40.0, 95.0]]); +} + /// One comment referenced many times, or a page-shared `/Annots` array, is listed once, and many /// small comments sharing one large `/Contents` string stop at the retained-bytes budget. #[test] From 65ebdab392d0e5d74691ec924edad742c4b9a48f Mon Sep 17 00:00:00 2001 From: Josias Eli Rodriguez <339499413+chamber074@users.noreply.github.com> Date: Sat, 7 Nov 2026 05:58:15 -0600 Subject: [PATCH 2/2] redact, annot: usable quad values on the f32 grid; appearances resolve indirect quads Review round on #6525 (CodeRabbit, plus eliets2's first-pass finding): - usable_quads returns usable values on the f32 grid, so readers folding f32 and f64 agree on the numbers as well as on the usable set (67108869 is 67108872 for every reader). New geom test pins it. - The end-to-end redact agreement test gains the 4e38 case. - Appearance drawing read /QuadPoints off the raw dictionary, dropping an indirect array or indirect entries the other readers keep. appearance::build now takes the document and resolves both, as redact's reader does; the restyle probes pass the document they already hold. New test pins both indirect shapes against the direct output. - One shipped test block reformatted (rustfmt). Drafted with AI assistance; verified by the contributor, fail-before included. --- crates/annot/src/appearance.rs | 32 ++++++++++-------- crates/annot/src/draw.rs | 2 +- crates/annot/src/info.rs | 2 +- crates/annot/src/modify.rs | 6 ++-- crates/annot/src/tests.rs | 59 +++++++++++++++++++++++----------- crates/geom/src/quad_points.rs | 22 ++++++++++--- crates/redact/src/tests.rs | 3 ++ 7 files changed, 84 insertions(+), 42 deletions(-) diff --git a/crates/annot/src/appearance.rs b/crates/annot/src/appearance.rs index c2b61eeba3..fcb3cb22b5 100644 --- a/crates/annot/src/appearance.rs +++ b/crates/annot/src/appearance.rs @@ -1,10 +1,10 @@ //! Appearance streams (§12.5.5) for the comment types PdfCraft creates, execution plan M5.2. //! -//! [`build`] draws a normal appearance (`/AP /N`) from the annotation dictionary alone, so the -//! same code serves new comments and restyled ones. Drawings are in page space with +//! [`build`] draws a normal appearance (`/AP /N`) from the annotation dictionary and the +//! objects it points at, so the same code serves new comments and restyled ones. Drawings are in page space with //! `/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. +//! endings), so callers never replace an appearance with a worse one. //! //! 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 @@ -14,7 +14,7 @@ use pdfcraft_cos::{Dict, Object, PdfString, Stream}; use pdfcraft_fonts::{first_undrawable, literal, win_ansi}; pub use pdfcraft_fonts::{helvetica_width as text_width, wrap}; -use crate::{NOTE_SIZE, Rgb, n}; +use crate::{Document, NOTE_SIZE, Rgb, n}; mod symbols; @@ -168,9 +168,13 @@ fn nums(d: &Dict, key: &[u8]) -> Option> { /// The usable `/QuadPoints` quads, by the rule redaction apply reads them with /// ([`pdfcraft_geom::quad_points::usable_quads`]): a malformed or non-finite quad is not drawn (it -/// would write `NaN` into the stream) and the others still are. `None` when none is usable. -fn quads(d: &Dict) -> Option> { - let q = pdfcraft_geom::quad_points::usable_quads(d.get(b"QuadPoints")?.as_array()?.iter().map(Object::as_f64)); +/// would write `NaN` into the stream) and the others still are. The array and its entries are +/// resolved first, as the other readers resolve them, so an indirect quad is drawn too. `None` +/// when none is usable. +fn quads(context: &AppearanceContext<'_>) -> Option> { + let doc = context.doc; + let q = + pdfcraft_geom::quad_points::usable_quads(doc.resolve(context.dict.get(b"QuadPoints")?).as_array()?.iter().map(|x| doc.resolve(x).as_f64())); (!q.is_empty()).then_some(q) } @@ -456,6 +460,7 @@ fn caret_layout(d: &Dict, rect: [f64; 4]) -> ([f64; 4], Option<(f64, f64, f64)>) #[derive(Clone, Copy)] struct AppearanceContext<'a> { + doc: &'a Document, dict: &'a Dict, subtype: &'a [u8], rect: [f64; 4], @@ -492,8 +497,8 @@ impl AppearanceDrawing { /// Draw the normal appearance of an annotation, or `None` if this subtype/variant isn't supported. /// Text the standard fonts can't draw comes out as `?`: [`undrawable_text`] says which, and /// [`crate::set_appearance`] marks the annotation with it (`pdfcraft_fonts::UNDRAWN_MARK`). -pub fn build(d: &Dict) -> Option { - let context = appearance_context(d)?; +pub fn build(doc: &Document, d: &Dict) -> Option { + let context = appearance_context(doc, d)?; let drawing = AppearanceDrawing::new(&context); match context.subtype { b"Text" => return draw_icon(&context, "Note", [1.0, 0.82, 0.0], drawing), @@ -507,7 +512,7 @@ pub fn build(d: &Dict) -> Option { finish_appearance(&context, drawing) } -fn appearance_context(d: &Dict) -> Option> { +fn appearance_context<'a>(doc: &'a Document, d: &'a Dict) -> Option> { // Imported measurement appearances may include leaders, captions and formatting that // this builder cannot reproduce. Preserve them rather than silently losing detail. if d.contains(b"Measure") && !d.contains(b"PCMeasureValue") { @@ -522,6 +527,7 @@ fn appearance_context(d: &Dict) -> Option> { return None; } Some(AppearanceContext { + doc, dict: d, subtype, rect, @@ -569,11 +575,10 @@ fn finish_appearance(context: &AppearanceContext<'_>, mut drawing: AppearanceDra } fn draw_markup(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) -> Option<()> { - let d = context.dict; let subtype = context.subtype; let stroke = context.stroke; let c = &mut drawing.content; - let q = quads(d)?; + let q = quads(context)?; let col = stroke?; for quad in q.iter() { let p = |i: usize| (quad[2 * i], quad[2 * i + 1]); @@ -616,11 +621,10 @@ fn draw_markup(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) } fn draw_redaction(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) -> Option<()> { - let d = context.dict; let stroke = context.stroke; let c = &mut drawing.content; // While marked: the outline of each area (the fill comes when applied). - let q = quads(d)?; + let q = quads(context)?; let col = stroke.unwrap_or([0.89, 0.13, 0.13]); c.push_str(&format!("{}1 w\n", rg_stroke(col))); for quad in q.iter() { diff --git a/crates/annot/src/draw.rs b/crates/annot/src/draw.rs index 5e698332da..eac1f09fea 100644 --- a/crates/annot/src/draw.rs +++ b/crates/annot/src/draw.rs @@ -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 { - let mut stream = appearance::build(d)?; + let mut stream = appearance::build(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| { diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index 6696ad4b0f..8af76cb2ed 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -261,7 +261,7 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option { width, icon: (subtype == "Text").then(|| d.name(b"Name").and_then(|n| NoteIcon::from_name(&String::from_utf8_lossy(n))).unwrap_or(NoteIcon::Note)), modified: text_value(doc, d, b"M"), - restylable: appearance::build(d).is_some(), + restylable: appearance::build(doc, d).is_some(), locked: annotation_flags(doc, d) & FLAG_LOCKED != 0, fillable: fillable(&subtype), fill: fill_of(d), diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index c7d87be090..70abf987aa 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -710,7 +710,7 @@ pub fn set_style(doc: &mut Document, page: usize, index: usize, change: StyleCha let d = annot_dict(doc, r); let subtype = String::from_utf8_lossy(d.name(b"Subtype").unwrap_or_default()).into_owned(); // Check before changing anything: a stale appearance would contradict the new style. - if appearance::build(&d).is_none() { + if appearance::build(doc, &d).is_none() { return Err(AnnotError::Unsupported(subtype.clone())); } if color.is_some_and(|c| !finite(&c)) || opacity.is_some_and(|o| !o.is_finite()) || width.is_some_and(|w| !w.is_finite()) { @@ -871,7 +871,7 @@ pub fn set_fill(doc: &mut Document, page: usize, index: usize, fill: Option return Err(AnnotError::Invalid("only rectangles, ovals, polygons and text boxes can be filled".into())); } // Check before changing anything: a stale appearance would contradict the new fill. - if appearance::build(&d).is_none() { + if appearance::build(doc, &d).is_none() { return Err(AnnotError::Unsupported(subtype)); } if fill.is_some_and(|c| !finite(&c)) { @@ -912,7 +912,7 @@ pub fn set_part_opacity(doc: &mut Document, page: usize, index: usize, fill: Opt return Err(AnnotError::Invalid("invalid opacity".into())); } // Check before changing anything: a stale appearance would contradict the new opacity. - if appearance::build(&d).is_none() { + if appearance::build(doc, &d).is_none() { return Err(AnnotError::Unsupported(subtype)); } refuse_foreign_text(doc, &d)?; diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index d9983b8b41..e14838aac8 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1350,6 +1350,12 @@ fn the_eraser_cuts_strokes_and_removes_empty_drawings() { assert!(!list(&doc, 0).iter().any(|d| d.name(b"Subtype") == Some(b"Ink"))); } +/// A document holding only a catalog, for appearance builds from a bare dictionary whose +/// values are all direct. +fn bare_doc() -> Document { + document_of(&[b"<< /Type /Catalog /Pages << /Type /Pages /Kids [] /Count 0 >> >>"]) +} + /// 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(); @@ -1445,7 +1451,7 @@ fn malformed_optional_interior_color_does_not_hide_shape_strokes() { polyline.set(b"IC".to_vec(), malformed); for (name, annotation) in [("Square", square), ("Circle", circle), ("Polygon", polygon), ("Line", line), ("PolyLine", polyline)] { - assert!(appearance::build(&annotation).is_some(), "malformed optional /IC must not hide a valid {name} stroke"); + assert!(appearance::build(&bare_doc(), &annotation).is_some(), "malformed optional /IC must not hide a valid {name} stroke"); } let doc = document_of(&[ @@ -1540,7 +1546,7 @@ fn a_stamp_without_an_appearance_draws_the_stamp_it_names() { // A stamp's own artwork is never replaced, and the drawing is for display only: edits // (restyling, flattening) still find nothing they may write. assert!(appearance::display_stamp(&stamp_dict("/Name /Draft /AP << /N 9 0 R >>")).is_none()); - assert!(appearance::build(&stamp_dict("/Name /Draft")).is_none()); + assert!(appearance::build(&bare_doc(), &stamp_dict("/Name /Draft")).is_none()); assert!(appearance::display_stamp(&stamp_dict("/Name /Draft /PCStamp true")).is_none(), "PdfCraft's own stamps are drawn by `build`"); assert!(appearance::display_stamp(&stamp_dict("/Name /Draft /Rect [5 5 5 50]")).is_none(), "no width"); } @@ -1643,8 +1649,8 @@ fn every_line_ending_is_drawn() { assert_eq!(call.name(b"LE"), Some(b"Diamond".as_slice())); assert!(ap_content(&doc, call).contains("h B"), "diamond callout"); - assert!(appearance::build(&line_dict([f64::NAN, 0.0, 10.0, 0.0], "OpenArrow")).is_none(), "a non-finite endpoint is rejected"); - assert!(appearance::build(&line_dict([0.0, 0.0, 10.0, 0.0], "Foo")).is_none(), "an unknown ending is not replaced"); + assert!(appearance::build(&bare_doc(), &line_dict([f64::NAN, 0.0, 10.0, 0.0], "OpenArrow")).is_none(), "a non-finite endpoint is rejected"); + assert!(appearance::build(&bare_doc(), &line_dict([0.0, 0.0, 10.0, 0.0], "Foo")).is_none(), "an unknown ending is not replaced"); } #[test] @@ -2185,7 +2191,7 @@ fn note_drawing(name: Option<&str>, color: [f64; 3]) -> Vec { if let Some(n) = name { d.set(b"Name".to_vec(), Object::name(n)); } - crate::appearance::build(&d).expect("a note is drawn").decoded().unwrap() + crate::appearance::build(&bare_doc(), &d).expect("a note is drawn").decoded().unwrap() } /// ISO 32000-2 §12.5.6.4, Table 175: a viewer shall provide icons for the seven standard names, @@ -2233,7 +2239,7 @@ fn a_star_note_keeps_its_star_when_recoloured() { fn content_of(src: &str) -> String { let doc = document_of(&[src.as_bytes()]); let d = doc.get(ObjRef::new(1, 0)).as_dict().cloned().expect("a dictionary"); - let stream = appearance::build(&d).expect("drawn"); + let stream = appearance::build(&bare_doc(), &d).expect("drawn"); String::from_utf8_lossy(&stream.decoded().unwrap()).into_owned() } @@ -2285,7 +2291,7 @@ fn bad_dash_arrays_draw_solid() { for w in [f64::NAN, f64::INFINITY, -2.0] { let dash = Object::Array([f64::NAN, 2.0].map(Object::Real).to_vec()); d.set(b"Border".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Real(w), dash])); - let c = String::from_utf8_lossy(&appearance::build(&d).unwrap().decoded().unwrap()).into_owned(); + let c = String::from_utf8_lossy(&appearance::build(&bare_doc(), &d).unwrap().decoded().unwrap()).into_owned(); assert!(c.contains("1 w") && !c.contains(" d\n") && !c.contains("NaN") && !c.contains("inf"), "{w}: {c}"); } } @@ -2307,7 +2313,7 @@ fn a_caret_with_a_paragraph_symbol_draws_the_pilcrow() { let caret = |extra: &str| { let doc = document_of(&[format!("<< /Subtype /Caret /Rect [30 30 60 60] /C [0.9 0.1 0.1] {extra} >>").as_bytes()]); let d = doc.get(ObjRef::new(1, 0)).as_dict().cloned().unwrap(); - let s = appearance::build(&d).unwrap(); + let s = appearance::build(&bare_doc(), &d).unwrap(); (s.decoded().unwrap(), s.dict.get(b"Resources").and_then(Object::as_dict).is_some_and(|r| r.contains(b"Font"))) }; let has_pilcrow = |c: &[u8]| c.windows(5).any(|w| w == b"(\xb6) T"); @@ -2335,7 +2341,7 @@ fn a_caret_with_a_paragraph_symbol_draws_the_pilcrow() { } let doc = document_of(&[b"<< /Subtype /Caret /Rect [30 30 30 30] /Sy /P >>"]); let d = doc.get(ObjRef::new(1, 0)).as_dict().cloned().unwrap(); - assert!(appearance::build(&d).is_some()); + assert!(appearance::build(&bare_doc(), &d).is_some()); } // ── Line annotations: leader lines and captions (ISO 32000-2 §12.5.6.7, Table 178) ────────── @@ -2355,7 +2361,7 @@ fn table_178_line(l: [f64; 4], extra: &[(&str, Object)]) -> Dict { /// The built appearance's content. fn line_drawing(d: &Dict) -> String { - let s = crate::appearance::build(d).expect("the line is drawn"); + let s = crate::appearance::build(&bare_doc(), d).expect("the line is drawn"); String::from_utf8_lossy(&s.decoded().unwrap()).into_owned() } @@ -2508,7 +2514,7 @@ fn a_diagonal_lines_leaders_are_perpendicular_and_its_caption_turns_with_it() { fn a_caption_from_rich_text_and_one_the_font_cant_draw() { let rc = "

Dim & 50 mm <x>

"; let d = table_178_line([0.0, 0.0, 100.0, 0.0], &[("Cap", Object::Bool(true)), ("RC", Object::String(PdfString::text(rc)))]); - let s = crate::appearance::build(&d).unwrap().decoded().unwrap(); + let s = crate::appearance::build(&bare_doc(), &d).unwrap().decoded().unwrap(); assert!(s.windows(17).any(|w| w == b"(Dim & 50 mm )"), "{}", String::from_utf8_lossy(&s)); let cjk = table_178_line([0.0, 0.0, 100.0, 0.0], &[("Cap", Object::Bool(true)), ("Contents", Object::String(PdfString::text("長さ")))]); assert_eq!(crate::appearance::undrawable_text(&cjk), Some(("the line's caption", '長'))); @@ -2788,12 +2794,31 @@ fn reals(v: &[f64]) -> Vec { /// The appearance a `subtype` with `/QuadPoints` `quads` draws, as text. fn quad_appearance(subtype: &str, quads: Vec) -> Option { + quad_appearance_with(&bare_doc(), subtype, Object::Array(quads)) +} + +fn quad_appearance_with(doc: &Document, subtype: &str, quad_points: Object) -> Option { let mut d = Dict::new(); d.set(b"Subtype".to_vec(), Object::name(subtype)); d.set(b"Rect".to_vec(), Object::Array([20, 95, 40, 110].map(Object::Int).to_vec())); d.set(b"C".to_vec(), Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)])); - d.set(b"QuadPoints".to_vec(), Object::Array(quads)); - appearance::build(&d).map(|s| String::from_utf8_lossy(&s.decoded().unwrap()).into_owned()) + d.set(b"QuadPoints".to_vec(), quad_points); + appearance::build(doc, &d).map(|s| String::from_utf8_lossy(&s.decoded().unwrap()).into_owned()) +} + +/// Appearance drawing reads `/QuadPoints` by the rule the other readers do, indirect objects +/// included: entries by reference, and the array itself by reference, draw the same quads as +/// the direct values. +#[test] +fn appearances_draw_indirect_quads_like_direct_ones() { + for subtype in ["Redact", "Highlight"] { + let clean = quad_appearance(subtype, reals(&QUAD)).unwrap(); + let mut doc = bare_doc(); + let by_ref: Vec = QUAD.iter().map(|v| Object::Ref(doc.add(Object::Real(*v)))).collect(); + assert_eq!(quad_appearance_with(&doc, subtype, Object::Array(by_ref)).as_deref(), Some(clean.as_str()), "{subtype}: entries by reference"); + let array_ref = Object::Ref(doc.add(Object::Array(reals(&QUAD)))); + assert_eq!(quad_appearance_with(&doc, subtype, array_ref).as_deref(), Some(clean.as_str()), "{subtype}: the array by reference"); + } } /// Redaction outlines and text markup draw the quads redaction apply would use, no more: a NaN @@ -2808,12 +2833,8 @@ fn appearances_draw_only_usable_quads() { assert!(!clean.contains("NaN"), "{clean}"); let mut name_first = reals(&[QUAD, QUAD].concat()); name_first[2] = Object::name("oops"); - let same = [ - reals(&[one_nan, QUAD].concat()), - name_first, - reals(&[&QUAD[..], &QUAD[..4]].concat()), - reals(&[QUAD, [f64::INFINITY; 8]].concat()), - ]; + let same = + [reals(&[one_nan, QUAD].concat()), name_first, reals(&[&QUAD[..], &QUAD[..4]].concat()), reals(&[QUAD, [f64::INFINITY; 8]].concat())]; for (i, quads) in same.into_iter().enumerate() { assert_eq!(quad_appearance(subtype, quads).as_deref(), Some(clean.as_str()), "{subtype} case {i}"); } diff --git a/crates/geom/src/quad_points.rs b/crates/geom/src/quad_points.rs index 5acba5d0dc..de48755839 100644 --- a/crates/geom/src/quad_points.rs +++ b/crates/geom/src/quad_points.rs @@ -9,9 +9,11 @@ /// /// A quadrilateral is usable only when all eight of its entries are present, numbers, and within /// the range of a PDF real (finite as an `f32`, Annex C), so the readers that keep `f32` see the -/// same set as those that keep `f64`. An unusable one is skipped whole: the ones after it keep -/// their coordinates, and it never becomes a folded box or zeros. A trailing partial group is -/// dropped. +/// same set as those that keep `f64`. Usable values come back on the `f32` grid (each one put +/// through `f32` and back), so the readers fold the same numbers too: a value `f32` cannot hold +/// exactly, like 67108869, is 67108872 for every reader, not just the ones that keep `f32`. An +/// unusable one is skipped whole: the ones after it keep their coordinates, and it never becomes +/// a folded box or zeros. A trailing partial group is dropped. pub fn usable_quads>>(entries: I) -> Vec<[f64; 8]> { let entries: Vec> = entries.into_iter().collect(); entries @@ -22,7 +24,7 @@ pub fn usable_quads>>(entries: I) -> Vec<[f64 let mut quad = [0.0; 8]; for (slot, v) in quad.iter_mut().zip(q) { // NaN and infinities stay non-finite as `f32`; so does a value past its range. - *slot = v.filter(|v| (*v as f32).is_finite())?; + *slot = v.filter(|v| (*v as f32).is_finite()).map(|v| (v as f32) as f64)?; } Some(quad) }) @@ -60,6 +62,18 @@ mod tests { } } + #[test] + fn usable_values_come_back_on_the_f32_grid() { + let mut q = A; + q[0] = 67_108_869.0; // `f32` cannot hold it exactly: it rounds to 67108872 there. + q[1] = 0.1; + let got = usable_quads(some(&q)); + assert_eq!(got.len(), 1); + assert_eq!(got[0][0], (67_108_869.0f64 as f32) as f64); + assert_eq!(got[0][1], (0.1f64 as f32) as f64); + assert_ne!(got[0][0], q[0]); + } + #[test] fn a_non_number_drops_its_quad_without_shifting_the_next() { let mut entries = some(&[A, B].concat()); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 51c51eca62..07479bac68 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2387,6 +2387,8 @@ fn the_outline_shown_is_the_area_apply_removes() { by_reference[2] = Object::Ref(forty); let mut one_nan = OVER_1234; one_nan[5] = f64::NAN; + let mut one_huge = OVER_1234; + one_huge[5] = 4e38; vec![ (reals(&[OVER_1234, OVER_CD].concat()), true), (name_inside, true), @@ -2394,6 +2396,7 @@ fn the_outline_shown_is_the_area_apply_removes() { (by_reference, true), (vec![Object::name("oops"); 8], true), (reals(&[one_nan, OVER_CD].concat()), false), + (reals(&[one_huge, OVER_CD].concat()), true), (reals(&[f64::NAN; 8]), false), ] };