From 4347024e42ebac277987eaf8b0e5ccbab13554ed Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:29:25 +0200 Subject: [PATCH 001/107] Redaction keeps Type 3 glyphs that annotations, patterns and masks show unlink_type3 (#4579) unlinks the Type 3 glyph programs that nothing shows any more. It found what is still shown by scanning page content and the forms reached through /XObject only. Annotation appearances, tiling pattern cells and soft mask groups draw too. A glyph only they showed counted as unused, so its program left /CharProcs, or the whole font lost /CharProcs and left the page's /Font. This ran on every Apply Redactions, even when the mark covered text in another font. The scan now also walks the appearance streams of each page's annotations, the tiling pattern an scn or SCN selects and the group of the soft mask a gs sets. Each is scanned once per stream, font and holder, through its own /Resources or its caller's, with no current font. Forms take the same path through scan_stream_object. Three tests, one per drawer. All three fail without the fix. --- .../redact/src/tests/surviving_references.rs | 83 +++++++++++++++++++ crates/redact/src/type3.rs | 81 ++++++++++++++++-- 2 files changed, 156 insertions(+), 8 deletions(-) diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 477f03f9a5..1a7077974e 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -298,3 +298,86 @@ fn type3_font_used_in_a_form_xobject_has_its_redacted_glyph_removed() { assert_eq!(report.glyphs, 1); assert!(!full_save_holds(&doc, b"SECRET_FORM_GLYPH_PAYLOAD"), "redacted glyph program survived"); } + +/// A Type 3 font with glyphs A, B and C, whose programs are objects `a`, `a + 1` and `a + 2`. +fn type3_abc(a: u32) -> Vec { + let (b, c) = (a + 1, a + 2); + format!( + "<< /Type /Font /Subtype /Type3 /FontBBox [0 -200 1000 800] /FontMatrix [0.001 0 0 0.001 0 0] /FirstChar 65 /LastChar 67 /Widths [500 500 500] /Encoding << /Differences [65 /A /B /C] >> /CharProcs << /A {a} 0 R /B {b} 0 R /C {c} 0 R >> >>" + ) + .into_bytes() +} + +/// An annotation's appearance shows glyph B of a Type 3 font the page lists too. The appearance +/// still draws it, so its program is kept; the redacted glyph A goes. +#[test] +fn a_type3_glyph_an_annotation_appearance_shows_is_kept() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R /Annots [7 0 R] >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET"), + b"<< /Font << /T3 6 0 R >> >>".to_vec(), + type3_abc(9), + b"<< /Type /Annot /Subtype /FreeText /Rect [150 200 250 220] /P 3 0 R /DA (/T3 10 Tf 0 g) /AP << /N 8 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Form /BBox [0 0 100 20] /Resources << /Font << /T3 6 0 R >> >>", b"BT /T3 10 Tf 2 5 Td (B) Tj ET"), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% APPEARANCE_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"APPEARANCE_GLYPH_B"), "the glyph the appearance shows was unlinked"); + assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked"); +} + +/// The cell of a tiling pattern the page fills with shows glyph B: its program is kept. +#[test] +fn a_type3_glyph_a_tiling_pattern_shows_is_kept() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET /Pattern cs /P1 scn 200 200 50 50 re f"), + b"<< /Font << /T3 6 0 R >> /Pattern << /P1 7 0 R >> >>".to_vec(), + type3_abc(8), + stream( + "/Type /Pattern /PatternType 1 /PaintType 1 /TilingType 1 /BBox [0 0 20 20] /XStep 20 /YStep 20 /Resources << /Font << /T3 6 0 R >> >>", + b"BT /T3 10 Tf 2 5 Td (B) Tj ET", + ), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% PATTERN_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"PATTERN_GLYPH_B"), "the glyph the pattern shows was unlinked"); + assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked"); +} + +/// The group of a soft mask the page sets shows glyph B: its program is kept. +#[test] +fn a_type3_glyph_a_soft_mask_shows_is_kept() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET /GS1 gs 150 150 100 100 re f"), + b"<< /Font << /T3 6 0 R >> /ExtGState << /GS1 << /Type /ExtGState /SMask << /Type /Mask /S /Luminosity /G 7 0 R >> >> >> >>".to_vec(), + type3_abc(8), + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Group << /S /Transparency /CS /DeviceGray >> /Resources << /Font << /T3 6 0 R >> >>", + b"1 g BT /T3 40 Tf 160 180 Td (B) Tj ET", + ), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% MASK_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"MASK_GLYPH_B"), "the glyph the soft mask shows was unlinked"); + assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked"); +} diff --git a/crates/redact/src/type3.rs b/crates/redact/src/type3.rs index f431dab902..b7df125513 100644 --- a/crates/redact/src/type3.rs +++ b/crates/redact/src/type3.rs @@ -258,6 +258,17 @@ fn scan_content_stream( scan_xobject(ctx, res, holder_ref, name, current_font, depth)?; } } + b"gs" => { + apply_text_op(ctx, res, &op, &mut current_font); + if let Some(name) = op.name(0) { + scan_soft_mask(ctx, res, holder_ref, name, depth)?; + } + } + b"scn" | b"SCN" => { + if let Some(name) = op.last_name() { + scan_pattern(ctx, res, holder_ref, name, depth)?; + } + } _ => apply_text_op(ctx, res, &op, &mut current_font), } } @@ -320,20 +331,73 @@ fn scan_xobject( ) -> Result<(), CosError> { let xo = res_dict(ctx.doc, res, b"XObject"); let Some(r) = xo.get(name).and_then(Object::as_ref) else { return Ok(()) }; - let obj = ctx.doc.get(r); - let Object::Stream(s) = &*obj else { return Ok(()) }; - if s.dict.name(b"Subtype") != Some(b"Form") { + if !matches!(&*ctx.doc.get(r), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Form")) { return Ok(()); } - let form_own_res = s.dict.get(b"Resources").and_then(|ro| ctx.doc.resolve(ro).as_dict().cloned()); - // A form with resources of its own draws through them; one without draws through its caller's. - let through = if form_own_res.is_some() { r } else { holder_ref }; + scan_stream_object(ctx, res, holder_ref, r, current_font, depth) +} + +/// Scan the tiling pattern `name` that an `scn` or `SCN` operator selects. Its cell is content of +/// its own and can show glyphs of a font the page lists too. A shading pattern is not a stream and +/// draws no text. +fn scan_pattern(ctx: &mut ScanContext<'_>, res: &Dict, holder_ref: ObjRef, name: &[u8], depth: usize) -> Result<(), CosError> { + let patterns = res_dict(ctx.doc, res, b"Pattern"); + let Some(r) = patterns.get(name).and_then(Object::as_ref) else { return Ok(()) }; + scan_stream_object(ctx, res, holder_ref, r, None, depth) +} + +/// Scan the transparency group of the soft mask that the ExtGState `name` sets: what it shows +/// shapes the page, so its glyphs are in use. +fn scan_soft_mask(ctx: &mut ScanContext<'_>, res: &Dict, holder_ref: ObjRef, name: &[u8], depth: usize) -> Result<(), CosError> { + let states = res_dict(ctx.doc, res, b"ExtGState"); + let Some(state) = states.get(name).and_then(|s| ctx.doc.resolve(s).as_dict().cloned()) else { return Ok(()) }; + let Some(mask) = state.get(b"SMask").and_then(|m| ctx.doc.resolve(m).as_dict().cloned()) else { return Ok(()) }; + let Some(group) = mask.get(b"G").and_then(Object::as_ref) else { return Ok(()) }; + scan_stream_object(ctx, res, holder_ref, group, None, depth) +} + +/// Scan the appearance streams of a page's annotations. A comment, stamp or field can show glyphs +/// of a Type 3 font that the page lists too. An appearance without resources of its own draws +/// through the page's. +fn scan_appearances(ctx: &mut ScanContext<'_>, page: &Dict, page_ref: ObjRef, page_res: &Dict) -> Result<(), CosError> { + for annot in crate::annots_of(ctx.doc, page) { + let Some(ap) = ctx.doc.resolve(&annot).as_dict().and_then(|a| a.get(b"AP").cloned()) else { continue }; + let Some(ap) = ctx.doc.resolve(&ap).as_dict().cloned() else { continue }; + for (_, entry) in ap.iter() { + // Each entry is one stream, or a dictionary of streams by appearance state. + let streams: Vec = match &*ctx.doc.resolve(entry) { + Object::Dict(states) => states.iter().filter_map(|(_, s)| s.as_ref()).collect(), + _ => entry.as_ref().into_iter().collect(), + }; + for r in streams { + scan_stream_object(ctx, page_res, page_ref, r, None, 0)?; + } + } + } + Ok(()) +} + +/// Scan a content stream object (a form, a pattern cell, a soft mask group or an appearance), once +/// per (stream, font, drawing holder). One with resources of its own draws through them; one +/// without draws through its caller's. +fn scan_stream_object( + ctx: &mut ScanContext<'_>, + res: &Dict, + holder_ref: ObjRef, + r: ObjRef, + current_font: Option, + depth: usize, +) -> Result<(), CosError> { + let obj = ctx.doc.get(r); + let Object::Stream(s) = &*obj else { return Ok(()) }; + let own_res = s.dict.get(b"Resources").and_then(|ro| ctx.doc.resolve(ro).as_dict().cloned()); + let through = if own_res.is_some() { r } else { holder_ref }; if !ctx.seen_forms.insert((r, current_font, through)) { return Ok(()); } let data = s.decoded()?; - match form_own_res { - Some(form_res) => scan_content_stream(ctx, std::slice::from_ref(&data), r, &form_res, current_font, depth + 1), + match own_res { + Some(own) => scan_content_stream(ctx, std::slice::from_ref(&data), r, &own, current_font, depth + 1), None => scan_content_stream(ctx, std::slice::from_ref(&data), holder_ref, res, current_font, depth + 1), } } @@ -480,6 +544,7 @@ pub(crate) fn unlink_type3(doc: &mut Document) -> Result<(), RedactError> { .unwrap_or_default(); scan_content_stream(&mut ctx, &content, p.obj, &page_res, None, 0)?; + scan_appearances(&mut ctx, &p.dict, p.obj, &page_res)?; } // Also scan any surviving form that wasn't reached by page streams directly. From 08237a5e3df8a00e8d01fb1953cfc2fb654893cc Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:32:10 +0200 Subject: [PATCH 002/107] Type 3 cleanup reads only drawn appearances, and keeps the fonts they select An annotation's /AP can hold entries that no reader draws. Only the normal, rollover and down appearances (N, R, D) now keep a Type 3 glyph, so an extra entry such as /X can no longer keep a redacted glyph program in the saved file. Rewriting a page also dropped a Type 3 font that only an annotation appearance without /Resources selects, although that appearance draws through the page's resources. The page prune now reads those appearances too. It reads them only when the page lists a Type 3 font, so other files are not refused over an appearance it cannot parse. Two tests, both failing without this change. --- crates/redact/src/interp.rs | 2 +- crates/redact/src/lib.rs | 2 +- crates/redact/src/sanitize.rs | 2 +- .../redact/src/tests/surviving_references.rs | 60 +++++++++++++++++ crates/redact/src/type3.rs | 66 +++++++++++++++---- 5 files changed, 115 insertions(+), 17 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index ac69c73acb..8ab7602503 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -884,7 +884,7 @@ fn xobject( out.error = Some(error); return None; } - if let Err(error) = crate::type3::prune_type3_from_resources(doc, &mut cleaned_res, content) { + if let Err(error) = crate::type3::prune_type3_from_resources(doc, &mut cleaned_res, content, None) { out.error = Some(error); return None; } diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index de9dafe7db..2c9f9fcf15 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -626,7 +626,7 @@ fn redact_page_content( pi, )?; retired_properties.extend(interp::prune_properties(doc, &mut res, &cleaned_data)?); - type3::prune_type3_from_resources(doc, &mut res, &cleaned_data)?; + type3::prune_type3_from_resources(doc, &mut res, &cleaned_data, Some(&page.dict))?; doc.update_dict(page.obj, |d| { d.set(b"Contents".to_vec(), Object::Array(new_list)); d.set(b"Resources".to_vec(), Object::Dict(res)); diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 98703dfbf2..d3e59bd684 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -424,7 +424,7 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re res.set(b"XObject".to_vec(), Object::Dict(xo)); } retired_properties.extend(crate::interp::prune_properties(doc, &mut res, &page_content)?); - crate::type3::prune_type3_from_resources(doc, &mut res, &page_content)?; + crate::type3::prune_type3_from_resources(doc, &mut res, &page_content, Some(&page.dict))?; doc.update_dict(page.obj, |d| { d.set(b"Contents".to_vec(), Object::Array(new_list)); d.set(b"Resources".to_vec(), Object::Dict(res)); diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 1a7077974e..5548736688 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -332,6 +332,66 @@ fn a_type3_glyph_an_annotation_appearance_shows_is_kept() { assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked"); } +/// The same annotation also has an `/AP` entry no reader draws (`/X`), which shows the redacted +/// glyph A. It keeps nothing: A's program goes, and B, which the normal appearance shows, stays. +#[test] +fn a_type3_glyph_only_an_undrawn_appearance_entry_shows_is_unlinked() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R /Annots [7 0 R] >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET"), + b"<< /Font << /T3 6 0 R >> >>".to_vec(), + type3_abc(10), + b"<< /Type /Annot /Subtype /FreeText /Rect [150 200 250 220] /P 3 0 R /DA (/T3 10 Tf 0 g) /AP << /N 8 0 R /X 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Form /BBox [0 0 100 20] /Resources << /Font << /T3 6 0 R >> >>", b"BT /T3 10 Tf 2 5 Td (B) Tj ET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 100 20] /Resources << /Font << /T3 6 0 R >> >>", b"BT /T3 10 Tf 2 5 Td (A) Tj ET"), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% APPEARANCE_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "an undrawn appearance entry kept the redacted glyph program"); + assert!(full_save_holds(&doc, b"APPEARANCE_GLYPH_B"), "the glyph the normal appearance shows was unlinked"); + assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked"); +} + +/// An appearance without /Resources shows glyph B of a second Type 3 font, which the page lists +/// but its content never selects. The appearance draws through the page's resources, so the +/// rewritten page still lists that font and B's program stays. +#[test] +fn a_type3_font_only_an_appearance_without_resources_selects_stays_on_the_page() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R /Annots [8 0 R] >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET"), + b"<< /Font << /T3 6 0 R /T4 7 0 R >> >>".to_vec(), + type3_abc(10), + type3_abc(13), + b"<< /Type /Annot /Subtype /FreeText /Rect [150 200 250 220] /P 3 0 R /DA (/T4 10 Tf 0 g) /AP << /N 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Form /BBox [0 0 100 20]", b"BT /T4 10 Tf 2 5 Td (B) Tj ET"), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% UNUSED_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + stream("", b"% UNUSED_T4_GLYPH_A"), + stream("", b"% APPEARANCE_GLYPH_B"), + stream("", b"% UNUSED_T4_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"APPEARANCE_GLYPH_B"), "the glyph the appearance shows was unlinked"); + + let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); + let saved = Document::open(Arc::new(bytes)).unwrap(); + let page = pdfcraft_model::pages(&saved).swap_remove(0); + let res = saved.resolve(page.dict.get(b"Resources").unwrap()).as_dict().unwrap().clone(); + let fonts = saved.resolve(res.get(b"Font").unwrap()).as_dict().unwrap().clone(); + assert!(fonts.contains(b"T4"), "the page no longer lists the font the appearance selects"); +} + /// The cell of a tiling pattern the page fills with shows glyph B: its program is kept. #[test] fn a_type3_glyph_a_tiling_pattern_shows_is_kept() { diff --git a/crates/redact/src/type3.rs b/crates/redact/src/type3.rs index e796d0d5c0..a39f1af144 100644 --- a/crates/redact/src/type3.rs +++ b/crates/redact/src/type3.rs @@ -357,22 +357,50 @@ fn scan_soft_mask(ctx: &mut ScanContext<'_>, res: &Dict, holder_ref: ObjRef, nam /// through the page's. fn scan_appearances(ctx: &mut ScanContext<'_>, page: &Dict, page_ref: ObjRef, page_res: &Dict) -> Result<(), CosError> { for annot in crate::annots_of(ctx.doc, page) { - let Some(ap) = ctx.doc.resolve(&annot).as_dict().and_then(|a| a.get(b"AP").cloned()) else { continue }; - let Some(ap) = ctx.doc.resolve(&ap).as_dict().cloned() else { continue }; - for (_, entry) in ap.iter() { - // Each entry is one stream, or a dictionary of streams by appearance state. - let streams: Vec = match &*ctx.doc.resolve(entry) { - Object::Dict(states) => states.iter().filter_map(|(_, s)| s.as_ref()).collect(), - _ => entry.as_ref().into_iter().collect(), - }; - for r in streams { - scan_stream_object(ctx, page_res, page_ref, r, None, 0)?; - } + for r in appearance_streams(ctx.doc, &annot) { + scan_stream_object(ctx, page_res, page_ref, r, None, 0)?; } } Ok(()) } +/// The streams an annotation can show: its normal, rollover and down appearances (ISO 32000-2 +/// §12.5.5), each one stream or a dictionary of streams by appearance state. Readers draw no +/// other `/AP` entry, so one can't keep a glyph the page no longer shows. +fn appearance_streams(doc: &Document, annot: &Object) -> Vec { + let Some(ap) = doc.resolve(annot).as_dict().and_then(|a| a.get(b"AP").cloned()) else { return Vec::new() }; + let Some(ap) = doc.resolve(&ap).as_dict().cloned() else { return Vec::new() }; + let mut streams = Vec::new(); + for key in [b"N", b"R", b"D"] { + let Some(entry) = ap.get(key) else { continue }; + match &*doc.resolve(entry) { + Object::Dict(states) => streams.extend(states.iter().filter_map(|(_, s)| s.as_ref())), + _ => streams.extend(entry.as_ref()), + } + } + streams +} + +/// The decoded appearance streams of a page's annotations that have no resources of their own: +/// they draw through the page's, as [`scan_stream_object`] reads them. +fn resourceless_appearances(doc: &Document, page: &Dict) -> Result>, CosError> { + let mut seen = HashSet::new(); + let mut out = Vec::new(); + for annot in crate::annots_of(doc, page) { + for r in appearance_streams(doc, &annot) { + if !seen.insert(r) { + continue; + } + if let Object::Stream(s) = &*doc.get(r) + && s.dict.get(b"Resources").is_none_or(|ro| doc.resolve(ro).as_dict().is_none()) + { + out.push(s.decoded()?); + } + } + } + Ok(out) +} + /// Scan a content stream object (a form, a pattern cell, a soft mask group or an appearance), once /// per (stream, font, drawing holder). One with resources of its own draws through them; one /// without draws through its caller's. @@ -406,8 +434,9 @@ fn gs_font_object(doc: &Document, state_obj: &Object) -> Option { } /// Prune Type 3 fonts from a rewritten page or form's `/Resources` dictionary if the rewritten -/// content no longer draws any text using that font name. -pub(crate) fn prune_type3_from_resources(doc: &Document, res: &mut Dict, content: &[Vec]) -> Result<(), CosError> { +/// content no longer draws any text using that font name. `page` is the page's dictionary when +/// `res` are a page's resources. +pub(crate) fn prune_type3_from_resources(doc: &Document, res: &mut Dict, content: &[Vec], page: Option<&Dict>) -> Result<(), CosError> { let mut fonts = res_dict(doc, res, b"Font"); if fonts.is_empty() { return Ok(()); @@ -415,7 +444,16 @@ pub(crate) fn prune_type3_from_resources(doc: &Document, res: &mut Dict, content // The operators of forms that draw through these resources count too: dropping a font a child // form still shows would leave its text without one. - let ops = crate::interp::resource_ops(doc, res, content)?; + let mut ops = crate::interp::resource_ops(doc, res, content)?; + // So do those of a page's annotation appearances without resources of their own. They are + // read only when there is a Type 3 font to keep, so other files are not refused over them. + if let Some(page) = page + && fonts.iter().any(|(_, f)| doc.resolve(f).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Type3"))) + { + for data in resourceless_appearances(doc, page)? { + ops.extend(crate::interp::resource_ops(doc, res, std::slice::from_ref(&data))?); + } + } let mut drawn_font_names = HashSet::new(); for op in &ops { From 0cb06e4568899dc61031e354cfa898f097087fd8 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:46:22 +0200 Subject: [PATCH 003/107] Redaction keeps a page Type 3 font that a pattern or soft mask shows The page font prune counted the page content, forms without /Resources and annotation appearances, but not tiling patterns or soft mask groups without /Resources. Those draw through the page's resources too, so a Type 3 font only they selected left the page and their text lost its font. They are now read as well, only when the page lists a Type 3 font. Two new tests fail without the change. --- .../redact/src/tests/surviving_references.rs | 68 ++++++++++++++++++- crates/redact/src/type3.rs | 61 +++++++++++++++-- 2 files changed, 119 insertions(+), 10 deletions(-) diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 5548736688..0864c701bd 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -383,13 +383,75 @@ fn a_type3_font_only_an_appearance_without_resources_selects_stays_on_the_page() assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); assert!(full_save_holds(&doc, b"APPEARANCE_GLYPH_B"), "the glyph the appearance shows was unlinked"); + assert!(saved_first_page_lists_font(&doc, b"T4"), "the page no longer lists the font the appearance selects"); +} - let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); +/// Whether the first page of a full save of `doc` lists the font `name` in its resources. +fn saved_first_page_lists_font(doc: &Document, name: &[u8]) -> bool { + let bytes = write_full(doc, &SaveOptions::default()).unwrap(); let saved = Document::open(Arc::new(bytes)).unwrap(); let page = pdfcraft_model::pages(&saved).swap_remove(0); let res = saved.resolve(page.dict.get(b"Resources").unwrap()).as_dict().unwrap().clone(); - let fonts = saved.resolve(res.get(b"Font").unwrap()).as_dict().unwrap().clone(); - assert!(fonts.contains(b"T4"), "the page no longer lists the font the appearance selects"); + res.get(b"Font").and_then(|f| saved.resolve(f).as_dict().map(|fonts| fonts.contains(name))).unwrap_or(false) +} + +/// A tiling pattern without /Resources shows glyph B of a second Type 3 font, which the page lists +/// but its content never selects. The cell draws through the page's resources, so the rewritten +/// page still lists that font and B's program stays. +#[test] +fn a_type3_font_only_a_pattern_without_resources_selects_stays_on_the_page() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET /Pattern cs /P1 scn 200 200 50 50 re f"), + b"<< /Font << /T3 6 0 R /T4 7 0 R >> /Pattern << /P1 8 0 R >> >>".to_vec(), + type3_abc(9), + type3_abc(12), + stream("/Type /Pattern /PatternType 1 /PaintType 1 /TilingType 1 /BBox [0 0 20 20] /XStep 20 /YStep 20", b"BT /T4 10 Tf 2 5 Td (B) Tj ET"), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% UNUSED_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + stream("", b"% UNUSED_T4_GLYPH_A"), + stream("", b"% PATTERN_GLYPH_B"), + stream("", b"% UNUSED_T4_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"PATTERN_GLYPH_B"), "the glyph the pattern shows was unlinked"); + assert!(saved_first_page_lists_font(&doc, b"T4"), "the page no longer lists the font the pattern selects"); +} + +/// The same with the group of a soft mask without /Resources: the page still lists the second +/// font, and the glyph the mask shows stays. +#[test] +fn a_type3_font_only_a_soft_mask_without_resources_selects_stays_on_the_page() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R >>".to_vec(), + stream("", b"BT /T3 10 Tf 10 100 Td (AC) Tj ET /GS1 gs 150 150 100 100 re f"), + b"<< /Font << /T3 6 0 R /T4 7 0 R >> /ExtGState << /GS1 << /Type /ExtGState /SMask << /Type /Mask /S /Luminosity /G 8 0 R >> >> >> >>" + .to_vec(), + type3_abc(9), + type3_abc(12), + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Group << /S /Transparency /CS /DeviceGray >>", + b"1 g BT /T4 40 Tf 160 180 Td (B) Tj ET", + ), + stream("", b"% REDACTED_GLYPH_A"), + stream("", b"% UNUSED_GLYPH_B"), + stream("", b"% PAGE_GLYPH_C"), + stream("", b"% UNUSED_T4_GLYPH_A"), + stream("", b"% MASK_GLYPH_B"), + stream("", b"% UNUSED_T4_GLYPH_C"), + ]); + mark(&mut doc, 0, &[[9.0, 95.0, 14.9, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 1); + assert!(!full_save_holds(&doc, b"REDACTED_GLYPH_A"), "the redacted glyph program survived"); + assert!(full_save_holds(&doc, b"MASK_GLYPH_B"), "the glyph the soft mask shows was unlinked"); + assert!(saved_first_page_lists_font(&doc, b"T4"), "the page no longer lists the font the soft mask selects"); } /// The cell of a tiling pattern the page fills with shows glyph B: its program is kept. diff --git a/crates/redact/src/type3.rs b/crates/redact/src/type3.rs index a39f1af144..32145c426a 100644 --- a/crates/redact/src/type3.rs +++ b/crates/redact/src/type3.rs @@ -401,6 +401,50 @@ fn resourceless_appearances(doc: &Document, page: &Dict) -> Result>, Ok(out) } +/// Add to `ops` those of the tiling patterns and soft mask groups without resources of their own +/// that `ops` select (`scn`, `SCN`, `gs`), and of the forms and further ones those draw in turn: +/// they draw through `res`, as [`scan_stream_object`] reads them. The forms that the first `known` +/// operators draw are in `ops` already. +fn add_pattern_and_mask_ops(doc: &Document, res: &Dict, ops: &mut Vec, known: usize) -> Result<(), CosError> { + let xobjects = res_dict(doc, res, b"XObject"); + let patterns = res_dict(doc, res, b"Pattern"); + let states = res_dict(doc, res, b"ExtGState"); + let mut seen = HashSet::new(); + let mut cursor = 0; + while let Some(op) = ops.get(cursor) { + let selected = match op.op.as_slice() { + b"scn" | b"SCN" => op.last_name().and_then(|n| patterns.get(n)).and_then(Object::as_ref), + b"gs" => op + .name(0) + .and_then(|n| states.get(n)) + .and_then(|s| doc.resolve(s).as_dict().and_then(|s| s.get(b"SMask").cloned())) + .and_then(|m| doc.resolve(&m).as_dict().and_then(|m| m.get(b"G").and_then(Object::as_ref))), + b"Do" if cursor >= known => op + .last_name() + .and_then(|n| xobjects.get(n)) + .and_then(Object::as_ref) + .filter(|r| matches!(&*doc.get(*r), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Form"))), + _ => None, + }; + cursor += 1; + let Some(r) = selected else { continue }; + if !seen.insert(r) { + continue; + } + let obj = doc.get(r); + let Object::Stream(s) = &*obj else { continue }; + if s.dict.get(b"Resources").is_some_and(|ro| doc.resolve(ro).as_dict().is_some()) { + continue; + } + let parsed = pdfcraft_content::parse(&s.decoded()?); + if parsed.skipped > 0 || ops.len().saturating_add(parsed.ops.len()) > 2_000_000 { + return Err(CosError::Syntax { offset: 0, detail: "redaction could not fully inspect pattern or soft mask content".into() }); + } + ops.extend(parsed.ops); + } + Ok(()) +} + /// Scan a content stream object (a form, a pattern cell, a soft mask group or an appearance), once /// per (stream, font, drawing holder). One with resources of its own draws through them; one /// without draws through its caller's. @@ -445,14 +489,17 @@ pub(crate) fn prune_type3_from_resources(doc: &Document, res: &mut Dict, content // The operators of forms that draw through these resources count too: dropping a font a child // form still shows would leave its text without one. let mut ops = crate::interp::resource_ops(doc, res, content)?; - // So do those of a page's annotation appearances without resources of their own. They are - // read only when there is a Type 3 font to keep, so other files are not refused over them. - if let Some(page) = page - && fonts.iter().any(|(_, f)| doc.resolve(f).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Type3"))) - { - for data in resourceless_appearances(doc, page)? { - ops.extend(crate::interp::resource_ops(doc, res, std::slice::from_ref(&data))?); + // So do those of a page's annotation appearances, and of tiling patterns and soft mask groups, + // without resources of their own. They are read only when there is a Type 3 font to keep, so + // other files are not refused over them. + if fonts.iter().any(|(_, f)| doc.resolve(f).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Type3"))) { + if let Some(page) = page { + for data in resourceless_appearances(doc, page)? { + ops.extend(crate::interp::resource_ops(doc, res, std::slice::from_ref(&data))?); + } } + let known = ops.len(); + add_pattern_and_mask_ops(doc, res, &mut ops, known)?; } let mut drawn_font_names = HashSet::new(); From 2468d8e470e87a271dbfb20e1e397b614474faf1 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:35:12 +0200 Subject: [PATCH 004/107] Say why the form path prunes Type 3 fonts without a page Annotation appearances draw through a page's resources, never through a form's own, so the form path passes no page. The form's own patterns and soft masks are still counted. Asked in review. --- crates/redact/src/interp.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 8ab7602503..edebb99dcb 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -884,6 +884,8 @@ fn xobject( out.error = Some(error); return None; } + // No page: annotation appearances draw through a page's resources, never through a + // form's. The form's own patterns and soft masks are still counted. if let Err(error) = crate::type3::prune_type3_from_resources(doc, &mut cleaned_res, content, None) { out.error = Some(error); return None; From 8a6aff26e056ced3bca4a943af6ff3e4a5e22924 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:59:36 +0800 Subject: [PATCH 005/107] fix(xfa): bound the text the draw phase lays out, so large text in many instances stops draw_value_text re-measures overflowing text up to six times to shrink it, and a repeating subform draws its text once per instance, with no shared cache. A small file with large text drawn across many instances therefore cost one full text layout per instance per pass, for seconds of work, and finished with a valid layout, so neither the page limit nor the measurement budget stopped it. Each pass over a draw's text now counts its characters against a new draw budget (MAX_DRAW_CHARS), the way the measurement budget bounds container sizing. A real form's draws stay far below it; a template that lays out far more text stops with TooLarge. The budget is reduced under test so the guard is exercised without laying out hundreds of megabytes. Test: large_text_drawn_in_many_instances_spends_the_draw_budget (fifty instances of a 20,000-character draw give TooLarge; one copy lays out); it fails without the cap. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Pq7HyG234xeYkJfr41E6iP --- crates/xfa/src/layout.rs | 12 ++++++++++++ crates/xfa/src/layout/draw.rs | 10 +++++++++- crates/xfa/src/layout/pages.rs | 12 ++++++++++-- crates/xfa/src/tests.rs | 16 ++++++++++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/crates/xfa/src/layout.rs b/crates/xfa/src/layout.rs index eab26c6dd4..2eb3d08462 100644 --- a/crates/xfa/src/layout.rs +++ b/crates/xfa/src/layout.rs @@ -43,6 +43,15 @@ const MAX_DEPTH: usize = 64; /// Most size measurements of one layout. Measurements of containers are memoized, so a real /// form stays far below this; it bounds the work a hostile template can cause. const MAX_MEASURES: usize = 2_000_000; +/// Most characters the draw phase lays out in one layout, each pass counted. A draw re-measures +/// overflowing text up to six times to shrink it, and a repeating subform draws its text once +/// per instance, so large text drawn many times would otherwise cost seconds; a real form's +/// draws stay far below this. Reduced under test so the guard is exercised without laying out +/// hundreds of megabytes of text. +#[cfg(not(test))] +const MAX_DRAW_CHARS: usize = 256 << 20; +#[cfg(test)] +const MAX_DRAW_CHARS: usize = 1 << 20; /// Most initial instances of one repeating subform. const MAX_INSTANCES: usize = 50; /// Largest picture taken from the data (decoded bytes), as for template pictures. @@ -149,6 +158,8 @@ struct Layouter<'t> { measured: RefCell>, /// Measurements made so far (see [`MAX_MEASURES`]). measures: Cell, + /// Characters the draw phase has laid out, each pass counted (see [`MAX_DRAW_CHARS`]). + drawn_chars: Cell, /// The fields and draws of the block being placed: (first item, one past the last, top, /// bottom) on the last page. A page break keeps each whole (see `split_block`). leaves: Vec<(usize, usize, f64, f64)>, @@ -199,6 +210,7 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result image_bytes: 0, measured: RefCell::new(HashMap::new()), measures: Cell::new(0), + drawn_chars: Cell::new(0), leaves: Vec::new(), frames: Vec::new(), }; diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index 02659c812e..f1ad5c45d9 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -229,11 +229,19 @@ impl Layouter<'_> { _ => return Ok(()), }; let face = Face::of(font); + // Each pass over the text — a re-measure that shrinks it, or the final layout — counts + // its characters against the draw budget, so large text drawn in many instances stops. + let cost = rich.plain().chars().count().max(1); // Text that doesn't fit the height it was given (the template's face is narrower than // the standard one drawn in its place, so it wraps to more lines) is drawn smaller, // down to 70%, rather than over what lies below. - let fits = |k: f64| text::measure_height(&rich, rect.w / k, para, &face, &|id, max| self.embed(id, max)) <= rect.h / k + 1.0; + let fits = |k: f64| { + self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); + text::measure_height(&rich, rect.w / k, para, &face, &|id, max| self.embed(id, max)) <= rect.h / k + 1.0 + }; let scale = if rect.h < 4.0 || fits(1.0) { 1.0 } else { [0.95, 0.9, 0.85, 0.8, 0.75].into_iter().find(|k| fits(*k)).unwrap_or(0.7) }; + self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); + self.check_drawn_chars()?; let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &|id, max| self.embed(id, max)); if scale < 1.0 { for s in &mut block.spans { diff --git a/crates/xfa/src/layout/pages.rs b/crates/xfa/src/layout/pages.rs index dac17c1169..c37cf83bd2 100644 --- a/crates/xfa/src/layout/pages.rs +++ b/crates/xfa/src/layout/pages.rs @@ -2,8 +2,8 @@ use super::EMBED_COPIED; use super::items::{Item, Page, UNDERLINE_DROP}; use super::{ - Block, Border, Ctx, Cursor, FlowMark, Font, Frame, Layouter, MAX_ITEMS, MAX_MEASURES, MAX_PAGES, PAGE_COUNT_MARK, PageRole, Para, Rc, Rect, - SomIndices, XfaError, text, + Block, Border, Ctx, Cursor, FlowMark, Font, Frame, Layouter, MAX_DRAW_CHARS, MAX_ITEMS, MAX_MEASURES, MAX_PAGES, PAGE_COUNT_MARK, PageRole, Para, + Rc, Rect, SomIndices, XfaError, text, }; impl Layouter<'_> { @@ -39,6 +39,14 @@ impl Layouter<'_> { Ok(()) } + /// `TooLarge` once the draw phase has laid out far more text than a real form does. + pub(super) fn check_drawn_chars(&self) -> Result<(), XfaError> { + if self.drawn_chars.get() > MAX_DRAW_CHARS { + return Err(XfaError::TooLarge("the form draws far more text than the reader lays out".into())); + } + Ok(()) + } + pub(super) fn push(&mut self, item: Item) -> Result<(), XfaError> { self.check_measures()?; self.items += 1; diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 6d93e1bc99..b360fa08f7 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -678,6 +678,22 @@ fn empty_repeating_subforms_spend_the_layout_budget() { assert!(layout_xml(&tpl).is_ok()); } +#[test] +fn large_text_drawn_in_many_instances_spends_the_draw_budget() { + // A draw whose text overflows its fixed height is re-measured up to six times to shrink it, + // and a repeating subform draws it once per instance. Large text drawn in many instances + // used to cost one full text layout per instance per pass, for seconds of work on a small + // file; each pass now counts its characters against the draw budget. + let words = "word ".repeat(4000); // 20,000 characters + let draw = format!("{words}"); + let many = + format!(""); + assert!(matches!(layout_xml(&many), Err(XfaError::TooLarge(_))), "large text in many instances must be refused"); + // One copy of the same draw lays out: the budget does not reject an ordinary form. + let one = format!(""); + assert!(layout_xml(&one).is_ok(), "a single large draw must lay out"); +} + #[test] fn a_packet_stream_listed_many_times_is_read_once() { let mut doc = Document::open(Arc::new(shell(""))).unwrap(); From c251b3a0a725c460774e85b717ec878b52a1e286 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 15:28:23 +0000 Subject: [PATCH 006/107] fix(redact): read inherited forms once for all names in resource cleanup unlink_doomed and unlink_from_forms asked, name by name, whether the remaining content still draws an XObject, and each question parsed the content and decoded every form without /Resources of its own again. A holder with many aliases of one removed image, drawn by a large inheriting form, multiplied that decoding by the number of aliases. names_drawn now works out once per content which names it draws, directly or through such forms (each read once), and whether any of it could not be read; every alias is answered from that set. A name not in the set while something could not be read still fails closed. --- crates/redact/src/interp/prune.rs | 79 +++++++++++-------- .../redact/src/tests/surviving_references.rs | 26 ++++++ 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/crates/redact/src/interp/prune.rs b/crates/redact/src/interp/prune.rs index 6ca5ea2803..d8742a73c5 100644 --- a/crates/redact/src/interp/prune.rs +++ b/crates/redact/src/interp/prune.rs @@ -20,33 +20,45 @@ pub(crate) fn prune(xo: &mut Dict, replaced: &[Vec], content: &[Vec]) { } } -/// Whether `content` draws the XObject `name` of the resource dictionary `xo`: directly, or -/// through a form that has no /Resources of its own (such a form draws through its caller's). -/// `None` when that cannot be told (a form that will not decode, or nesting past the limit). -fn content_draws(doc: &Document, content: &[Vec], xo: &Dict, name: &[u8], depth: usize, seen: &mut HashSet) -> Option { - let joined = Pieces::join(content); - let ops = joined.parse().ops; - for op in ops.iter().filter(|op| op.is("Do")) { - let Some(n) = op.last_name() else { continue }; - if n == name { - return Some(true); - } - let Some(r) = xo.get(n).and_then(Object::as_ref) else { continue }; - let obj = doc.get(r); - let Object::Stream(s) = &*obj else { continue }; - if s.dict.name(b"Subtype") != Some(b"Form") || s.dict.contains(b"Resources") || !seen.insert(r) { - continue; - } - if depth >= MAX_DEPTH { - return None; - } - let Ok(data) = s.decoded() else { return None }; - match content_draws(doc, std::slice::from_ref(&data), xo, name, depth + 1, seen) { - Some(false) => {} - other => return other, +/// The XObject names of the resource dictionary `xo` that `content` draws: directly, or through +/// a form that has no /Resources of its own (such a form draws through its caller's). Each such +/// form is read once, however many names are asked about. The flag says whether some of what +/// `content` draws could not be read (a form that will not decode, or nesting past the limit), so +/// a name missing from the set may still be drawn. +fn names_drawn(doc: &Document, content: &[Vec], xo: &Dict) -> (HashSet>, bool) { + let mut drawn = HashSet::new(); + let mut seen = HashSet::new(); + let mut pending = vec![(Pieces::join(content).parse().ops, 0)]; + while let Some((ops, depth)) = pending.pop() { + for n in ops.iter().filter(|op| op.is("Do")).filter_map(Op::last_name) { + if !drawn.insert(n.to_vec()) { + continue; + } + let Some(r) = xo.get(n).and_then(Object::as_ref) else { continue }; + let obj = doc.get(r); + let Object::Stream(s) = &*obj else { continue }; + if s.dict.name(b"Subtype") != Some(b"Form") || s.dict.contains(b"Resources") || !seen.insert(r) { + continue; + } + match s.decoded() { + Ok(data) if depth < MAX_DEPTH => pending.push((pdfcraft_content::parse(&data).ops, depth + 1)), + _ => return (drawn, true), + } } } - Some(false) + (drawn, false) +} + +/// Whether any of `drawn` (from [`names_drawn`]) draws `name`: `None` when none is known to and +/// one could not be read in full. +fn any_draws(drawn: &[(HashSet>, bool)], name: &[u8]) -> Option { + if drawn.iter().any(|(names, _)| names.contains(name)) { + Some(true) + } else if drawn.iter().any(|(_, unknown)| *unknown) { + None + } else { + Some(false) + } } /// Unlink the XObjects a rewrite dropped or replaced (`doomed`) from every page-tree resource @@ -112,15 +124,10 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope contents.push(page_streams(doc, &page, index.get(user).copied().unwrap_or(0))?.1); } let mut pruned = false; + // What each page draws through this dictionary, worked out once for all the names. + let drawn: Vec<_> = if names.is_empty() { Vec::new() } else { contents.iter().map(|content| names_drawn(doc, content, &xo)).collect() }; for n in names { - let mut drawn = Some(false); - for content in &contents { - drawn = content_draws(doc, content, &xo, &n, 0, &mut HashSet::new()); - if drawn != Some(false) { - break; - } - } - match drawn { + match any_draws(&drawn, &n) { Some(true) => {} Some(false) => { xo.remove(&n); @@ -230,8 +237,12 @@ fn unlink_from_forms(doc: &mut Document, holders: &[ObjRef], doomed: &HashSet], xo: &mut Dict, doomed_names: &[Vec]) -> Result { let mut pruned = false; + if doomed_names.is_empty() { + return Ok(false); + } + let drawn = [names_drawn(doc, content, xo)]; for name in doomed_names { - match content_draws(doc, content, xo, name, 0, &mut HashSet::new()) { + match any_draws(&drawn, name) { Some(true) => {} Some(false) => pruned |= xo.remove(name).is_some(), None => return Err(CosError::Syntax { offset: 0, detail: "redaction could not fully inspect shared form resources".into() }), diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 477f03f9a5..bfcebd171e 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -298,3 +298,29 @@ fn type3_font_used_in_a_form_xobject_has_its_redacted_glyph_removed() { assert_eq!(report.glyphs, 1); assert!(!full_save_holds(&doc, b"SECRET_FORM_GLYPH_PAYLOAD"), "redacted glyph program survived"); } + +/// Resource cleanup read a form without /Resources of its own again for every name that could +/// still reach the removed image. Here 3 000 names of one redacted image sit in the inherited +/// /Pages resources, and the other page draws a large such form: it is read once, and every name +/// leaves with the image. +#[test] +fn many_names_of_a_redacted_image_are_unlinked_reading_inherited_forms_once() { + const ALIASES: usize = 3_000; + let aliases: String = (0..ALIASES).map(|i| format!("/A{i} 6 0 R ")).collect(); + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /Resources 5 0 R >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 8 0 R >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 9 0 R >>".to_vec(), + format!("<< /XObject << /Im 6 0 R /Fm 7 0 R {aliases}>> >>").into_bytes(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"200 200 m 210 210 l S ".repeat(20_000)), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q"), + stream("", b"/Fm Do"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + let started = std::time::Instant::now(); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(started.elapsed() < std::time::Duration::from_secs(60), "{:?}", started.elapsed()); + assert!(!full_save_holds(&doc, b"IMAGESECRET"), "an alias kept the removed image reachable"); +} From 962af16e087e9b49b12fc6a3b813451611758cfa Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 5 Nov 2026 07:05:07 +0800 Subject: [PATCH 007/107] fix(xfa): the draw budget counts the values embedded fields add to a text Each pass over a draw's text counted only its own characters, so a short text embedding a long field value, drawn in many instances, never spent the budget. The embedded values each pass lays out are counted too. (CodeRabbit review on #5852.) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/xfa/src/layout/draw.rs | 11 +++++++++-- crates/xfa/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index f1ad5c45d9..9c0b385c63 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -232,17 +232,24 @@ impl Layouter<'_> { // Each pass over the text — a re-measure that shrinks it, or the final layout — counts // its characters against the draw budget, so large text drawn in many instances stops. let cost = rich.plain().chars().count().max(1); + // The values of embedded fields are laid out with it, so each pass counts those too: a + // short text embedding a long value is as much work as the value written out. + let embed = |id: &str, max: usize| { + let v = self.embed(id, max); + self.drawn_chars.set(self.drawn_chars.get().saturating_add(v.chars().count())); + v + }; // Text that doesn't fit the height it was given (the template's face is narrower than // the standard one drawn in its place, so it wraps to more lines) is drawn smaller, // down to 70%, rather than over what lies below. let fits = |k: f64| { self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); - text::measure_height(&rich, rect.w / k, para, &face, &|id, max| self.embed(id, max)) <= rect.h / k + 1.0 + text::measure_height(&rich, rect.w / k, para, &face, &embed) <= rect.h / k + 1.0 }; let scale = if rect.h < 4.0 || fits(1.0) { 1.0 } else { [0.95, 0.9, 0.85, 0.8, 0.75].into_iter().find(|k| fits(*k)).unwrap_or(0.7) }; self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); self.check_drawn_chars()?; - let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &|id, max| self.embed(id, max)); + let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &embed); if scale < 1.0 { for s in &mut block.spans { s.x = rect.x + (s.x - rect.x) * scale; diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 9026ceeded..a55d74498e 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -694,6 +694,25 @@ fn large_text_drawn_in_many_instances_spends_the_draw_budget() { assert!(layout_xml(&one).is_ok(), "a single large draw must lay out"); } +/// The draw budget counts the values of embedded fields as well as the text around them: a short +/// text embedding a long value, drawn in many instances, is as much work as the value written out +/// each time (CodeRabbit review on #5852). +#[test] +fn embedded_values_drawn_in_many_instances_spend_the_draw_budget() { + let value = "word ".repeat(4000); // 20,000 characters + let span = r##""##; + let draw = format!( + r#"

x {span}

"# + ); + let field = format!(r#"{value}"#); + let many = + format!(r#""#); + assert!(matches!(layout_xml(&many), Err(XfaError::TooLarge(_))), "a long embedded value in many instances must be refused"); + // One copy lays out: the budget does not reject an ordinary form. + let one = format!(r#""#); + assert!(layout_xml(&one).is_ok(), "a single draw embedding a long value must lay out"); +} + #[test] fn a_packet_stream_listed_many_times_is_read_once() { let mut doc = Document::open(Arc::new(shell(""))).unwrap(); From 32ab43eab075511949c62ecaf95792e161fa7106 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 5 Nov 2026 07:06:48 +0800 Subject: [PATCH 008/107] fix(xfa): the embedded-copy test allows the draw budget to stop it first With embedded values counted, the 1 MiB test budget can stop this draw before its last pass; the bound on what is copied out still holds. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/xfa/src/tests.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index a55d74498e..29e5e0da0a 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -3992,7 +3992,12 @@ fn embedded_values_are_copied_out_only_as_far_as_the_budget() { "# ); crate::layout::EMBED_COPIED.with(|c| c.set(0)); - layout_xml(&tpl).unwrap(); + // The draw budget counts embedded values too (1 MiB in tests), so it may stop the draw + // before its last pass; either way no more than this is copied out. + match layout_xml(&tpl) { + Ok(_) | Err(XfaError::TooLarge(_)) => {} + Err(e) => panic!("{e:?}"), + } let copied = crate::layout::EMBED_COPIED.with(|c| c.get()); // The draw is measured at most seven times before it is laid out once. assert!(copied <= 8 * text::MAX_EMBEDDED as u64 + 8 * 4 * 201, "{copied} bytes of embedded values copied"); From 54ff776de9d76fb0f0aea2842ac21c228f69f020 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 5 Nov 2026 07:25:53 +0800 Subject: [PATCH 009/107] fix(xfa): fit passes stop once the draw budget is spent, and the last pass is checked A draw's fit passes ran on after one had spent the budget, each a full measure, before the draw was refused; each pass now checks the budget before it starts and after it ends. The final layout pass, which expands the embedded values once more, is checked too, so the draw that crosses the budget there is refused even when it is the form's last. (CodeRabbit review on #5852.) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/xfa/src/layout/draw.rs | 21 ++++++++++++++++++--- crates/xfa/src/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index 9c0b385c63..ace5bb93ac 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -242,14 +242,29 @@ impl Layouter<'_> { // Text that doesn't fit the height it was given (the template's face is narrower than // the standard one drawn in its place, so it wraps to more lines) is drawn smaller, // down to 70%, rather than over what lies below. - let fits = |k: f64| { + // A pass is not started once the budget is spent, and one that spends it stops the rest. + let fits = |k: f64| -> Result { + self.check_drawn_chars()?; self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); - text::measure_height(&rich, rect.w / k, para, &face, &embed) <= rect.h / k + 1.0 + let fit = text::measure_height(&rich, rect.w / k, para, &face, &embed) <= rect.h / k + 1.0; + self.check_drawn_chars()?; + Ok(fit) }; - let scale = if rect.h < 4.0 || fits(1.0) { 1.0 } else { [0.95, 0.9, 0.85, 0.8, 0.75].into_iter().find(|k| fits(*k)).unwrap_or(0.7) }; + let mut scale = 1.0; + if rect.h >= 4.0 && !fits(1.0)? { + scale = 0.7; + for k in [0.95, 0.9, 0.85, 0.8, 0.75] { + if fits(k)? { + scale = k; + break; + } + } + } self.drawn_chars.set(self.drawn_chars.get().saturating_add(cost)); self.check_drawn_chars()?; let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &embed); + // The layout itself expands the embedded values once more. + self.check_drawn_chars()?; if scale < 1.0 { for s in &mut block.spans { s.x = rect.x + (s.x - rect.x) * scale; diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 29e5e0da0a..d1edc8f6d0 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -713,6 +713,40 @@ fn embedded_values_drawn_in_many_instances_spend_the_draw_budget() { assert!(layout_xml(&one).is_ok(), "a single draw embedding a long value must lay out"); } +/// The last layout pass of a draw counts against the budget too: a draw that fits is measured +/// once and laid out once, each pass expanding its embedded value, and when the form's last draw +/// crosses the budget in that final pass it is still refused (CodeRabbit review on #5852). +#[test] +fn the_draw_that_crosses_the_budget_in_its_last_pass_is_refused() { + // 15,000 characters: with the 1 MiB test budget, the tenth such draw crosses it during its + // final layout pass, after every check before that pass has passed. + let value = "word ".repeat(3000); + let span = r##""##; + let form = |n: usize| { + format!( + r#""# + ) + }; + assert!(layout_xml(&form(9)).is_ok()); + assert!(matches!(layout_xml(&form(10)), Err(XfaError::TooLarge(_))), "the draw crossing the budget in its last pass must be refused"); +} + +/// Once a fit pass spends the budget, no more passes are made: a draw that doesn't fit and embeds +/// a long value is measured once, not seven times, before it is refused (CodeRabbit review on +/// #5852). +#[test] +fn fit_passes_stop_once_the_draw_budget_is_spent() { + let value = "word ".repeat(text::MAX_EMBEDDED / 5 + 1000); + let span = r##""##; + let tpl = format!( + r#""# + ); + crate::layout::EMBED_COPIED.with(|c| c.set(0)); + assert!(matches!(layout_xml(&tpl), Err(XfaError::TooLarge(_)))); + let copied = crate::layout::EMBED_COPIED.with(|c| c.get()); + assert!(copied <= 2 * text::MAX_EMBEDDED as u64, "{copied} bytes copied: more passes after the budget was spent"); +} + #[test] fn a_packet_stream_listed_many_times_is_read_once() { let mut doc = Document::open(Arc::new(shell(""))).unwrap(); From 8942b4332a83df94d7240ff6a08526648d169712 Mon Sep 17 00:00:00 2001 From: quincy-jc <184324679+quincy-jc@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:32:11 +1100 Subject: [PATCH 010/107] fix(measure): snap on heavy sheets, off the window thread Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/measure.rs | 2 +- crates/engine/src/document.rs | 10 + crates/measure/README.md | 2 +- crates/measure/examples/snap_timing.rs | 32 +++ crates/measure/src/snap.rs | 328 ++++++++++++++++++++++--- crates/measure/src/tests.rs | 56 +++-- crates/measure/tests/snap_property.rs | 108 +++++++- crates/ui-egui/src/tools/measure_ui.rs | 114 ++++++++- 8 files changed, 577 insertions(+), 75 deletions(-) create mode 100644 crates/measure/examples/snap_timing.rs diff --git a/crates/automation/src/measure.rs b/crates/automation/src/measure.rs index ac3d2d95f1..932f85ad9a 100644 --- a/crates/automation/src/measure.rs +++ b/crates/automation/src/measure.rs @@ -147,7 +147,7 @@ impl Automation { Ok(json!({"point":point,"kind":s.kind,"distance":measure::distance(at,point)})) }) .transpose()?; - Ok(json!({"snap":snap,"truncated":geometry.truncated,"intersection_limited":intersection_limited,"segments":geometry.segments.len()})) + Ok(json!({"snap":snap,"truncated":geometry.truncated,"intersection_limited":intersection_limited,"segments":geometry.segment_count()})) } pub(crate) fn measurement_export(&self, a: &Args) -> Result { let all = self.doc(a)?.measurements().map_err(failed)?; diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index fa98a18417..987a2b627e 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -289,6 +289,16 @@ impl Document { measure::snap::geometry(&e.cos, page).map_err(|e| e.to_string()) } + /// [`Self::measurement_paths`] as work to run away from the UI: a heavy drawing takes + /// seconds to read (millions of segments). It works on a copy of the document as it is now. + pub fn measurement_paths_job(&self, page: usize) -> impl FnOnce() -> Result + Send + 'static { + let cos = self.editor.as_ref().map(|e| e.cos.clone()); + move || { + let cos = cos.ok_or("the document can't be read")?; + measure::snap::geometry(&cos, page).map_err(|e| e.to_string()) + } + } + /// A counter that changes with every edit (for caches of derived data). pub fn edit_generation(&self) -> u64 { self.generation diff --git a/crates/measure/README.md b/crates/measure/README.md index 95a04a2b64..2e438d7a1e 100644 --- a/crates/measure/README.md +++ b/crates/measure/README.md @@ -8,7 +8,7 @@ Scale::calibrate derives a conversion from two points and a known real-world dis The desktop Measure objects panel exposes all three tools, live values and coordinate deltas, 45-degree constraints, vertex cancellation, snapping, calibration and CSV export. Automation provides measure_distance, measure_perimeter, measure_area, measure_info, measure_list, measure_scale, measure_snap and measure_export. The panel's drawing scale and measure_scale's `page_distance`/`page_unit` give the page side in a unit, by default the real-world one. Tool coordinates and explicit units-per-point scales use physical display points. -Snapping interprets painted paths and nested Form XObjects, including graphics-state transforms. Cubic paths are flattened to a maximum deviation of 0.2 user units. Raster edges, glyph outlines, clipping and optional-content visibility are not interpreted. Extraction stops at 20,000 segments, 40,000 endpoint and midpoint targets, 16 MiB of decoded content, 4,096 streams, 32 nested forms or 256 nested `q`; intersection searches use at most 256 nearby segments. Extraction is lenient: a content stream that can't be decoded is skipped and counted, and the rest of the page still snaps. The UI and automation report these limits. +Snapping interprets painted paths and nested Form XObjects, including graphics-state transforms. Cubic paths are flattened to a maximum deviation of 0.2 user units. Raster edges, glyph outlines, clipping and optional-content visibility are not interpreted. Extraction stops at 8,000,000 segments, 24,000,000 endpoint and midpoint targets, 512 MiB of decoded content, 4,096 streams, 32 nested forms or 256 nested `q`; intersection searches use at most 256 nearby segments. Extraction is lenient: a content stream that can't be decoded is skipped and counted, and the rest of the page still snaps. The UI and automation report these limits. Imported single-unit decimal rectilinear scales support differing X and Y conversion factors when CYX is present. Unit labels are text strings and may be non-ASCII ("m²"). Compound units (such as feet and inches), fraction formats and non-rectilinear scales are not supported: `list` reports such measurements, and ones with invalid geometry, under `unsupported` with a reason instead of failing, and leaves their PDF data untouched. A polygon whose last vertex repeats the first is treated as closed. Interoperability with Acrobat-authored measurements is untested. Geospatial and 3D measuring are outside this component. diff --git a/crates/measure/examples/snap_timing.rs b/crates/measure/examples/snap_timing.rs new file mode 100644 index 0000000000..319434f8cd --- /dev/null +++ b/crates/measure/examples/snap_timing.rs @@ -0,0 +1,32 @@ +//! Times extracting a page's snapping geometry and snapping at points on it. +//! +//! Usage: `snap_timing FILE PAGE` (PAGE is 1-based). + +use std::time::Instant; + +fn main() -> Result<(), Box> { + let args: Vec = std::env::args().collect(); + let (Some(path), Some(page)) = (args.get(1), args.get(2)) else { + return Err("usage: snap_timing FILE PAGE".into()); + }; + let page: usize = page.parse::()?.checked_sub(1).ok_or("pages are numbered from 1")?; + let bytes = std::sync::Arc::new(std::fs::read(path)?); + let doc = pdfcraft_cos::Document::open(bytes).map_err(|e| format!("{e:?}"))?; + let start = Instant::now(); + let g = pdfcraft_measure::snap::geometry(&doc, page)?; + let extracted = start.elapsed(); + println!("extracted {} segments in {} ms: truncated {} unreadable {}", g.segment_count(), extracted.as_millis(), g.truncated, g.unreadable); + // Snaps at points spread over the drawing, with a 6-unit tolerance (a few pixels on screen). + let points: Vec<[f64; 2]> = + g.segments().step_by((g.segment_count() / 2000).max(1)).map(|[a, b]| [(a[0] + b[0]) / 2.0 + 0.7, (a[1] + b[1]) / 2.0 - 0.4]).collect(); + let start = Instant::now(); + let mut hits = 0; + for &p in &points { + if g.snap(p, 6.0, pdfcraft_measure::snap::SnapOptions::default())?.is_some() { + hits += 1; + } + } + let per = start.elapsed().as_secs_f64() * 1e6 / points.len().max(1) as f64; + println!("{} snaps, {hits} hits, {per:.0} us per snap", points.len()); + Ok(()) +} diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index 930ffe3053..661b083187 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -5,19 +5,42 @@ use pdfcraft_cos::{Dict, Document, ObjRef, Object}; use serde::Serialize; use std::collections::HashSet; -const MAX_SEGMENTS: usize = 20_000; -/// Endpoints and midpoints together, across the whole page. -const MAX_TARGETS: usize = 2 * MAX_SEGMENTS; -const MAX_BYTES: usize = 16 * 1024 * 1024; +/// Open subpaths remembered for a fill, which closes each of them. +const MAX_OPEN_SUBPATHS: usize = Limits::DEFAULT.segments; +/// How much of a page extraction reads and keeps. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Limits { + /// Straight segments (curves are split into several). + pub segments: usize, + /// Endpoints and midpoints together, across the whole page. + pub targets: usize, + /// Decoded content bytes, across every stream read. + pub bytes: usize, +} +impl Limits { + /// Enough for heavy CAD sheets (several million segments), and + /// the renderer's own content limit. At about 40 bytes a segment, the cap holds ~320 MB. + pub const DEFAULT: Self = Self { segments: 8_000_000, targets: 24_000_000, bytes: 512 << 20 }; +} +impl Default for Limits { + fn default() -> Self { + Self::DEFAULT + } +} +/// The page's drawn geometry in user space, compactly (`f32`: about 0.0003 units of precision at +/// A0 sizes), with a grid index so that a snap looks only near the pointer. #[derive(Clone, Debug, Default)] pub struct Geometry { - pub segments: Vec<[Point; 2]>, - pub endpoints: Vec, - pub midpoints: Vec, + /// Segments as `[ax, ay, bx, by]`. + segs: Vec<[f32; 4]>, + /// Snap targets: a straight edge's or curve's two ends, then its midpoint. + tgts: Vec<[f32; 6]>, /// A limit stopped extraction early (segments, targets, bytes, streams, nesting). pub truncated: bool, /// Content streams that couldn't be decoded and were skipped. pub unreadable: usize, + limits: Limits, + grid: Grid, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] #[serde(rename_all = "lowercase")] @@ -80,7 +103,7 @@ fn remember_close(closures: &mut Vec<[Point; 2]>, current: Option, start: if let (Some(a), Some(b)) = (current, start) && distance(a, b) > 1e-9 { - if closures.len() >= MAX_SEGMENTS { + if closures.len() >= MAX_OPEN_SUBPATHS { return false; } closures.push([a, b]); @@ -88,10 +111,42 @@ fn remember_close(closures: &mut Vec<[Point; 2]>, current: Option, start: true } +fn seg(s: &[f32; 4]) -> [Point; 2] { + [[f64::from(s[0]), f64::from(s[1])], [f64::from(s[2]), f64::from(s[3])]] +} +fn pt(x: f32, y: f32) -> Point { + [f64::from(x), f64::from(y)] +} impl Geometry { + fn with_limits(limits: Limits) -> Self { + Self { limits, ..Self::default() } + } + /// How many segments were kept. + pub fn segment_count(&self) -> usize { + self.segs.len() + } + /// The segments, in drawing order. + pub fn segments(&self) -> impl ExactSizeIterator + '_ { + self.segs.iter().map(seg) + } + /// Snap endpoints: each edge's or curve's two ends, in drawing order. + pub fn endpoints(&self) -> impl Iterator + '_ { + self.tgts.iter().flat_map(|t| [pt(t[0], t[1]), pt(t[2], t[3])]) + } + /// Snap midpoints: each edge's or curve's middle, in drawing order. + pub fn midpoints(&self) -> impl Iterator + '_ { + self.tgts.iter().map(|t| pt(t[4], t[5])) + } /// Dense intersection searches are capped independently from path extraction. pub fn intersection_limited(&self, at: Point, tolerance: f64) -> bool { - self.segments.iter().filter(|[a, b]| distance(at, nearest(at, *a, *b)) <= tolerance).take(257).count() > 256 + let (segs, _) = self.grid.near(self, at, tolerance); + segs.iter() + .filter_map(|&i| self.segs.get(i as usize)) + .map(seg) + .filter(|[a, b]| distance(at, nearest(at, *a, *b)) <= tolerance) + .take(257) + .count() + > 256 } pub fn snap(&self, at: Point, tolerance: f64, options: SnapOptions) -> Result> { check_points(&[at])?; @@ -106,19 +161,25 @@ impl Geometry { *best = Some(Snap { point: p, kind, distance }); } }; + // Only what the grid finds near the pointer: everything within `tolerance` is there, in + // drawing order, so the result is the one a scan of the whole page would give. + let (segs, tgts) = self.grid.near(self, at, tolerance); + let targets: Vec<&[f32; 6]> = tgts.iter().filter_map(|&i| self.tgts.get(i as usize)).collect(); if options.endpoints { - for &p in &self.endpoints { - consider(&mut best, p, SnapKind::Endpoint); + for t in &targets { + consider(&mut best, pt(t[0], t[1]), SnapKind::Endpoint); + consider(&mut best, pt(t[2], t[3]), SnapKind::Endpoint); } } if options.midpoints { - for &p in &self.midpoints { - consider(&mut best, p, SnapKind::Midpoint); + for t in &targets { + consider(&mut best, pt(t[4], t[5]), SnapKind::Midpoint); } } + let near: Vec<[Point; 2]> = segs.iter().filter_map(|&i| self.segs.get(i as usize)).map(seg).collect(); // Only segments in the pointer's neighbourhood participate in intersection tests. // Cap the local quadratic work even on a malicious densely overlapping drawing. - let local: Vec<_> = self.segments.iter().filter(|[a, b]| distance(at, nearest(at, *a, *b)) <= tolerance).take(256).collect(); + let local: Vec<_> = near.iter().filter(|[a, b]| distance(at, nearest(at, *a, *b)) <= tolerance).take(256).collect(); if options.intersections { for (i, s) in local.iter().enumerate() { for t in local.iter().skip(i + 1) { @@ -129,35 +190,36 @@ impl Geometry { } } if options.paths && best.is_none() { - for [a, b] in &self.segments { + for [a, b] in &near { consider(&mut best, nearest(at, *a, *b), SnapKind::Path); } } Ok(best) } + /// Endpoints and midpoints together. fn targets(&self) -> usize { - self.endpoints.len().saturating_add(self.midpoints.len()) + self.tgts.len().saturating_mul(3) } /// Whether the segment was accepted (finite, non-degenerate and within the cap). fn segment(&mut self, a: Point, b: Point) -> bool { - if self.segments.len() >= MAX_SEGMENTS { + if self.segs.len() >= self.limits.segments { self.truncated = true; return false; } if check_points(&[a, b]).is_ok() && distance(a, b) > 1e-9 { - self.segments.push([a, b]); + self.segs.push([a[0] as f32, a[1] as f32, b[0] as f32, b[1] as f32]); return true; } false } /// Snap targets are recorded only for accepted geometry, and are capped. fn target(&mut self, ends: [Point; 2], mid: Point) { - if self.targets() >= MAX_TARGETS { + if self.targets() >= self.limits.targets { self.truncated = true; return; } - self.endpoints.extend(ends); - self.midpoints.push(mid); + let [a, b] = ends; + self.tgts.push([a[0] as f32, a[1] as f32, b[0] as f32, b[1] as f32, mid[0] as f32, mid[1] as f32]); } fn edge(&mut self, a: Point, b: Point) { if self.segment(a, b) { @@ -166,7 +228,7 @@ impl Geometry { } fn curve(&mut self, points: [Point; 4], depth: usize) { let [a, b, c, d] = points; - if self.segments.len() >= MAX_SEGMENTS { + if self.segs.len() >= self.limits.segments { self.truncated = true; return; } @@ -195,6 +257,7 @@ struct Walker<'a> { seen: HashSet, bytes: usize, streams: usize, + limits: Limits, } impl Walker<'_> { fn walk(&mut self, object: &Object, resources: &Dict, initial: Matrix, depth: usize) -> Result<()> { @@ -209,7 +272,7 @@ impl Walker<'_> { for part in arr { let s = self.doc.resolve(part); if let Object::Stream(s) = s.as_ref() { - let remaining = MAX_BYTES.saturating_sub(self.bytes).saturating_sub(joined.len()); + let remaining = self.limits.bytes.saturating_sub(self.bytes).saturating_sub(joined.len()); if remaining == 0 { self.geometry.truncated = true; break; @@ -228,7 +291,7 @@ impl Walker<'_> { return self.ops(&joined, resources, initial, depth); } let Object::Stream(stream) = o.as_ref() else { return Ok(()) }; - let remaining = MAX_BYTES.saturating_sub(self.bytes); + let remaining = self.limits.bytes.saturating_sub(self.bytes); if remaining == 0 { self.geometry.truncated = true; return Ok(()); @@ -255,7 +318,7 @@ impl Walker<'_> { let mut start = None; let mut current = None; let mut closures = Vec::new(); - let mut path = Geometry::default(); + let mut path = Geometry::with_limits(self.limits); // Read on the first `Do` and kept: resolving a direct dictionary copies it. let mut xobjects = None; let point = |m: Matrix, x: f64, y: f64| { @@ -263,8 +326,8 @@ impl Walker<'_> { [x, y] }; for op in parse(data).ops { - if self.geometry.segments.len().saturating_add(path.segments.len()) >= MAX_SEGMENTS - || self.geometry.targets().saturating_add(path.targets()) >= MAX_TARGETS + if self.geometry.segs.len().saturating_add(path.segs.len()) >= self.limits.segments + || self.geometry.targets().saturating_add(path.targets()) >= self.limits.targets { self.geometry.truncated = true; break; @@ -320,9 +383,9 @@ impl Walker<'_> { if let Some([a, b, c, d]) = curve && check_points(&[a, b, c, d]).is_ok() { - let before = path.segments.len(); + let before = path.segs.len(); path.curve([a, b, c, d], 0); - if path.segments.len() > before { + if path.segs.len() > before { path.target([a, d], [(a[0] + 3.0 * b[0] + 3.0 * c[0] + d[0]) / 8.0, (a[1] + 3.0 * b[1] + 3.0 * c[1] + d[1]) / 8.0]); } current = Some(d); @@ -362,8 +425,8 @@ impl Walker<'_> { // Filling implicitly closes every open subpath, not just the last one. // Stroke-only S leaves its open subpaths unchanged. for [a, b] in closures.drain(..) { - if self.geometry.segments.len().saturating_add(path.segments.len()) >= MAX_SEGMENTS - || self.geometry.targets().saturating_add(path.targets()) >= MAX_TARGETS + if self.geometry.segs.len().saturating_add(path.segs.len()) >= self.limits.segments + || self.geometry.targets().saturating_add(path.targets()) >= self.limits.targets { path.truncated = true; break; @@ -375,9 +438,8 @@ impl Walker<'_> { { path.edge(a, b); } - self.geometry.segments.append(&mut path.segments); - self.geometry.endpoints.append(&mut path.endpoints); - self.geometry.midpoints.append(&mut path.midpoints); + self.geometry.segs.append(&mut path.segs); + self.geometry.tgts.append(&mut path.tgts); self.geometry.truncated |= path.truncated; closures.clear(); current = None; @@ -385,7 +447,7 @@ impl Walker<'_> { } "n" => { self.geometry.truncated |= path.truncated; - path = Geometry::default(); + path = Geometry::with_limits(self.limits); closures.clear(); current = None; start = None; @@ -438,15 +500,207 @@ thread_local! { pub(crate) static XOBJECT_DICT_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; } +/// The page's geometry for snapping, within [`Limits::DEFAULT`]. pub fn geometry(doc: &Document, page_index: usize) -> Result { + geometry_with_limits(doc, page_index, Limits::DEFAULT) +} + +/// The page's geometry for snapping, within `limits`. +pub fn geometry_with_limits(doc: &Document, page_index: usize, limits: Limits) -> Result { let p = crate::page(doc, page_index)?; let resources = p.dict.get(b"Resources").map(|o| doc.resolve(o)); let empty = Dict::new(); let resources = resources.as_ref().and_then(|o| o.as_dict()).unwrap_or(&empty); - let mut walker = Walker { doc, geometry: Geometry::default(), seen: HashSet::new(), bytes: 0, streams: 0 }; + let mut walker = Walker { doc, geometry: Geometry::with_limits(limits), seen: HashSet::new(), bytes: 0, streams: 0, limits }; // Extraction is lenient: limits and unreadable streams are reported in the geometry. if let Some(contents) = p.dict.get(b"Contents") { walker.walk(contents, resources, Matrix::IDENTITY, 0)?; } - Ok(walker.geometry) + let mut geometry = walker.geometry; + geometry.segs.shrink_to_fit(); + geometry.tgts.shrink_to_fit(); + geometry.grid = Grid::build(&geometry); + Ok(geometry) +} + +/// The most cells a side of the grid has. +const GRID_SIDE: usize = 1024; +/// A segment whose box covers more cells than this is kept in one list checked at every snap +/// (a long line would otherwise fill a whole row of cells). +const WIDE_CELLS: usize = 64; +/// The most cells one snap reads; a larger search area (a huge tolerance) scans everything. +const MAX_QUERY_CELLS: usize = 4096; + +/// A uniform grid over the geometry's box: per cell, the segments and targets touching it. +#[derive(Clone, Debug, Default)] +struct Grid { + x0: f32, + y0: f32, + cell: f32, + cols: usize, + rows: usize, + /// Per cell, where its segment indices start in `seg_items` (one more entry than cells). + seg_start: Vec, + seg_items: Vec, + tgt_start: Vec, + tgt_items: Vec, + /// Segments too long for the cells. + wide: Vec, +} + +impl Grid { + fn build(g: &Geometry) -> Self { + let mut bounds: Option<[f32; 4]> = None; + let mut grow = |x: f32, y: f32| { + let b = bounds.get_or_insert([x, y, x, y]); + *b = [b[0].min(x), b[1].min(y), b[2].max(x), b[3].max(y)]; + }; + for s in &g.segs { + grow(s[0], s[1]); + grow(s[2], s[3]); + } + for t in &g.tgts { + grow(t[0], t[1]); + grow(t[2], t[3]); + grow(t[4], t[5]); + } + let Some([x0, y0, x1, y1]) = bounds else { return Self::default() }; + let items = g.segs.len().max(g.tgts.len()).max(1); + // About eight items a cell. + let side = ((items as f64 / 8.0).sqrt().ceil() as usize).clamp(1, GRID_SIDE); + let extent = (x1 - x0).max(y1 - y0).max(1e-3); + let cell = extent / side as f32; + let cols = (((x1 - x0) / cell) as usize + 1).min(GRID_SIDE); + let rows = (((y1 - y0) / cell) as usize + 1).min(GRID_SIDE); + let mut grid = Self { x0, y0, cell, cols, rows, ..Self::default() }; + let cells = cols * rows; + // Segments: count, then fill (compressed rows). + let seg_cells = |s: &[f32; 4]| grid.range(s[0].min(s[2]), s[1].min(s[3]), s[0].max(s[2]), s[1].max(s[3])); + let mut counts = vec![0u32; cells]; + let mut wide = Vec::new(); + for (i, s) in g.segs.iter().enumerate() { + let (c0, r0, c1, r1) = seg_cells(s); + if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + wide.push(i as u32); + continue; + } + for r in r0..=r1 { + for c in c0..=c1 { + counts[r * cols + c] += 1; + } + } + } + let (seg_start, mut fill) = Self::starts(&counts); + let mut seg_items = vec![0u32; seg_start.last().copied().unwrap_or(0) as usize]; + for (i, s) in g.segs.iter().enumerate() { + let (c0, r0, c1, r1) = seg_cells(s); + if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + continue; + } + for r in r0..=r1 { + for c in c0..=c1 { + let k = r * cols + c; + seg_items[fill[k] as usize] = i as u32; + fill[k] += 1; + } + } + } + // Targets: the cells of their three points. + let tgt_cells = |t: &[f32; 6]| { + let mut cs = [grid.cell_of(t[0], t[1]), grid.cell_of(t[2], t[3]), grid.cell_of(t[4], t[5])]; + cs.sort_unstable(); + cs + }; + let mut counts = vec![0u32; cells]; + for t in &g.tgts { + let cs = tgt_cells(t); + for (j, &k) in cs.iter().enumerate() { + if j == 0 || cs[j - 1] != k { + counts[k] += 1; + } + } + } + let (tgt_start, mut fill) = Self::starts(&counts); + let mut tgt_items = vec![0u32; tgt_start.last().copied().unwrap_or(0) as usize]; + for (i, t) in g.tgts.iter().enumerate() { + let cs = tgt_cells(t); + for (j, &k) in cs.iter().enumerate() { + if j == 0 || cs[j - 1] != k { + tgt_items[fill[k] as usize] = i as u32; + fill[k] += 1; + } + } + } + grid.seg_start = seg_start; + grid.seg_items = seg_items; + grid.tgt_start = tgt_start; + grid.tgt_items = tgt_items; + grid.wide = wide; + grid + } + + /// Prefix sums of `counts` (the starts), and a copy to fill from. + fn starts(counts: &[u32]) -> (Vec, Vec) { + let mut start = Vec::with_capacity(counts.len() + 1); + let mut total = 0u32; + start.push(0); + for &c in counts { + total = total.saturating_add(c); + start.push(total); + } + let fill = start[..counts.len()].to_vec(); + (start, fill) + } + + fn col(&self, x: f32) -> usize { + (((x - self.x0) / self.cell).max(0.0) as usize).min(self.cols.saturating_sub(1)) + } + fn row(&self, y: f32) -> usize { + (((y - self.y0) / self.cell).max(0.0) as usize).min(self.rows.saturating_sub(1)) + } + fn cell_of(&self, x: f32, y: f32) -> usize { + self.row(y) * self.cols + self.col(x) + } + /// The cells (first and last column, first and last row) a box covers. + fn range(&self, x0: f32, y0: f32, x1: f32, y1: f32) -> (usize, usize, usize, usize) { + (self.col(x0), self.row(y0), self.col(x1), self.row(y1)) + } + + /// The segments and targets near `at` (all within `tolerance` of it, and maybe more), each + /// list in drawing order without repeats. + fn near(&self, g: &Geometry, at: Point, tolerance: f64) -> (Vec, Vec) { + let all = || ((0..g.segs.len() as u32).collect(), (0..g.tgts.len() as u32).collect()); + if self.cols == 0 || self.rows == 0 { + return if g.segs.is_empty() && g.tgts.is_empty() { (Vec::new(), Vec::new()) } else { all() }; + } + let t = tolerance as f32; + let (x, y) = (at[0] as f32, at[1] as f32); + if !(x.is_finite() && y.is_finite() && t.is_finite()) { + return all(); + } + let (c0, r0, c1, r1) = self.range(x - t, y - t, x + t, y + t); + // Entirely outside the grid: nothing is near (except wide segments). + let outside = + x + t < self.x0 || y + t < self.y0 || x - t > self.x0 + self.cell * self.cols as f32 || y - t > self.y0 + self.cell * self.rows as f32; + if (c1 - c0 + 1) * (r1 - r0 + 1) > MAX_QUERY_CELLS { + return all(); + } + let mut segs: Vec = self.wide.clone(); + let mut tgts: Vec = Vec::new(); + if !outside { + for r in r0..=r1 { + for c in c0..=c1 { + let k = r * self.cols + c; + let range = |start: &[u32]| (start[k] as usize)..(start[k + 1] as usize); + segs.extend_from_slice(self.seg_items.get(range(&self.seg_start)).unwrap_or(&[])); + tgts.extend_from_slice(self.tgt_items.get(range(&self.tgt_start)).unwrap_or(&[])); + } + } + } + segs.sort_unstable(); + segs.dedup(); + tgts.sort_unstable(); + tgts.dedup(); + (segs, tgts) + } } diff --git a/crates/measure/src/tests.rs b/crates/measure/src/tests.rs index 5b277e5546..2fb589d23e 100644 --- a/crates/measure/src/tests.rs +++ b/crates/measure/src/tests.rs @@ -112,7 +112,7 @@ fn filled_open_triangle_snaps_to_closing_edge() { for (op, closed) in [("f", true), ("F", true), ("f*", true), ("B", true), ("B*", true), ("h f", true), ("S", false)] { let d = fixture(&format!("40 50 m 160 50 l 100 170 l {op}"), "", &[]); let paths = snap::geometry(&d, 0).unwrap(); - assert_eq!(paths.segments.len(), if closed { 3 } else { 2 }, "{op}"); + assert_eq!(paths.segment_count(), if closed { 3 } else { 2 }, "{op}"); let hit = paths.snap([70.0, 110.0], 5.0, snap::SnapOptions::default()).unwrap(); assert_eq!(hit.is_some(), closed, "{op}"); } @@ -121,7 +121,7 @@ fn filled_open_triangle_snaps_to_closing_edge() { fn snap_paths_endpoints_midpoints_and_intersections() { let d = fixture("10 20 m 110 20 l S 60 0 m 60 80 l S 10 100 100 40 re S", "", &[]); let paths = snap::geometry(&d, 0).unwrap(); - assert_eq!(paths.segments.len(), 6); + assert_eq!(paths.segment_count(), 6); let opts = snap::SnapOptions::default(); let hit = paths.snap([11.0, 20.0], 3.0, opts).unwrap().unwrap(); assert_eq!(hit.kind, snap::SnapKind::Endpoint); @@ -147,9 +147,9 @@ fn filled_open_subpath_offers_the_same_snap_edges_as_explicit_close() { let explicit = snap::geometry(&closed, 0).unwrap(); let stroked = snap::geometry(&stroke, 0).unwrap(); - assert_eq!(filled.segments, explicit.segments); - assert_eq!(filled.segments.len(), 3); - assert_eq!(stroked.segments.len(), 2); + assert!(filled.segments().eq(explicit.segments())); + assert_eq!(filled.segment_count(), 3); + assert_eq!(stroked.segment_count(), 2); let paths_only = snap::SnapOptions { endpoints: false, midpoints: false, intersections: false, paths: true }; let hit = filled.snap([55.0, 80.0], 5.0, paths_only).unwrap().unwrap(); assert_eq!(hit.kind, snap::SnapKind::Path); @@ -163,12 +163,12 @@ fn fills_close_all_open_subpaths_but_strokes_do_not() { for paint in ["f", "F", "f*", "B", "B*", "b", "b*"] { let doc = fixture(&format!("{triangles} {paint}"), "", &[]); let geometry = snap::geometry(&doc, 0).unwrap(); - assert_eq!(geometry.segments.len(), 6, "{paint}"); - assert!(geometry.segments.contains(&[[10.0, 20.0], [0.0, 0.0]]), "{paint}"); - assert!(geometry.segments.contains(&[[50.0, 20.0], [40.0, 0.0]]), "{paint}"); + assert_eq!(geometry.segment_count(), 6, "{paint}"); + assert!(geometry.segments().any(|s| s == [[10.0, 20.0], [0.0, 0.0]]), "{paint}"); + assert!(geometry.segments().any(|s| s == [[50.0, 20.0], [40.0, 0.0]]), "{paint}"); } let stroked = fixture(&format!("{triangles} S"), "", &[]); - assert_eq!(snap::geometry(&stroked, 0).unwrap().segments.len(), 4); + assert_eq!(snap::geometry(&stroked, 0).unwrap().segment_count(), 4); } #[test] @@ -180,12 +180,12 @@ fn nested_forms_transforms_curves_and_cycles_are_bounded() { ); let d = fixture("q 1 0 0 1 100 200 cm /Nested Do Q", "", &[&form]); let geometry = snap::geometry(&d, 0).unwrap(); - assert_eq!(geometry.segments[0], [[100.0, 200.0], [120.0, 200.0]]); - assert!(geometry.endpoints.contains(&[120.0, 200.0])); - assert!(geometry.midpoints.contains(&[110.0, 222.5])); - assert!(geometry.segments.len() > 4 && geometry.segments.len() < 200); + assert_eq!(geometry.segments().next(), Some([[100.0, 200.0], [120.0, 200.0]])); + assert!(geometry.endpoints().any(|p| p == [120.0, 200.0])); + assert!(geometry.midpoints().any(|p| p == [110.0, 222.5])); + assert!(geometry.segment_count() > 4 && geometry.segment_count() < 200); let d = fixture(&"0 0 m 1 1 l S ".repeat(20_001), "", &[]); - assert!(snap::geometry(&d, 0).unwrap().truncated); + assert!(snap::geometry_with_limits(&d, 0, SMALL).unwrap().truncated); } #[test] fn measurement_csv_quotes_newlines_and_blocks_formula_cells() { @@ -242,35 +242,39 @@ fn imported_measurement_appearance_is_not_silently_replaced() { close(list(&d).measurements[0].reading.value, 100.0); } +/// The limits snapping had before heavy sheets, which the cap tests below were written for. +const SMALL: snap::Limits = snap::Limits { segments: 20_000, targets: 40_000, bytes: 16 << 20 }; + #[test] fn degenerate_and_dense_paths_keep_snap_targets_bounded() { // Rejected (zero-length) segments record no endpoints or midpoints at all. let d = fixture(&"0 0 0 0 re f ".repeat(100_000), "", &[]); - let g = snap::geometry(&d, 0).unwrap(); - assert!(g.segments.is_empty() && g.endpoints.is_empty() && g.midpoints.is_empty(), "{} {}", g.endpoints.len(), g.midpoints.len()); + let g = snap::geometry_with_limits(&d, 0, SMALL).unwrap(); + assert!(g.segment_count() == 0 && g.endpoints().next().is_none() && g.midpoints().next().is_none()); // Every paint drains into the page geometry; the page-wide target cap still holds. let d = fixture(&"0 0 1 1 re f ".repeat(30_000), "", &[]); - let g = snap::geometry(&d, 0).unwrap(); + let g = snap::geometry_with_limits(&d, 0, SMALL).unwrap(); assert!(g.truncated); - assert!(g.endpoints.len() + g.midpoints.len() <= 40_000 + 12, "{}", g.endpoints.len() + g.midpoints.len()); - assert!(g.segments.len() <= 20_000 + 4); + let targets = g.endpoints().count() + g.midpoints().count(); + assert!(targets <= 40_000 + 12, "{targets}"); + assert!(g.segment_count() <= 20_000 + 4); // Unpainted curves and lines are capped too. let d = fixture(&"0 0 m 5 9 1 9 7 0 c 0 3 l ".repeat(30_000), "", &[]); - let g = snap::geometry(&d, 0).unwrap(); - assert!(g.truncated && g.endpoints.is_empty()); + let g = snap::geometry_with_limits(&d, 0, SMALL).unwrap(); + assert!(g.truncated && g.endpoints().next().is_none()); } #[test] fn deep_graphics_state_and_unreadable_streams_are_skipped_not_fatal() { let d = fixture(&format!("0 0 m 10 0 l S {}", "q ".repeat(300)), "", &[]); let g = snap::geometry(&d, 0).unwrap(); assert!(g.truncated); - assert_eq!(g.segments.len(), 1); + assert_eq!(g.segment_count(), 1); // An unsupported filter cannot be decoded at all (corrupt Flate is decoded tolerantly). let broken = "<< /Length 10 /Filter /NoSuchDecode >>\nstream\nnot-zlib!!\nendstream"; let d = fixture_with("[4 0 R 5 0 R]", "0 0 m 10 0 l S", "", &[broken]); let g = snap::geometry(&d, 0).unwrap(); assert_eq!(g.unreadable, 1); - assert_eq!(g.segments.len(), 1); + assert_eq!(g.segment_count(), 1); let d = fixture_with("5 0 R", "", "", &[broken]); assert_eq!(snap::geometry(&d, 0).unwrap().unreadable, 1); } @@ -410,7 +414,7 @@ fn xobject_resources_are_read_once_per_stream() { let d = fixture(&"/Missing Do\n".repeat(10_000), "", &[]); snap::XOBJECT_DICT_READS.with(|c| c.set(0)); let paths = snap::geometry(&d, 0).unwrap(); - assert!(paths.segments.is_empty()); + assert_eq!(paths.segment_count(), 0); let reads = snap::XOBJECT_DICT_READS.with(std::cell::Cell::get); assert!(reads <= 1, "{reads} reads of the page's XObject resources"); } @@ -456,8 +460,8 @@ fn a_stream_over_the_byte_budget_uses_it_up_once() { let bomb = String::from_utf8(pdfcraft_testkit::stream_obj("/Filter [/ASCIIHexDecode /FlateDecode]", hex)).unwrap(); let contents = format!("[4 0 R{}]", " 5 0 R".repeat(50)); let d = fixture_with(&contents, "0 0 m 10 0 l S", "", &[bomb.as_str()]); - let g = snap::geometry(&d, 0).unwrap(); + let g = snap::geometry_with_limits(&d, 0, SMALL).unwrap(); assert_eq!(g.unreadable, 1); assert!(g.truncated); - assert_eq!(g.segments.len(), 1, "the parts before it still snap"); + assert_eq!(g.segment_count(), 1, "the parts before it still snap"); } diff --git a/crates/measure/tests/snap_property.rs b/crates/measure/tests/snap_property.rs index a38bb80aaf..ddb071dc33 100644 --- a/crates/measure/tests/snap_property.rs +++ b/crates/measure/tests/snap_property.rs @@ -131,18 +131,20 @@ fn finite(point: [f64; 2]) -> bool { fn geometry_of(content: &[u8]) -> Geometry { let doc = Document::open(Arc::new(one_page_pdf(content))).unwrap(); - snap::geometry(&doc, 0).unwrap() + // The caps these properties were written for. + let limits = snap::Limits { segments: MAX_GEOMETRY_SEGMENTS, targets: 2 * MAX_GEOMETRY_SEGMENTS, bytes: 16 << 20 }; + snap::geometry_with_limits(&doc, 0, limits).unwrap() } /// Checks the bounds and finiteness that snapping relies on, and returns the extracted geometry. fn check_snap_invariants(content: &[u8]) -> Geometry { assert!(content.len() <= MAX_CONTENT_BYTES, "generated content exceeded the byte cap"); let geometry = geometry_of(content); - assert!(geometry.segments.len() <= MAX_GEOMETRY_SEGMENTS, "geometry exceeded the segment cap: {}", geometry.segments.len()); - assert!(geometry.endpoints.len().saturating_add(geometry.midpoints.len()) <= MAX_SNAP_TARGETS, "geometry exceeded the snap-target cap"); - assert!(geometry.segments.iter().flatten().copied().all(finite), "geometry contains a non-finite segment coordinate"); - assert!(geometry.endpoints.iter().copied().all(finite), "geometry contains a non-finite endpoint"); - assert!(geometry.midpoints.iter().copied().all(finite), "geometry contains a non-finite midpoint"); + assert!(geometry.segment_count() <= MAX_GEOMETRY_SEGMENTS, "geometry exceeded the segment cap: {}", geometry.segment_count()); + assert!(geometry.endpoints().count().saturating_add(geometry.midpoints().count()) <= MAX_SNAP_TARGETS, "geometry exceeded the snap-target cap"); + assert!(geometry.segments().flatten().all(finite), "geometry contains a non-finite segment coordinate"); + assert!(geometry.endpoints().all(finite), "geometry contains a non-finite endpoint"); + assert!(geometry.midpoints().all(finite), "geometry contains a non-finite midpoint"); geometry.intersection_limited([0.0, 0.0], 10_000.0); let snap = geometry.snap([0.0, 0.0], 10_000.0, SnapOptions::default()).unwrap(); @@ -173,7 +175,7 @@ fn mutated_snap_geometry_stays_bounded_and_finite() { fn high_curvature_geometry_hits_segment_cap() { let stress_curve = b"0 0 m 0 100000000 100000000 100000000 100000000 0 c S ".repeat(6); let geometry = check_snap_invariants(&stress_curve); - assert!(geometry.truncated, "high-curvature geometry did not report its extraction cap ({} segments)", geometry.segments.len()); + assert!(geometry.truncated, "high-curvature geometry did not report its extraction cap ({} segments)", geometry.segment_count()); } fn two_piece_pdf(first: &[u8], second: &[u8]) -> Vec { @@ -216,3 +218,95 @@ fn trailing_comment_in_previous_piece_hides_next_piece_first_line() { assert!(snap_at_box_corner(b"q % tail comment, no end of line").is_none(), "snapped to a rectangle the renderer comments out"); assert!(snap_at_box_corner(b"q % tail comment\n").is_some(), "control: the rectangle is drawn"); } + +/// A snap computed by scanning the whole page, as snapping did before its grid index: the +/// reference the indexed snap must equal. +fn snap_by_scan(g: &Geometry, at: [f64; 2], tolerance: f64, options: SnapOptions) -> Option { + let distance = |a: [f64; 2], b: [f64; 2]| (a[0] - b[0]).hypot(a[1] - b[1]); + let nearest = |p: [f64; 2], a: [f64; 2], b: [f64; 2]| { + let (dx, dy) = (b[0] - a[0], b[1] - a[1]); + let len2 = dx * dx + dy * dy; + if len2 <= f64::EPSILON { + return a; + } + let t = (((p[0] - a[0]) * dx + (p[1] - a[1]) * dy) / len2).clamp(0.0, 1.0); + [a[0] + t * dx, a[1] + t * dy] + }; + let mut best: Option = None; + let consider = |best: &mut Option, p: [f64; 2], kind: snap::SnapKind| { + let d = distance(at, p); + if d <= tolerance && best.is_none_or(|b| d < b.distance - 1e-9) { + *best = Some(snap::Snap { point: p, kind, distance: d }); + } + }; + if options.endpoints { + g.endpoints().for_each(|p| consider(&mut best, p, snap::SnapKind::Endpoint)); + } + if options.midpoints { + g.midpoints().for_each(|p| consider(&mut best, p, snap::SnapKind::Midpoint)); + } + let local: Vec<[[f64; 2]; 2]> = g.segments().filter(|[a, b]| distance(at, nearest(at, *a, *b)) <= tolerance).take(256).collect(); + if options.intersections { + for (i, s) in local.iter().enumerate() { + for t in local.iter().skip(i + 1) { + if let Some(p) = snap::intersection(s[0], s[1], t[0], t[1]) { + consider(&mut best, p, snap::SnapKind::Intersection); + } + } + } + } + if options.paths && best.is_none() { + g.segments().for_each(|[a, b]| consider(&mut best, nearest(at, a, b), snap::SnapKind::Path)); + } + best +} + +/// Whether more than 256 segments pass within `tolerance` of `at`, by a scan of the whole page. +fn limited_by_scan(g: &Geometry, at: [f64; 2], tolerance: f64) -> bool { + g.segments() + .filter(|[a, b]| { + let (dx, dy) = (b[0] - a[0], b[1] - a[1]); + let len2 = dx * dx + dy * dy; + let t = if len2 <= f64::EPSILON { 0.0 } else { (((at[0] - a[0]) * dx + (at[1] - a[1]) * dy) / len2).clamp(0.0, 1.0) }; + (at[0] - (a[0] + t * dx)).hypot(at[1] - (a[1] + t * dy)) <= tolerance + }) + .count() + > 256 +} + +#[test] +fn indexed_snapping_matches_a_scan_of_the_whole_page() { + let mut rng = Rng(SEED ^ 0x5eed); + let mut checked = 0usize; + for case in 0..200 { + let seed = SEEDS[rng.below(SEEDS.len())]; + // A drawing with many short lines, some long ones (kept outside the cells) and curves. + let mut content = mutate(&mut rng, seed); + for i in 0..200 + rng.below(800) { + let (x, y) = (rng.below(600) as f64, rng.below(800) as f64); + let line = match i % 7 { + 0 => format!("{x} {y} m {} {} l S ", x + rng.below(600) as f64, y), + 1 => format!("{x} {y} m {} {} {} {} {} {} c S ", x + 5.0, y + 9.0, x + 9.0, y + 5.0, x + 12.0, y), + _ => format!("{x} {y} m {} {} l S ", x + rng.below(20) as f64, y + rng.below(20) as f64), + }; + content.extend_from_slice(line.as_bytes()); + } + let g = geometry_of(&content); + for _ in 0..40 { + let at = [rng.below(700) as f64 - 50.0 + 0.37, rng.below(900) as f64 - 50.0 + 0.61]; + let tolerance = [0.5, 2.0, 7.5, 40.0, 400.0][rng.below(5)]; + let options = SnapOptions { + endpoints: rng.below(4) != 0, + midpoints: rng.below(4) != 0, + intersections: rng.below(3) != 0, + paths: rng.below(2) == 0, + }; + let indexed = g.snap(at, tolerance, options).unwrap(); + let scanned = snap_by_scan(&g, at, tolerance, options); + assert_eq!(indexed, scanned, "case {case} at {at:?} tolerance {tolerance} {options:?}"); + assert_eq!(g.intersection_limited(at, tolerance), limited_by_scan(&g, at, tolerance), "case {case} at {at:?} tolerance {tolerance}"); + checked += 1; + } + } + assert!(checked >= 8000); +} diff --git a/crates/ui-egui/src/tools/measure_ui.rs b/crates/ui-egui/src/tools/measure_ui.rs index fffbd9d6d8..7eb09c7b7c 100644 --- a/crates/ui-egui/src/tools/measure_ui.rs +++ b/crates/ui-egui/src/tools/measure_ui.rs @@ -41,6 +41,9 @@ impl Tool { } } } +/// Where a thread reading snapping geometry sends it. +type PathsReceiver = std::sync::mpsc::Receiver>; + pub struct MeasureView { pub points: Vec, pub page: Option, @@ -70,6 +73,9 @@ pub struct MeasureView { /// Snapping geometry (or why it failed) per (page, edit generation): extraction is not /// repeated on every hover frame. paths: Option<(usize, u64, Result)>, + /// The snapping geometry being read on a thread, for (page, edit generation): a heavy + /// drawing takes seconds, which would freeze the window. + pending_paths: Option<(usize, u64, PathsReceiver)>, /// Saved measurements per edit generation, for the panel. listing: Option<(u64, Result)>, } @@ -98,6 +104,7 @@ impl Default for MeasureView { label: String::new(), seeded_page: None, paths: None, + pending_paths: None, listing: None, } } @@ -171,6 +178,7 @@ impl MeasureView { /// Snaps `point` to the page's drawn geometry (endpoints, midpoints, intersections, paths), /// recording in `state` why it couldn't when the content is too complex or unreadable. fn snap_to_content( + ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize, @@ -178,8 +186,9 @@ fn snap_to_content( pixels_per_unit: f32, point: Point, ) -> Option<(Point, measure::snap::SnapKind)> { - if state.paths.as_ref().is_none_or(|(p, g, _)| *p != page || *g != doc.edit_generation()) { - state.paths = Some((page, doc.edit_generation(), doc.measurement_paths(page))); + if !load_paths(ctx, state, doc, page) { + state.error = Some(tl!("Reading the drawing for snapping…").into()); + return None; } if let Some((_, _, Err(e))) = &state.paths { state.error = Some(e.clone()); @@ -204,6 +213,52 @@ fn snap_to_content( None } +/// A file smaller than this has its snapping geometry read on the spot. +const SNAP_SMALL_FILE: usize = 8 * 1024 * 1024; + +/// Whether the snapping geometry of `page` as it is now is at hand, starting (on a thread) or +/// collecting the read when it isn't. +fn load_paths(ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize) -> bool { + let generation = doc.edit_generation(); + let current = |p: usize, g: u64| p == page && g == generation; + if state.paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { + return true; + } + // Too little to read to be worth a thread, and the answer is there this frame. + if doc.bytes.len() < SNAP_SMALL_FILE { + state.pending_paths = None; + state.paths = Some((page, generation, doc.measurement_paths(page))); + return true; + } + if !state.pending_paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { + let (sender, receiver) = std::sync::mpsc::channel(); + state.pending_paths = Some((page, generation, receiver)); + let job = doc.measurement_paths_job(page); + let repaint = ctx.clone(); + let work = move || { + let _ = sender.send(job()); + repaint.request_repaint(); + }; + #[cfg(not(target_arch = "wasm32"))] + if let Err(e) = std::thread::Builder::new().name("pdfcraft-snap".into()).spawn(work) { + state.pending_paths = None; + state.paths = Some((page, generation, Err(e.to_string()))); + return true; + } + #[cfg(target_arch = "wasm32")] + work(); + } + let arrived = state.pending_paths.as_ref().and_then(|(_, _, r)| r.try_recv().ok()); + match arrived { + Some(result) => { + state.pending_paths = None; + state.paths = Some((page, generation, result)); + true + } + None => false, + } +} + /// The Measure tools' own clicks this frame: those of a press nobody holds. #[derive(Clone, Copy)] pub(crate) struct Clicks { @@ -258,7 +313,7 @@ pub(crate) fn page_input( let mut point = [f64::from(x), f64::from(y)]; let mut snapped = None; if state.snap_enabled { - snapped = snap_to_content(state, doc, page, [vx, vy], xf.zoom().max(1e-6), point); + snapped = snap_to_content(ui.ctx(), state, doc, page, [vx, vy], xf.zoom().max(1e-6), point); } if let Some((snapped_point, kind)) = snapped { point = snapped_point; @@ -653,6 +708,59 @@ mod tests { measure::reading(Kind::Distance, &[from, to], &scale).expect("reading").value } + /// A one-page PDF of `lines` short lines, written out uncompressed. + fn lined_pdf(lines: usize) -> Vec { + let content = "10 10 m 200 200 l S\n".repeat(lines); + let objects = [ + "<< /Type /Catalog /Pages 2 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_string(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R >>".to_string(), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + ]; + let mut pdf = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, object) in objects.iter().enumerate() { + offsets.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n{object}\nendobj\n", i + 1).as_bytes()); + } + let xref = pdf.len(); + pdf.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes()); + for offset in offsets { + pdf.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objects.len() + 1).as_bytes()); + pdf + } + + /// A big file's geometry is read on a thread, which the window doesn't wait for; a small + /// file's is there at once. + #[test] + fn only_a_small_file_is_read_for_snapping_on_the_window_thread() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + + let small = session.open_new("small.pdf", std::sync::Arc::new(lined_pdf(10))).unwrap(); + let mut state = MeasureView::default(); + assert!(load_paths(&ctx, &mut state, session.get(small).unwrap(), 0), "there at once"); + assert!(state.pending_paths.is_none()); + + let big = lined_pdf(SNAP_SMALL_FILE / 20 + 1); + assert!(big.len() >= SNAP_SMALL_FILE); + let big = session.open_new("big.pdf", std::sync::Arc::new(big)).unwrap(); + let doc = session.get(big).unwrap(); + let mut state = MeasureView::default(); + assert!(!load_paths(&ctx, &mut state, doc, 0), "started, and not waited for"); + assert!(state.pending_paths.is_some()); + let start = std::time::Instant::now(); + while !load_paths(&ctx, &mut state, doc, 0) { + assert!(start.elapsed() < std::time::Duration::from_secs(60), "it arrives"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(state.pending_paths.is_none()); + let geometry = state.paths.as_ref().and_then(|(_, _, g)| g.as_ref().ok()).expect("read"); + assert!(geometry.segment_count() > 0); + } + #[test] fn typed_ratios_are_read_leniently() { assert_eq!(parse_ratio("1:200"), Some(200.0)); From 80cd15b474e1a35019a69cbddb695b090dba4f4b Mon Sep 17 00:00:00 2001 From: quincy-jc <184324679+quincy-jc@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:24:51 +1100 Subject: [PATCH 011/107] fix(measure): bound the snap index and read one page at a time Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/document.rs | 6 + crates/measure/src/snap.rs | 107 +++++++++++++- crates/ui-egui/src/tools/measure_ui.rs | 186 ++++++++++++++++++++----- 3 files changed, 259 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index 987a2b627e..7124a32f36 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -289,6 +289,12 @@ impl Document { measure::snap::geometry(&e.cos, page).map_err(|e| e.to_string()) } + /// [`Self::measurement_paths`] within `limits`. + pub fn measurement_paths_within(&self, page: usize, limits: measure::snap::Limits) -> Result { + let e = self.editor.as_ref().ok_or("the document can't be read")?; + measure::snap::geometry_with_limits(&e.cos, page, limits).map_err(|e| e.to_string()) + } + /// [`Self::measurement_paths`] as work to run away from the UI: a heavy drawing takes /// seconds to read (millions of segments). It works on a copy of the document as it is now. pub fn measurement_paths_job(&self, page: usize) -> impl FnOnce() -> Result + Send + 'static { diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index 661b083187..95c1f2b08d 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -19,8 +19,16 @@ pub struct Limits { } impl Limits { /// Enough for heavy CAD sheets (several million segments), and - /// the renderer's own content limit. At about 40 bytes a segment, the cap holds ~320 MB. + /// the renderer's own content limit. At about 40 bytes a segment, the cap holds ~320 MB, plus + /// up to [`MAX_INDEX_ENTRIES`] in the index. pub const DEFAULT: Self = Self { segments: 8_000_000, targets: 24_000_000, bytes: 512 << 20 }; + /// What a window can read without missing frames that matter. + pub const WITHIN_A_FRAME: Self = Self { segments: 20_000, targets: 40_000, bytes: 16 << 20 }; + /// Within what the index can number. + fn indexable(self) -> Self { + let most = u32::MAX as usize / 2; + Self { segments: self.segments.min(most), targets: self.targets.min(most), ..self } + } } impl Default for Limits { fn default() -> Self { @@ -139,6 +147,9 @@ impl Geometry { } /// Dense intersection searches are capped independently from path extraction. pub fn intersection_limited(&self, at: Point, tolerance: f64) -> bool { + if tolerance.is_nan() || tolerance < 0.0 { + return false; + } let (segs, _) = self.grid.near(self, at, tolerance); segs.iter() .filter_map(|&i| self.segs.get(i as usize)) @@ -214,7 +225,7 @@ impl Geometry { } /// Snap targets are recorded only for accepted geometry, and are capped. fn target(&mut self, ends: [Point; 2], mid: Point) { - if self.targets() >= self.limits.targets { + if self.targets().saturating_add(3) > self.limits.targets { self.truncated = true; return; } @@ -507,6 +518,7 @@ pub fn geometry(doc: &Document, page_index: usize) -> Result { /// The page's geometry for snapping, within `limits`. pub fn geometry_with_limits(doc: &Document, page_index: usize, limits: Limits) -> Result { + let limits = limits.indexable(); let p = crate::page(doc, page_index)?; let resources = p.dict.get(b"Resources").map(|o| doc.resolve(o)); let empty = Dict::new(); @@ -519,7 +531,7 @@ pub fn geometry_with_limits(doc: &Document, page_index: usize, limits: Limits) - let mut geometry = walker.geometry; geometry.segs.shrink_to_fit(); geometry.tgts.shrink_to_fit(); - geometry.grid = Grid::build(&geometry); + geometry.grid = Grid::build(&geometry, MAX_INDEX_ENTRIES); Ok(geometry) } @@ -530,6 +542,9 @@ const GRID_SIDE: usize = 1024; const WIDE_CELLS: usize = 64; /// The most cells one snap reads; a larger search area (a huge tolerance) scans everything. const MAX_QUERY_CELLS: usize = 4096; +/// The most entries the segment lists of the cells hold (4 bytes each). Past it, shorter segments +/// go in the list checked at every snap: snaps get slower, and find the same. +const MAX_INDEX_ENTRIES: u64 = 64_000_000; /// A uniform grid over the geometry's box: per cell, the segments and targets touching it. #[derive(Clone, Debug, Default)] @@ -549,7 +564,7 @@ struct Grid { } impl Grid { - fn build(g: &Geometry) -> Self { + fn build(g: &Geometry, max_entries: u64) -> Self { let mut bounds: Option<[f32; 4]> = None; let mut grow = |x: f32, y: f32| { let b = bounds.get_or_insert([x, y, x, y]); @@ -576,11 +591,20 @@ impl Grid { let cells = cols * rows; // Segments: count, then fill (compressed rows). let seg_cells = |s: &[f32; 4]| grid.range(s[0].min(s[2]), s[1].min(s[3]), s[0].max(s[2]), s[1].max(s[3])); + let covered = |s: &[f32; 4]| { + let (c0, r0, c1, r1) = seg_cells(s); + (c1 - c0 + 1) * (r1 - r0 + 1) + }; + // A segment of one cell adds one entry, so the last step fits whatever the sheet. + let most_cells = [WIDE_CELLS, 16, 4] + .into_iter() + .find(|&most| g.segs.iter().map(covered).filter(|&n| n <= most).map(|n| n as u64).sum::() <= max_entries) + .unwrap_or(1); let mut counts = vec![0u32; cells]; let mut wide = Vec::new(); for (i, s) in g.segs.iter().enumerate() { let (c0, r0, c1, r1) = seg_cells(s); - if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + if (c1 - c0 + 1) * (r1 - r0 + 1) > most_cells { wide.push(i as u32); continue; } @@ -594,7 +618,7 @@ impl Grid { let mut seg_items = vec![0u32; seg_start.last().copied().unwrap_or(0) as usize]; for (i, s) in g.segs.iter().enumerate() { let (c0, r0, c1, r1) = seg_cells(s); - if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + if (c1 - c0 + 1) * (r1 - r0 + 1) > most_cells { continue; } for r in r0..=r1 { @@ -673,7 +697,8 @@ impl Grid { if self.cols == 0 || self.rows == 0 { return if g.segs.is_empty() && g.tgts.is_empty() { (Vec::new(), Vec::new()) } else { all() }; } - let t = tolerance as f32; + // Rounding to `f32` can stop the box short of a candidate's cell: this much wider can't. + let t = (tolerance + (at[0].abs().max(at[1].abs()) + tolerance) * 2.0 * f64::from(f32::EPSILON)) as f32; let (x, y) = (at[0] as f32, at[1] as f32); if !(x.is_finite() && y.is_finite() && t.is_finite()) { return all(); @@ -704,3 +729,71 @@ impl Grid { (segs, tgts) } } + +#[cfg(test)] +mod grid_tests { + use super::*; + + fn geometry(segs: Vec<[f32; 4]>, tgts: Vec<[f32; 6]>, max_entries: u64) -> Geometry { + let mut g = Geometry { segs, tgts, ..Geometry::default() }; + g.grid = Grid::build(&g, max_entries); + g + } + + /// Past the allowance the index holds fewer entries and finds the same. + #[test] + fn a_bounded_index_finds_what_a_full_one_does() { + let mut segs = Vec::new(); + for i in 0..4000u16 { + let (x, y) = (f32::from(i % 64) * 15.0, f32::from(i / 64) * 15.0); + segs.push([x, y, x + 37.0, y + 41.0]); + } + let tgts = Vec::new(); + let full = geometry(segs.clone(), tgts.clone(), u64::MAX); + let budget = segs.len() as u64; + let bounded = geometry(segs, tgts, budget); + assert!(full.grid.seg_items.len() as u64 > budget, "the sheet needs more than the allowance"); + assert!(bounded.grid.seg_items.len() as u64 <= budget, "{} entries", bounded.grid.seg_items.len()); + let options = SnapOptions { endpoints: false, midpoints: false, intersections: true, paths: true }; + for i in 0..400u16 { + let at = [f64::from(i) * 2.37, f64::from(i) * 1.13]; + assert_eq!(bounded.snap(at, 6.0, options).unwrap(), full.snap(at, 6.0, options).unwrap(), "at {at:?}"); + } + } + + #[test] + fn limits_stay_within_what_the_index_can_number() { + let limits = Limits { segments: usize::MAX, targets: usize::MAX, bytes: 1 }.indexable(); + assert!(limits.segments <= u32::MAX as usize && limits.targets <= u32::MAX as usize); + assert_eq!(Limits::DEFAULT.indexable(), Limits::DEFAULT); + } + + /// A tolerance below zero finds nothing; it must not reverse the cells searched. + #[test] + fn a_negative_tolerance_finds_nothing() { + let segs = (0..4000u16) + .map(|i| [f32::from(i % 64) * 15.0, f32::from(i / 64) * 15.0, f32::from(i % 64) * 15.0 + 5.0, f32::from(i / 64) * 15.0]) + .collect(); + let g = geometry(segs, Vec::new(), u64::MAX); + assert!(g.grid.cols > 1 && g.grid.rows > 1); + assert!(!g.intersection_limited([400.0, 400.0], -200.0)); + } + + /// Near 1e8 one `f32` step is 8 units, which must not hide an end just within tolerance. + #[test] + fn rounding_doesnt_hide_a_target_at_the_edge_of_the_grid() { + let g = geometry(Vec::new(), vec![[100_000_008.0, 0.0, 100_000_016.0, 0.0, 100_000_012.0, 0.0]], u64::MAX); + let options = SnapOptions { endpoints: true, midpoints: false, intersections: false, paths: false }; + let hit = g.snap([100_000_003.999_999_9, 0.0], 4.000_000_11, options).unwrap(); + assert_eq!(hit.map(|h| h.point), Some([100_000_008.0, 0.0])); + } + + /// A target is three points, and all three must fit. + #[test] + fn a_target_is_kept_only_if_all_its_points_fit() { + let mut g = Geometry::with_limits(Limits { segments: 10, targets: 2, bytes: 1 }); + g.target([[0.0, 0.0], [1.0, 0.0]], [0.5, 0.0]); + assert_eq!(g.targets(), 0); + assert!(g.truncated); + } +} diff --git a/crates/ui-egui/src/tools/measure_ui.rs b/crates/ui-egui/src/tools/measure_ui.rs index 7eb09c7b7c..37249d0ded 100644 --- a/crates/ui-egui/src/tools/measure_ui.rs +++ b/crates/ui-egui/src/tools/measure_ui.rs @@ -186,10 +186,14 @@ fn snap_to_content( pixels_per_unit: f32, point: Point, ) -> Option<(Point, measure::snap::SnapKind)> { + let reading = tl!("Reading the drawing for snapping…"); if !load_paths(ctx, state, doc, page) { - state.error = Some(tl!("Reading the drawing for snapping…").into()); + state.error = Some(reading.into()); return None; } + if state.error.as_deref() == Some(reading) { + state.error = None; + } if let Some((_, _, Err(e))) = &state.paths { state.error = Some(e.clone()); } @@ -213,50 +217,81 @@ fn snap_to_content( None } -/// A file smaller than this has its snapping geometry read on the spot. +/// A file smaller than this may have its snapping geometry read on the spot. +#[cfg(not(target_arch = "wasm32"))] const SNAP_SMALL_FILE: usize = 8 * 1024 * 1024; /// Whether the snapping geometry of `page` as it is now is at hand, starting (on a thread) or /// collecting the read when it isn't. fn load_paths(ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize) -> bool { + use std::sync::mpsc::TryRecvError; let generation = doc.edit_generation(); let current = |p: usize, g: u64| p == page && g == generation; if state.paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { return true; } - // Too little to read to be worth a thread, and the answer is there this frame. - if doc.bytes.len() < SNAP_SMALL_FILE { - state.pending_paths = None; - state.paths = Some((page, generation, doc.measurement_paths(page))); - return true; - } - if !state.pending_paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { - let (sender, receiver) = std::sync::mpsc::channel(); - state.pending_paths = Some((page, generation, receiver)); - let job = doc.measurement_paths_job(page); - let repaint = ctx.clone(); - let work = move || { - let _ = sender.send(job()); - repaint.request_repaint(); + // One read at a time, each being a whole page: one of another page or edit is let finish, and + // its answer dropped. + if let Some((p, g, receiver)) = &state.pending_paths { + let answer = match receiver.try_recv() { + Ok(result) => result, + Err(TryRecvError::Empty) => return false, + Err(TryRecvError::Disconnected) => Err("the snapping read stopped unexpectedly".into()), }; - #[cfg(not(target_arch = "wasm32"))] - if let Err(e) = std::thread::Builder::new().name("pdfcraft-snap".into()).spawn(work) { - state.pending_paths = None; - state.paths = Some((page, generation, Err(e.to_string()))); + let wanted = current(*p, *g); + state.pending_paths = None; + if wanted { + state.paths = Some((page, generation, answer)); return true; } - #[cfg(target_arch = "wasm32")] - work(); } - let arrived = state.pending_paths.as_ref().and_then(|(_, _, r)| r.try_recv().ok()); - match arrived { - Some(result) => { - state.pending_paths = None; - state.paths = Some((page, generation, result)); - true - } - None => false, + start_read(ctx, state, doc, page, generation) +} + +/// What the window itself reads of `page`: a small file's page, if it is within what a frame can +/// afford. A small file can still hold millions of segments. +#[cfg(not(target_arch = "wasm32"))] +fn read_on_the_spot(doc: &Document, page: usize) -> Option> { + if doc.bytes.len() >= SNAP_SMALL_FILE { + return None; } + let read = doc.measurement_paths_within(page, measure::snap::Limits::WITHIN_A_FRAME); + (!read.as_ref().is_ok_and(|g| g.truncated)).then_some(read) +} + +/// Read `page`'s geometry, on a thread unless it is little. Whether it is at hand already. +#[cfg(not(target_arch = "wasm32"))] +fn start_read(ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize, generation: u64) -> bool { + if let Some(read) = read_on_the_spot(doc, page) { + state.paths = Some((page, generation, read)); + return true; + } + let (sender, receiver) = std::sync::mpsc::channel(); + let job = doc.measurement_paths_job(page); + let repaint = ctx.clone(); + let work = move || { + let _ = sender.send(job()); + repaint.request_repaint(); + }; + if let Err(e) = std::thread::Builder::new().name("pdfcraft-snap".into()).spawn(work) { + state.paths = Some((page, generation, Err(e.to_string()))); + return true; + } + state.pending_paths = Some((page, generation, receiver)); + false +} + +/// With no threads here, what a frame can afford; past it the drawing says it exceeds the limit. +#[cfg(target_arch = "wasm32")] +fn start_read(_ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize, generation: u64) -> bool { + state.paths = Some((page, generation, doc.measurement_paths_within(page, measure::snap::Limits::WITHIN_A_FRAME))); + true +} + +/// Whether a click may place a point: with snapping on, not while the drawing is being read, as +/// the point would land unsnapped. +fn takes_clicks(state: &MeasureView) -> bool { + !state.snap_enabled || state.pending_paths.is_none() } /// The Measure tools' own clicks this frame: those of a press nobody holds. @@ -325,7 +360,7 @@ pub(crate) fn page_input( point = measure::snap::constrain(*last, point); } hover = Some(point); - if clicks.single { + if clicks.single && takes_clicks(state) { if state.page != Some(page) { state.cancel(); state.page = Some(page); @@ -355,7 +390,8 @@ pub(crate) fn page_input( } } if state.page == Some(page) && !state.points.is_empty() { - if (clicks.double || !typing && ui.input(|i| i.key_pressed(egui::Key::Enter))) && matches!(tool, Tool::Perimeter | Tool::Area) { + let double = clicks.double && takes_clicks(state); + if (double || !typing && ui.input(|i| i.key_pressed(egui::Key::Enter))) && matches!(tool, Tool::Perimeter | Tool::Area) { finish(state, &mut view.pending_edit, doc, page, tool, author, units); } if !typing && ui.input(|i| i.key_pressed(egui::Key::Backspace)) { @@ -710,6 +746,11 @@ mod tests { /// A one-page PDF of `lines` short lines, written out uncompressed. fn lined_pdf(lines: usize) -> Vec { + padded_lined_pdf(lines, 0) + } + + /// [`lined_pdf`], with `padding` more bytes in the file that aren't on the page. + fn padded_lined_pdf(lines: usize, padding: usize) -> Vec { let content = "10 10 m 200 200 l S\n".repeat(lines); let objects = [ "<< /Type /Catalog /Pages 2 0 R >>".to_string(), @@ -718,6 +759,7 @@ mod tests { format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), ]; let mut pdf = b"%PDF-1.7\n".to_vec(); + pdf.extend_from_slice(format!("%{}\n", "x".repeat(padding)).as_bytes()); let mut offsets = Vec::new(); for (i, object) in objects.iter().enumerate() { offsets.push(pdf.len()); @@ -744,7 +786,8 @@ mod tests { assert!(load_paths(&ctx, &mut state, session.get(small).unwrap(), 0), "there at once"); assert!(state.pending_paths.is_none()); - let big = lined_pdf(SNAP_SMALL_FILE / 20 + 1); + // However little is on its page. + let big = padded_lined_pdf(10, SNAP_SMALL_FILE); assert!(big.len() >= SNAP_SMALL_FILE); let big = session.open_new("big.pdf", std::sync::Arc::new(big)).unwrap(); let doc = session.get(big).unwrap(); @@ -761,6 +804,83 @@ mod tests { assert!(geometry.segment_count() > 0); } + /// A small file can hold more than a frame can read: it goes to the thread, and comes whole. + #[test] + fn a_small_file_past_a_frames_limits_is_read_on_a_thread() { + let lines = measure::snap::Limits::WITHIN_A_FRAME.segments + 1; + let pdf = lined_pdf(lines); + assert!(pdf.len() < SNAP_SMALL_FILE); + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(pdf)).unwrap(); + let doc = session.get(id).unwrap(); + let mut state = MeasureView::default(); + assert!(!load_paths(&ctx, &mut state, doc, 0), "not read on the window's thread"); + let start = std::time::Instant::now(); + while !load_paths(&ctx, &mut state, doc, 0) { + assert!(start.elapsed() < std::time::Duration::from_secs(60), "it arrives"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let geometry = state.paths.as_ref().and_then(|(_, _, g)| g.as_ref().ok()).expect("read"); + assert_eq!(geometry.segment_count(), lines); + assert!(!geometry.truncated); + } + + /// A read of another page is let finish before the next starts, and only then is it dropped. + #[test] + fn only_one_snapping_read_runs_at_a_time() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("big.pdf", std::sync::Arc::new(lined_pdf(SNAP_SMALL_FILE / 20 + 1))).unwrap(); + let doc = session.get(id).unwrap(); + let (sender, receiver) = std::sync::mpsc::channel(); + let mut state = MeasureView { pending_paths: Some((9, 9, receiver)), ..MeasureView::default() }; + assert!(!load_paths(&ctx, &mut state, doc, 0)); + assert_eq!(state.pending_paths.as_ref().map(|(p, _, _)| *p), Some(9), "waits for it"); + sender.send(Ok(measure::snap::Geometry::default())).unwrap(); + assert!(!load_paths(&ctx, &mut state, doc, 0)); + assert_eq!(state.pending_paths.as_ref().map(|(p, _, _)| *p), Some(0), "then reads this page"); + assert!(state.paths.is_none(), "the other page's answer is dropped"); + } + + /// A read that dies says so, instead of reading on for ever. + #[test] + fn a_snapping_read_that_stops_is_an_error() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(lined_pdf(10))).unwrap(); + let doc = session.get(id).unwrap(); + let (sender, receiver) = std::sync::mpsc::channel(); + drop(sender); + let mut state = MeasureView { pending_paths: Some((0, doc.edit_generation(), receiver)), ..MeasureView::default() }; + assert!(load_paths(&ctx, &mut state, doc, 0)); + assert!(matches!(&state.paths, Some((_, _, Err(_))))); + assert!(state.pending_paths.is_none()); + } + + #[test] + fn the_reading_message_goes_when_the_drawing_is_read() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(lined_pdf(10))).unwrap(); + let doc = session.get(id).unwrap(); + let mut state = MeasureView { error: Some(tl!("Reading the drawing for snapping…").into()), ..MeasureView::default() }; + let _ = snap_to_content(&ctx, &mut state, doc, 0, [5.0, 5.0], 1.0, [5.0, 5.0]); + assert_eq!(state.error, None); + } + + #[test] + fn clicks_wait_for_the_drawing_only_with_snapping_on() { + let (_sender, receiver) = std::sync::mpsc::channel(); + let mut state = MeasureView { pending_paths: Some((0, 0, receiver)), ..MeasureView::default() }; + assert!(!takes_clicks(&state)); + state.snap_enabled = false; + assert!(takes_clicks(&state)); + state.snap_enabled = true; + state.pending_paths = None; + assert!(takes_clicks(&state)); + } + #[test] fn typed_ratios_are_read_leniently() { assert_eq!(parse_ratio("1:200"), Some(200.0)); From 7c9e006c8273a5305f50e020f3cd14c984cd7468 Mon Sep 17 00:00:00 2001 From: quincy-jc <184324679+quincy-jc@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:31:51 +1100 Subject: [PATCH 012/107] perf(measure): read a page's operators one at a time for snapping Co-Authored-By: Claude Opus 5.5 --- crates/measure/src/snap.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index 95c1f2b08d..0f02c76250 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -1,6 +1,6 @@ //! Bounded path extraction and snapping in PDF user space, including nested Form XObjects. use crate::{Point, Result, check_points, distance, invalid}; -use pdfcraft_content::{Matrix, parse}; +use pdfcraft_content::Matrix; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; use serde::Serialize; use std::collections::HashSet; @@ -336,7 +336,8 @@ impl Walker<'_> { let (x, y) = m.apply(x, y); [x, y] }; - for op in parse(data).ops { + // One at a time: a heavy page holds gigabytes as a list of operators. + for op in pdfcraft_content::ops(data) { if self.geometry.segs.len().saturating_add(path.segs.len()) >= self.limits.segments || self.geometry.targets().saturating_add(path.targets()) >= self.limits.targets { From 280584cc72c3d9c7ce56d4d3824cc62211cc2303 Mon Sep 17 00:00:00 2001 From: Hasan Date: Mon, 2 Nov 2026 18:27:08 +0500 Subject: [PATCH 013/107] M3: a snapshot keeps its resolution when zoomed out (#5716) A snapshot was drawn at the zoom on screen, so one taken with the page zoomed out came out too coarse to read. It is now drawn at 300 ppi, or at the zoom if that is finer, or as fine as the renderer draws in one tile: it stops at 4096 pixels a side and cuts a larger area short, which 300 ppi would reach on any area over about 983 pt a side. --- ROADMAP.md | 1 + crates/ui-egui/src/canvas/zoom_snap.rs | 51 ++++++++++++++++++++-- crates/ui-egui/tests/it/viewer.rs | 59 ++++++++++++++++++++++++++ docs/roadmap.md | 1 + parity/acrobat-features.toml | 4 +- 5 files changed, 111 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index cf8cacb73e..7596bd5d3b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -243,6 +243,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-11-02 (M10, #5743):** macOS print jobs name their title and owner (`com.apple.print.JobInfo.PMJobName`, `PMJobOwner`), as the system print dialog does, because a Kyocera filter crashes on a job without them. Checked by a test and through `doc_print` against a stand-in `lp`; not yet on a physical printer. +- **2026-11-02 (M3, #5716):** A snapshot is drawn at 300 ppi, or at the zoom if that is finer, so one taken zoomed out stays readable. A choice of resolution and a selection that stays active remain. - **2026-11-02 (M14, #5737, #5460):** Preferences can turn off the update check at start. It stays on by default, and Help ▸ Check for updates still asks. - **2026-11-01 (M7, #5657):** Header/footer, watermark and background additions and replacements stage the complete call before committing. Synthetic later-page failures preserve exact incremental bytes, object count, dirty state and allocation cursor; success controls retain other marks and shared/inherited resources. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas/zoom_snap.rs b/crates/ui-egui/src/canvas/zoom_snap.rs index a422907b89..a23f85a6a8 100644 --- a/crates/ui-egui/src/canvas/zoom_snap.rs +++ b/crates/ui-egui/src/canvas/zoom_snap.rs @@ -120,13 +120,13 @@ impl PdfCraftApp { Ok(()) } - /// Render `view_rect` (page view points) of `page` at the current zoom and copy it to the - /// clipboard (and keep it in `last_snapshot`). + /// Render `view_rect` (page view points) of `page` and copy it to the clipboard (and keep it + /// in `last_snapshot`), at the current zoom or [`SNAPSHOT_MIN_SCALE`] if that is finer. pub fn snapshot(&mut self, index: usize, page: usize, view_rect: [f32; 4]) -> Result<(u32, u32), String> { let view = &self.views[index]; let doc = self.session.get(view.id).ok_or("no document")?; let ppp = self.ctx.as_ref().map_or(2.0, |c| c.pixels_per_point()); - let scale = (view.zoom * ppp).clamp(0.5, 8.0); + let scale = snapshot_scale((view.zoom * ppp).clamp(0.5, 8.0), view_rect[2] - view_rect[0], view_rect[3] - view_rect[1]); let tile = Tile { x: (view_rect[0] * scale).floor().max(0.0) as u32, y: (view_rect[1] * scale).floor().max(0.0) as u32, @@ -153,6 +153,20 @@ impl PdfCraftApp { } } +/// The fewest pixels per point a snapshot is drawn at: 300 ppi, so what it copies stays readable +/// however far the page is zoomed out (#5716). +const SNAPSHOT_MIN_SCALE: f32 = 300.0 / 72.0; + +/// The longest side a snapshot has, in pixels: the renderer draws at most 4096 a side in one tile +/// and cuts a larger area short, and this leaves room for rounding up. +const SNAPSHOT_MAX_SIDE: f32 = 4095.0; + +/// The pixels per point to draw a `width` × `height` point area at, given the `on_screen` scale: +/// the finer of that and the minimum, or as fine as the renderer can draw the whole area. +fn snapshot_scale(on_screen: f32, width: f32, height: f32) -> f32 { + on_screen.max(SNAPSHOT_MIN_SCALE).min(SNAPSHOT_MAX_SIDE / width.max(height).max(1.0)).max(0.01) +} + /// The normalised bounds [u0, v0, u1, v1] of the pixels that aren't (near) white, if any. pub(crate) fn ink_bounds(w: usize, h: usize, rgba: &[u8]) -> Option<[f32; 4]> { if w == 0 || h == 0 || rgba.len() < w * h * 4 { @@ -175,6 +189,37 @@ pub(crate) fn ink_bounds(w: usize, h: usize, rgba: &[u8]) -> Option<[f32; 4]> { #[cfg(test)] mod tests { + use super::{SNAPSHOT_MAX_SIDE, SNAPSHOT_MIN_SCALE, snapshot_scale}; + + #[test] + fn a_snapshot_is_drawn_at_the_finer_of_the_zoom_and_the_minimum() { + assert_eq!(snapshot_scale(0.5, 100.0, 100.0), SNAPSHOT_MIN_SCALE, "zoomed out"); + assert_eq!(snapshot_scale(SNAPSHOT_MIN_SCALE, 100.0, 100.0), SNAPSHOT_MIN_SCALE); + assert_eq!(snapshot_scale(8.0, 100.0, 100.0), 8.0, "zoomed in further, it keeps the detail on screen"); + } + + #[test] + fn a_snapshot_stays_within_what_the_renderer_draws_at_once() { + // Past 983 pt a side, 300 ppi is more than one tile holds. + for (on_screen, w, h) in [(1.0, 2384.0, 3370.0), (1.0, 3370.0, 100.0), (8.0, 1000.0, 1000.0), (8.0, 14_400.0, 14_400.0)] { + let scale = snapshot_scale(on_screen, w, h); + let side = (w * scale).ceil().max((h * scale).ceil()); + assert!(side <= SNAPSHOT_MAX_SIDE + 1.0, "{on_screen} on {w} × {h}: {side} pixels"); + } + } + + #[test] + fn a_snapshot_of_no_area_is_drawn_at_the_minimum() { + assert_eq!(snapshot_scale(1.0, 0.0, 0.0), SNAPSHOT_MIN_SCALE); + } + + #[test] + fn a_snapshot_scale_survives_numbers_that_are_not() { + for scale in [snapshot_scale(f32::NAN, 5000.0, 5000.0), snapshot_scale(1.0, f32::NAN, 100.0), snapshot_scale(1.0, f32::INFINITY, 100.0)] { + assert!(scale.is_finite() && scale > 0.0, "{scale}"); + } + } + #[test] fn ink_bounds_finds_the_marked_area() { let (w, h) = (10, 20); diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index d383582915..7ab88d5888 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -968,6 +968,65 @@ fn marquee_zoom_and_snapshot() { assert!(after > before * 2.0, "{before} → {after}"); } +/// #5716: what a snapshot holds doesn't depend on how far the page is zoomed out. +#[test] +fn a_snapshot_keeps_its_resolution_when_zoomed_out() { + let mut h = harness(); + let i = h.state().active.unwrap(); + h.state_mut().system_clipboard = false; + let mut sizes = Vec::new(); + for zoom in ["25", "100"] { + h.state_mut().set_option("zoom", zoom).unwrap(); + h.run_steps(2); + // The text line of the page, as a marquee would give it. + sizes.push(h.state_mut().snapshot(i, 0, [15.0, 125.0, 180.0, 160.0]).expect("a snapshot")); + } + assert_eq!(sizes[0], sizes[1], "the same pixels at either zoom"); + assert_eq!(sizes[0], (688, 146), "165 × 35 pt at 300 ppi"); + let (_, _, px) = h.state().last_snapshot.clone().expect("a snapshot"); + assert!(px.as_chunks::<4>().0.iter().any(|p| p[0] < 100), "the text is in it"); + + // Zoomed in past the minimum, a snapshot still has what the screen shows. + h.state_mut().set_option("zoom", "800").unwrap(); + h.run_steps(2); + let (w, _) = h.state_mut().snapshot(i, 0, [15.0, 125.0, 180.0, 160.0]).expect("a snapshot"); + assert!(w > sizes[0].0, "{w}"); +} + +/// #5716: at 300 ppi this page is more than the renderer draws in one tile, which would cut off +/// the side the snapshot was taken for. A 2000 pt square with a black mark in its far corner. +#[test] +fn a_snapshot_of_a_large_page_reaches_its_far_corner() { + let stream = "0 0 0 rg 1900 0 100 100 re f"; + let pdf = format!( + "%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 2000 2000] /Contents 4 0 R >> endobj +4 0 obj << /Length {} >> stream +{stream} +endstream endobj +trailer << /Root 1 0 R >> +%%EOF", + stream.len() + ); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("large.pdf", None, pdf.into_bytes()).unwrap(); + app.system_clipboard = false; + app + }); + h.run_steps(4); + let i = h.state().active.unwrap(); + let (w, hgt) = h.state_mut().snapshot(i, 0, [0.0, 0.0, 2000.0, 2000.0]).expect("a snapshot"); + assert!(w <= 4096 && hgt <= 4096 && w == hgt, "{w} × {hgt}"); + let (_, _, px) = h.state().last_snapshot.clone().expect("a snapshot"); + let at = |x: u32, y: u32| px[((y * w + x) * 4) as usize]; + assert!(at(w * 98 / 100, hgt * 98 / 100) < 100, "the mark in the far corner is in it"); + assert!(at(w / 2, hgt / 2) > 200, "and the page around it is white"); +} + #[test] fn fit_visible_zooms_to_the_content_width() { let mut h = harness(); diff --git a/docs/roadmap.md b/docs/roadmap.md index a7f4a86b7e..92573aa0bb 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -314,6 +314,7 @@ Estimates unchanged. | 2026-11-03 | patch | M5 #5850 review: indirect typewriter intent and composer edits preserve outer whitespace; movement regression commits by outside click; estimates unchanged. | | 2026-11-03 | patch | M5 #5850: Enter adds Fill & Sign line breaks; outside clicks commit, Escape cancels, and saved multiline appearances retain their text and height; estimates unchanged. | | 2026-11-02 | patch | M10 #5743: macOS print jobs carry the job name and owner some vendors' filters need; not yet checked on a physical printer | +| 2026-11-02 | patch | M3 #5716: a snapshot keeps its resolution when zoomed out; a choice of resolution remains | | 2026-11-02 | patch | M14 #5737, #5460: Preferences turns off the update check at start, on by default | | 2026-11-01 | patch | M7 #5657: header/footer, watermark and background operations commit all selected pages together; later-page errors preserve marks, content, shared/inherited resources and pending edits; estimates unchanged. | | 2026-10-31 | patch | #1577: reconcile optional page shadows with current viewer/organizer and settings modules; CI validation pending. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 5cb6e1c56a..34d65d88e0 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1586,8 +1586,8 @@ milestone = "M3" status = "shipped" commands = ["edit.snapshot"] tools = ["page_render"] -evidence = ["crates/ui-egui/tests/it/viewer.rs::marquee_zoom_and_snapshot"] -notes = "copies the area at the current zoom to the clipboard as an image" +evidence = ["crates/ui-egui/tests/it/viewer.rs::marquee_zoom_and_snapshot", "crates/ui-egui/tests/it/viewer.rs::a_snapshot_keeps_its_resolution_when_zoomed_out", "crates/ui-egui/src/canvas/zoom_snap.rs::a_snapshot_is_drawn_at_the_finer_of_the_zoom_and_the_minimum", "crates/ui-egui/src/canvas/zoom_snap.rs::a_snapshot_stays_within_what_the_renderer_draws_at_once", "crates/ui-egui/tests/it/viewer.rs::a_snapshot_of_a_large_page_reaches_its_far_corner"] +notes = "copies the area to the clipboard as an image, at 300 ppi or the current zoom if that is finer (#5716), or as fine as the renderer draws in one tile (4096 pixels a side), drawn while the app waits. Missing: a choice of resolution, and a selection that stays active to copy later" [[feature]] id = "view.copy-image" From 7380241724f49d41c17b9871bacf8cee175af43d Mon Sep 17 00:00:00 2001 From: Hasan Date: Mon, 2 Nov 2026 21:30:30 +0500 Subject: [PATCH 014/107] M3: a snapshot uses the scale the page is drawn at (#5716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #5761: the scale came from zoom × device pixels, without the 96/72 the viewer draws at, and was capped at 8, so a snapshot zoomed in held fewer pixels than the screen showed: 417 against 534 for 100 pt at 400%. It now takes the viewer's own render scale, and the tile limit is what stops it. The roadmap entry says when 300 ppi can't be reached. --- ROADMAP.md | 2 +- crates/ui-egui/src/canvas/zoom_snap.rs | 2 +- crates/ui-egui/tests/it/viewer.rs | 14 +++++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 7596bd5d3b..84dfb3f489 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -243,7 +243,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-11-02 (M10, #5743):** macOS print jobs name their title and owner (`com.apple.print.JobInfo.PMJobName`, `PMJobOwner`), as the system print dialog does, because a Kyocera filter crashes on a job without them. Checked by a test and through `doc_print` against a stand-in `lp`; not yet on a physical printer. -- **2026-11-02 (M3, #5716):** A snapshot is drawn at 300 ppi, or at the zoom if that is finer, so one taken zoomed out stays readable. A choice of resolution and a selection that stays active remain. +- **2026-11-02 (M3, #5716):** A snapshot is drawn at 300 ppi, or at the zoom if that is finer, so one taken zoomed out stays readable; an area too large for one 4096-pixel tile comes out as fine as a tile allows (about 147 ppi for 2000 pt). A choice of resolution and a selection that stays active remain. - **2026-11-02 (M14, #5737, #5460):** Preferences can turn off the update check at start. It stays on by default, and Help ▸ Check for updates still asks. - **2026-11-01 (M7, #5657):** Header/footer, watermark and background additions and replacements stage the complete call before committing. Synthetic later-page failures preserve exact incremental bytes, object count, dirty state and allocation cursor; success controls retain other marks and shared/inherited resources. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas/zoom_snap.rs b/crates/ui-egui/src/canvas/zoom_snap.rs index a23f85a6a8..93e0b13210 100644 --- a/crates/ui-egui/src/canvas/zoom_snap.rs +++ b/crates/ui-egui/src/canvas/zoom_snap.rs @@ -126,7 +126,7 @@ impl PdfCraftApp { let view = &self.views[index]; let doc = self.session.get(view.id).ok_or("no document")?; let ppp = self.ctx.as_ref().map_or(2.0, |c| c.pixels_per_point()); - let scale = snapshot_scale((view.zoom * ppp).clamp(0.5, 8.0), view_rect[2] - view_rect[0], view_rect[3] - view_rect[1]); + let scale = snapshot_scale(view.render_scale(ppp), view_rect[2] - view_rect[0], view_rect[3] - view_rect[1]); let tile = Tile { x: (view_rect[0] * scale).floor().max(0.0) as u32, y: (view_rect[1] * scale).floor().max(0.0) as u32, diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index 7ab88d5888..f2c812ce78 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -986,11 +986,15 @@ fn a_snapshot_keeps_its_resolution_when_zoomed_out() { let (_, _, px) = h.state().last_snapshot.clone().expect("a snapshot"); assert!(px.as_chunks::<4>().0.iter().any(|p| p[0] < 100), "the text is in it"); - // Zoomed in past the minimum, a snapshot still has what the screen shows. - h.state_mut().set_option("zoom", "800").unwrap(); - h.run_steps(2); - let (w, _) = h.state_mut().snapshot(i, 0, [15.0, 125.0, 180.0, 160.0]).expect("a snapshot"); - assert!(w > sizes[0].0, "{w}"); + // Zoomed in past the minimum, a snapshot has every pixel the screen shows, however far. + let ppp = h.ctx.pixels_per_point(); + for zoom in [400.0_f32, 1600.0] { + h.state_mut().set_option("zoom", &zoom.to_string()).unwrap(); + h.run_steps(2); + let (w, _) = h.state_mut().snapshot(i, 0, [15.0, 125.0, 180.0, 160.0]).expect("a snapshot"); + let on_screen = (165.0 * zoom / 100.0 * pdfcraft_ui_egui::canvas::PT * ppp).ceil() as u32; + assert!(w.abs_diff(on_screen) <= 1, "{w} pixels at {zoom}%, {on_screen} on screen"); + } } /// #5716: at 300 ppi this page is more than the renderer draws in one tile, which would cut off From bf5bfbf7073078edc190a28d62b8853623c841b9 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Fri, 6 Nov 2026 08:53:15 +0800 Subject: [PATCH 015/107] M5: a text markup whose QuadPoints reach past its Rect is drawn over them (view.render-annotation-appearances) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/annot/src/appearance.rs | 20 ++++++++++++++++++ crates/annot/src/draw.rs | 4 +++- crates/annot/src/tests.rs | 37 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/crates/annot/src/appearance.rs b/crates/annot/src/appearance.rs index 7148e9fc2e..bcacc27a70 100644 --- a/crates/annot/src/appearance.rs +++ b/crates/annot/src/appearance.rs @@ -508,6 +508,7 @@ fn appearance_context(d: &Dict) -> Option> { let subtype = d.name(b"Subtype")?; let rect = nums(d, b"Rect").filter(|r| r.len() == 4)?; let rect = [rect[0].min(rect[2]), rect[1].min(rect[3]), rect[0].max(rect[2]), rect[1].max(rect[3])]; + let rect = markup_reach(d).unwrap_or(rect); // Cloudy borders (§12.5.4) are drawn for polygons only. let cloudy = d.get(b"BE").and_then(|b| b.as_dict()).is_some_and(|b| b.name(b"S") == Some(b"C")); if cloudy && subtype != b"Polygon" { @@ -525,6 +526,25 @@ fn appearance_context(d: &Dict) -> Option> { }) } +/// A text markup annotation's `/Rect` grown to take in its `/QuadPoints`, where they reach past +/// it; `None` when they don't (or aren't usable), and for every other subtype. The quadrilaterals +/// are what the markup marks (ISO 32000-2 Table 182), and the appearance is mapped onto `/Rect` +/// (§12.5.5), so a `/Rect` that leaves them out would cut off what is drawn: a highlight whose +/// `/Rect` lies beside its text drew nothing. +pub(crate) fn markup_reach(d: &Dict) -> Option<[f64; 4]> { + if !matches!(d.name(b"Subtype"), Some(b"Highlight" | b"Underline" | b"StrikeOut" | b"Squiggly")) { + return None; + } + let rect = nums(d, b"Rect").filter(|r| r.len() == 4 && r.iter().all(|v| v.is_finite()))?; + let rect = [rect[0].min(rect[2]), rect[1].min(rect[3]), rect[0].max(rect[2]), rect[1].max(rect[3])]; + let q = nums(d, b"QuadPoints").filter(|q| !q.is_empty() && q.len() % 8 == 0 && q.iter().all(|v| v.is_finite()))?; + let mut out = rect; + for p in q.as_chunks::<2>().0 { + out = [out[0].min(p[0]), out[1].min(p[1]), out[2].max(p[0]), out[3].max(p[1])]; + } + (out != rect).then_some(out) +} + fn draw_body(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) -> Option<()> { match context.subtype { b"Highlight" | b"Underline" | b"StrikeOut" | b"Squiggly" => draw_markup(context, drawing), diff --git a/crates/annot/src/draw.rs b/crates/annot/src/draw.rs index 5e698332da..2b5cedaef0 100644 --- a/crates/annot/src/draw.rs +++ b/crates/annot/src/draw.rs @@ -78,9 +78,11 @@ fn apply_appearance(doc: &mut Document, r: ObjRef, d: &Dict, stream: Stream) -> let (x, top) = (q[0].min(q[2]), q[1].max(q[3])); [x, top - NOTE_SIZE, x + NOTE_SIZE, top] }); + // Text markup whose /QuadPoints reach past /Rect: its appearance covers them, so /Rect does too. + let markup_rect = appearance::markup_reach(d); doc.update_dict(r, |d| { d.set(b"AP".to_vec(), Object::Dict(apd)); - if let Some(rect) = icon_rect { + if let Some(rect) = icon_rect.or(markup_rect) { d.set(b"Rect".to_vec(), num_array(&rect)); } d.remove(b"AS"); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index ff7f8925f0..4afd042481 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2779,3 +2779,40 @@ fn indirect_typewriter_names_keep_outer_whitespace_in_summaries() { check(&doc); check(&reopen(&doc)); } + +/// A text markup annotation is drawn over its `/QuadPoints` (ISO 32000-2 Table 182), and an +/// appearance is mapped onto `/Rect` (§12.5.5). A highlight whose `/Rect` lies beside its text +/// (here written with its corners swapped, as a real file has it) got an appearance cut to that +/// `/Rect`, so nothing showed. The display copy's appearance and `/Rect` now take in the +/// quadrilaterals; a markup whose `/Rect` already holds them keeps it. +#[test] +fn text_markup_whose_quads_reach_past_its_rect_is_drawn_over_them() { + let doc = document_of(&[ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", + b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>", + b"<< /Type /Annot /Subtype /Highlight /Rect [335.1 370.83 324.6 471.78] /C [1 1 0] /QuadPoints [370 287 471 287 370 277 471 277] >>", + b"<< /Type /Annot /Subtype /Underline /Rect [10 10 20 20] /C [0 0 1] /QuadPoints [100 120 200 120 100 100 200 100] >>", + b"<< /Type /Annot /Subtype /Highlight /Rect [50 50 150 70] /C [1 1 0] /QuadPoints [60 68 140 68 60 52 140 52] >>", + ]); + let display = with_missing_appearances(&doc).expect("three markups need an appearance"); + let shown = list(&display, 0); + let rect = |d: &Dict| nums_of(d, b"Rect"); + let bbox = |d: &Dict| { + let n = d.get(b"AP").map(|a| display.resolve(a)).and_then(|a| a.as_dict().and_then(|a| a.reference(b"N"))).unwrap(); + let obj = display.get(n); + let Object::Stream(s) = &*obj else { panic!("AP is not a stream") }; + nums_of(&s.dict, b"BBox") + }; + assert_eq!(rect(&shown[0]), [324.6, 277.0, 471.0, 471.78], "the highlight's /Rect takes in its text"); + assert_eq!(bbox(&shown[0]), rect(&shown[0])); + assert!(ap_content(&display, &shown[0]).contains("370 287 m 471 287 l 471 277 l 370 277 l h f"), "the quad is filled"); + assert_eq!(rect(&shown[1]), [10.0, 10.0, 200.0, 120.0], "an underline's too"); + assert_eq!(bbox(&shown[1]), rect(&shown[1])); + assert_eq!(rect(&shown[2]), [50.0, 50.0, 150.0, 70.0], "a /Rect that holds the quads stays as it is"); + assert_eq!(rect(&list(&doc, 0)[0]), [335.1, 370.83, 324.6, 471.78], "the source document is unchanged"); +} + +fn nums_of(d: &Dict, key: &[u8]) -> Vec { + d.get(key).and_then(|a| a.as_array()).map(|a| a.iter().filter_map(|o| o.as_f64()).collect()).unwrap_or_default() +} From 96b96c53da58f22e810400ad8a691c16bf59d7ce Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Fri, 6 Nov 2026 08:53:35 +0800 Subject: [PATCH 016/107] parity: text markup drawn over its QuadPoints past /Rect Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- parity/acrobat-features.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index ec144d3a07..1a176b5453 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -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/annot/src/tests.rs::text_markup_whose_quads_reach_past_its_rect_is_drawn_over_them", "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; text markup over its /QuadPoints where they reach past /Rect) 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)" [[feature]] id = "view.render-error-isolation" From 572cec5dca60b9db90f4eaa4aa7c6cc1719eac79 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Fri, 6 Nov 2026 09:11:02 +0800 Subject: [PATCH 017/107] A markup's quads too far apart for a width keep its /Rect; StrikeOut and Squiggly in the test Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/annot/src/appearance.rs | 4 +++- crates/annot/src/tests.rs | 12 ++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/annot/src/appearance.rs b/crates/annot/src/appearance.rs index bcacc27a70..7d5fde14d0 100644 --- a/crates/annot/src/appearance.rs +++ b/crates/annot/src/appearance.rs @@ -542,7 +542,9 @@ pub(crate) fn markup_reach(d: &Dict) -> Option<[f64; 4]> { for p in q.as_chunks::<2>().0 { out = [out[0].min(p[0]), out[1].min(p[1]), out[2].max(p[0]), out[3].max(p[1])]; } - (out != rect).then_some(out) + // Finite corners can still be too far apart for a width or height (`-1e308` to `1e308`). + let usable = (out[2] - out[0]).is_finite() && (out[3] - out[1]).is_finite(); + (usable && out != rect).then_some(out) } fn draw_body(context: &AppearanceContext<'_>, drawing: &mut AppearanceDrawing) -> Option<()> { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 4afd042481..8a6db33f94 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2790,12 +2790,15 @@ fn text_markup_whose_quads_reach_past_its_rect_is_drawn_over_them() { let doc = document_of(&[ b"<< /Type /Catalog /Pages 2 0 R >>", b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", - b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>", + b"<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R 7 0 R 8 0 R 9 0 R] >>", b"<< /Type /Annot /Subtype /Highlight /Rect [335.1 370.83 324.6 471.78] /C [1 1 0] /QuadPoints [370 287 471 287 370 277 471 277] >>", b"<< /Type /Annot /Subtype /Underline /Rect [10 10 20 20] /C [0 0 1] /QuadPoints [100 120 200 120 100 100 200 100] >>", b"<< /Type /Annot /Subtype /Highlight /Rect [50 50 150 70] /C [1 1 0] /QuadPoints [60 68 140 68 60 52 140 52] >>", + b"<< /Type /Annot /Subtype /StrikeOut /Rect [10 10 20 20] /C [1 0 0] /QuadPoints [100 220 200 220 100 200 200 200] >>", + b"<< /Type /Annot /Subtype /Squiggly /Rect [10 10 20 20] /C [0 1 0] /QuadPoints [100 320 200 320 100 300 200 300] >>", + b"<< /Type /Annot /Subtype /Highlight /Rect [10 10 20 20] /C [1 1 0] /QuadPoints [-1e308 20 1e308 20 -1e308 10 1e308 10] >>", ]); - let display = with_missing_appearances(&doc).expect("three markups need an appearance"); + let display = with_missing_appearances(&doc).expect("the markups need an appearance"); let shown = list(&display, 0); let rect = |d: &Dict| nums_of(d, b"Rect"); let bbox = |d: &Dict| { @@ -2810,6 +2813,11 @@ fn text_markup_whose_quads_reach_past_its_rect_is_drawn_over_them() { assert_eq!(rect(&shown[1]), [10.0, 10.0, 200.0, 120.0], "an underline's too"); assert_eq!(bbox(&shown[1]), rect(&shown[1])); assert_eq!(rect(&shown[2]), [50.0, 50.0, 150.0, 70.0], "a /Rect that holds the quads stays as it is"); + assert_eq!(rect(&shown[3]), [10.0, 10.0, 200.0, 220.0], "a strikeout's too"); + assert_eq!(bbox(&shown[3]), rect(&shown[3])); + assert_eq!(rect(&shown[4]), [10.0, 10.0, 200.0, 320.0], "a squiggly's too"); + assert_eq!(bbox(&shown[4]), rect(&shown[4])); + assert_eq!(rect(&shown[5]), [10.0, 10.0, 20.0, 20.0], "quads too far apart for a width keep the /Rect"); assert_eq!(rect(&list(&doc, 0)[0]), [335.1, 370.83, 324.6, 471.78], "the source document is unchanged"); } From 89e6a86029c344248ba0f340d4c5c4001c65fbe9 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:03:13 +0200 Subject: [PATCH 018/107] Hidden layers includes layers a View usage rule turns off Hidden layers read only the default configuration's ON, OFF and BaseState. A layer listed on there but turned off on screen by a usage rule (/AS, ISO 32000-2, 8.11.4.4: the View event in the View category, with the layer's /Usage /View /ViewState OFF) is hidden in viewers that apply the rules, yet its content stayed. Such a layer now counts as hidden. Rules that turn a layer on, or that apply only when printing or exporting, leave it as the lists say, so the removal never keeps less than it did. --- crates/redact/src/sanitize.rs | 34 ++++++++++++++++++++++++++-------- crates/redact/src/tests.rs | 21 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index f8088e0bba..9b431736d7 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -324,22 +324,40 @@ fn is_comment(subtype: &[u8]) -> bool { !matches!(subtype, b"FileAttachment" | b"Redact") && pdfcraft_annot::is_comment_subtype(&String::from_utf8_lossy(subtype)) } -/// The layers that are off in the default configuration. +/// The layers that are off in the default configuration: listed off (or not listed on, under +/// `/BaseState /OFF`), or turned off on screen by one of its usage rules (`/AS`, ISO 32000-2, +/// 8.11.4.4): a rule for the View event in the View category hides a layer whose +/// `/Usage /View /ViewState` is OFF. Rules that turn a layer on, or apply only when printing or +/// exporting, leave it as the lists say. fn hidden_layers(doc: &Document) -> Vec { let Some((_, cat)) = catalog(doc) else { return Vec::new() }; let Some(oc) = sub(doc, &cat, b"OCProperties") else { return Vec::new() }; let Some(d) = sub(doc, &oc, b"D") else { return Vec::new() }; - let all: Vec = - oc.get(b"OCGs").and_then(|g| doc.resolve(g).as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default(); - let list = |k: &[u8]| -> Vec { - d.get(k).and_then(|g| doc.resolve(g).as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default() + let refs = |o: Option<&Object>| -> Vec { + o.and_then(|g| doc.resolve(g).as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default() }; - let (on, off) = (list(b"ON"), list(b"OFF")); - if d.name(b"BaseState") == Some(b"OFF") { - all.into_iter().filter(|g| !on.contains(g)).collect() + let all = refs(oc.get(b"OCGs")); + let (on, off) = (refs(d.get(b"ON")), refs(d.get(b"OFF"))); + let mut hidden: Vec = if d.name(b"BaseState") == Some(b"OFF") { + all.iter().copied().filter(|g| !on.contains(g)).collect() } else { off.into_iter().filter(|g| all.contains(g)).collect() + }; + let rules = d.get(b"AS").and_then(|a| doc.resolve(a).as_array().cloned()).unwrap_or_default(); + for rule in rules.iter().filter_map(|r| doc.resolve(r).as_dict().cloned()) { + let categories = rule.get(b"Category").and_then(|c| doc.resolve(c).as_array().cloned()).unwrap_or_default(); + if rule.name(b"Event") != Some(b"View") || !categories.iter().any(|c| c.as_name() == Some(b"View")) { + continue; + } + for g in refs(rule.get(b"OCGs")) { + let usage = doc.get(g).as_dict().and_then(|layer| sub(doc, layer, b"Usage")); + let view_off = usage.and_then(|u| sub(doc, &u, b"View")).is_some_and(|v| v.name(b"ViewState") == Some(b"OFF")); + if view_off && all.contains(&g) && !hidden.contains(&g) { + hidden.push(g); + } + } } + hidden } /// Glyphs of hidden text, or content of hidden layers, on every page (`write` = remove them). diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 8978e0e7d6..e552abe5e9 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -4207,3 +4207,24 @@ fn a_redacted_form_xobject_drops_the_source_text_of_added_content() { let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap(); assert!(!bytes.windows(4).any(|w| w == b"1234"), "redacted text left in the saved file"); } + +/// A layer the default configuration lists on but a View usage rule turns off is hidden on screen +/// (ISO 32000-2, 8.11.4.4): Hidden layers removes it. A Print rule leaves its layer as it is. +#[test] +fn a_layer_a_view_rule_turns_off_is_hidden() { + use crate::sanitize::{Hidden, remove_hidden, scan}; + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /OCProperties << /OCGs [6 0 R 7 0 R] /D << /AS [<< /Event /View /OCGs [6 0 R] /Category [/View] >> << /Event /Print /OCGs [7 0 R] /Category [/Print] >>] >> >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /Properties << /L1 6 0 R /L2 7 0 R >> >> >>".to_vec(), + stream("", b"/OC /L1 BDC BT /F1 10 Tf 10 50 Td (ScreenOff) Tj ET EMC /OC /L2 BDC BT /F1 10 Tf 10 100 Td (PrintOff) Tj ET EMC"), + FONT.replace("95 0 R", "8 0 R").into_bytes(), + b"<< /Type /OCG /Name (Screen) /Usage << /View << /ViewState /OFF >> >> >>".to_vec(), + b"<< /Type /OCG /Name (Print) /Usage << /Print << /PrintState /OFF >> >> >>".to_vec(), + widths(), + ]); + assert!(scan(&doc).into_iter().any(|(h, n)| h == Hidden::HiddenLayers && n > 0)); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + let c = content(&doc, 0); + assert!(!c.contains("ScreenOff") && c.contains("(PrintOff)"), "{c}"); +} From ac8eb5146430fa8d94d418addbbb0d5c4a1b44af Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:03:14 +0200 Subject: [PATCH 019/107] Removed layers leave every optional-content configuration Hidden layers took a removed layer out of the default configuration's top-level ON, OFF, Order and Locked lists only. A sub-list of /Order, a radio-button group (/RBGroups), a usage rule (/AS) and every alternate configuration in /Configs still named it, so its dictionary, name included, stayed in the full-rewrite save. The removal now cleans all of these lists, through nested arrays, in the default configuration and in every alternate one. A usage rule left with no layer goes. --- crates/redact/src/sanitize.rs | 60 ++++++++++++++++++++++++++++++++--- crates/redact/src/tests.rs | 24 ++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index f8088e0bba..3777ac54d4 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -342,6 +342,52 @@ fn hidden_layers(doc: &Document) -> Vec { } } +/// `o` without the entries `keep` refuses, through nested arrays (an `/Order` holds sub-lists, and +/// `/RBGroups` holds groups); arrays written by reference are copied in place. +fn filtered(doc: &Document, o: &Object, keep: &impl Fn(&Object) -> bool, depth: usize) -> Result { + if depth > MAX_DIRECT_DEPTH { + return Err(RedactError::TooDeep); + } + match &*doc.resolve(o) { + Object::Array(a) => { + let mut out = Vec::with_capacity(a.len()); + for x in a.iter().filter(|x| keep(x)) { + out.push(filtered(doc, x, keep, depth + 1)?); + } + Ok(Object::Array(out)) + } + _ => Ok(o.clone()), + } +} + +/// An optional-content configuration without the layers `keep` refuses, in every list that names +/// them: on, off, order, locked, radio-button groups and usage rules (a rule left with no layer +/// goes). A removed layer still named in any of them would stay in the file, its name included. +fn config_without(doc: &Document, mut d: Dict, keep: &impl Fn(&Object) -> bool) -> Result { + for k in [&b"ON"[..], b"OFF", b"Order", b"Locked", b"RBGroups"] { + if let Some(v) = d.get(k).cloned() { + let v = filtered(doc, &v, keep, 0)?; + d.set(k.to_vec(), v); + } + } + if let Some(Object::Array(rules)) = d.get(b"AS").map(|a| (*doc.resolve(a)).clone()) { + let mut kept = Vec::with_capacity(rules.len()); + for rule in rules { + let Some(mut rule) = doc.resolve(&rule).as_dict().cloned() else { continue }; + if let Some(layers) = rule.get(b"OCGs").cloned() { + let layers = filtered(doc, &layers, keep, 0)?; + if layers.as_array().is_some_and(Vec::is_empty) { + continue; + } + rule.set(b"OCGs".to_vec(), layers); + } + kept.push(Object::Dict(rule)); + } + d.set(b"AS".to_vec(), Object::Array(kept)); + } + Ok(d) +} + /// Glyphs of hidden text, or content of hidden layers, on every page (`write` = remove them). fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Result<(usize, std::collections::HashSet), RedactError> { let off = if layers { hidden_layers(doc) } else { Vec::new() }; @@ -636,13 +682,17 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result>] >> /Configs [8 0 R] >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /Properties << /L1 6 0 R >> >> >>".to_vec(), + stream("", b"/OC /L1 BDC BT /F1 10 Tf 10 50 Td (Gone) Tj ET EMC BT /F1 10 Tf 10 100 Td (Kept) Tj ET"), + FONT.replace("95 0 R", "9 0 R").into_bytes(), + b"<< /Type /OCG /Name (SECRETLAYERNAME) >>".to_vec(), + b"<< /Type /OCG /Name (Shown layer) >>".to_vec(), + b"<< /Name (Other view) /OFF [7 0 R] /ON [6 0 R] /Order [6 0 R 7 0 R] >>".to_vec(), + widths(), + ]); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap(); + let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle); + assert!(has(b"(Shown layer)") && has(b"(Other view)"), "the layer that stays and the configuration stay"); + assert!(!has(b"SECRETLAYERNAME"), "the removed layer survived the save"); +} From 18a4cd117ef2bf664000799e0ab6f948739af4dd Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:11:40 +0200 Subject: [PATCH 020/107] Combine and Insert bring only the bookmarks and attachments of the pages chosen With a selection of a file's pages, Combine and Insert Pages from File brought every bookmark title along, the ones whose page was left out without a destination, and Combine brought every attachment. Only the bookmarks that lead to a copied page, and those above them, come along now, as Extract keeps them since #4041. Attachments belong to the whole file: they come along with all of its pages only. --- crates/engine/src/session/files.rs | 3 +- crates/organize/src/import.rs | 4 +-- crates/organize/src/import/carry.rs | 6 +++- crates/organize/src/import/combine.rs | 22 +++++++++----- crates/organize/src/tests.rs | 44 +++++++++++++++++++++++++-- 5 files changed, 65 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/session/files.rs b/crates/engine/src/session/files.rs index 44bed85cde..85ed558f94 100644 --- a/crates/engine/src/session/files.rs +++ b/crates/engine/src/session/files.rs @@ -289,7 +289,8 @@ impl Session { /// [`Self::combine_unlocked`] where sources with the same `groups` key (by position) are one /// file split into several runs placed apart: each file is opened and copied once, so it keeps - /// one bookmark, its links between its own pages, its fields and its attachments whole (see + /// one bookmark, and its links between its own pages and its fields whole across its runs; its + /// attachments come along when its runs use all of its pages (see /// `pdfcraft_organize::combine_grouped`). A group's later sources use its first source's bytes /// and password; a source without a key is a file of its own. pub fn combine_grouped(&self, sources: &[CombineSource], groups: &[u64], passwords: &[Option<&str>]) -> Result>, EditError> { diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 9f86d6455a..e967e08533 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -100,8 +100,8 @@ fn added_objects(dst: &Document, before: &HashSet) -> Vec { /// Insert Pages from File: [`import_pages_with`], and the inserted file's bookmarks come along /// under one new bookmark titled `title` (the file's name) at its first inserted page, as Combine /// Files does for each file. That bookmark goes among the top-level bookmarks where the pages went: -/// before the first one that points after them. Bookmarks to pages left out lose their destination; -/// a file without bookmarks adds none. +/// before the first one that points after them. Bookmarks to pages left out stay behind, unless a +/// bookmark below them leads to an inserted page; a file without bookmarks adds none. pub fn insert_file_pages( dst: &mut Document, src: &Document, diff --git a/crates/organize/src/import/carry.rs b/crates/organize/src/import/carry.rs index 15a412e231..00fb9bd0b8 100644 --- a/crates/organize/src/import/carry.rs +++ b/crates/organize/src/import/carry.rs @@ -98,9 +98,13 @@ pub(super) fn source_outline(src: &Document, pages: HashMap) -> .and_then(|r| src.get(r).as_dict().cloned()) .and_then(|c| c.get(b"Outlines").map(|o| src.resolve(o))) .and_then(|o| o.as_dict().and_then(|d| d.reference(b"First"))); + let whole = crate::page_count(src).is_ok_and(|n| pages.len() >= n); let mapper = Copier::new(src, HashMap::new(), pages); let mut budget = 10_000usize; - outline_level(&mapper, first, 0, &mut budget) + let outline = outline_level(&mapper, first, 0, &mut budget); + // Of some of the pages, only the bookmarks that lead to a copied page and those above them: + // the others would carry their titles along into a file without their pages. + if whole { outline } else { kept_bookmarks(outline) } } /// `nodes` without the bookmarks whose page was not copied (no destination left), unless a bookmark diff --git a/crates/organize/src/import/combine.rs b/crates/organize/src/import/combine.rs index 06972ae101..7782e5de18 100644 --- a/crates/organize/src/import/combine.rs +++ b/crates/organize/src/import/combine.rs @@ -1,5 +1,5 @@ use super::carry::{Mark, add_outline, collect_attachments, kept_bookmarks, set_attachments, source_outline, write_outline_level}; -use super::{Dict, Document, FieldConflicts, HashMap, ObjRef, Object, OrganizeError, import_pages_mapped, rebuild}; +use super::{Dict, Document, FieldConflicts, HashMap, HashSet, ObjRef, Object, OrganizeError, import_pages_mapped, rebuild}; /// A new document containing copies of `pages` from `src` (Extract Pages / Split). /// Document information (title, author…) is carried over, and so is the print standard the @@ -95,7 +95,8 @@ pub fn combine_with(sources: &[(&str, &Document)], conflicts: FieldConflicts) -> } /// Combine Files with chosen pages: each source contributes `pages` (0-based, in that order; -/// `None` for all of them). Bookmarks that point at pages left out lose their destination. +/// `None` for all of them). A source whose pages are not all used brings only the bookmarks that +/// lead to its copied pages (and those above them), and none of its attachments. pub fn combine_selected(sources: &[(&str, &Document, Option<&[usize]>)]) -> Result { combine_selected_with(sources, FieldConflicts::Rename).map(|o| o.doc) } @@ -187,7 +188,11 @@ impl Combiner { let outline = source_outline(src, imported.page_map); self.marks.push(Mark { title: title.to_string(), page: *first, outline }); } - collect_attachments(&mut self.out, src, &mut self.attachments)?; + // Attachments belong to the whole file: they come along with all of its pages, not with a + // selection of them. + if pages.iter().collect::>().len() == n { + collect_attachments(&mut self.out, src, &mut self.attachments)?; + } let declared = crate::pdfx::PrintStandard::of(src); match self.standard.as_ref().map(|first| first.declared.same_as(&declared)) { Some(same) => self.agreed &= same, @@ -227,9 +232,9 @@ pub type Run<'a> = (usize, &'a str, &'a Document, Option<&'a [usize]>); /// Combine Files where a file's pages may be split into several runs with other files' pages /// between them. Each file (group) is copied once, all its pages together, then the pages are put -/// in the runs' order: its links between its own pages, its form fields and its attachments stay -/// whole, and it gets one top-level bookmark (the title of its first run) at its first page, its -/// own bookmarks nested. Otherwise as [`combine_selected`] (print standard, shared resources). +/// in the runs' order: its links between its own pages and its form fields stay whole across its +/// runs, its attachments come along when its runs use all of its pages, and it gets one top-level +/// bookmark (the title of its first run) at its first page, its own bookmarks nested. Otherwise as [`combine_selected`] (print standard, shared resources). pub fn combine_grouped(runs: &[Run<'_>]) -> Result { combine_grouped_with(runs, FieldConflicts::Rename).map(|o| o.doc) } @@ -281,7 +286,10 @@ pub fn combine_grouped_with(runs: &[Run<'_>], conflicts: FieldConflicts) -> Resu if let Some(first) = imported.pages.first() { marks.push(Mark { title: file.title.to_string(), page: *first, outline: source_outline(file.src, imported.page_map) }); } - collect_attachments(&mut out, file.src, &mut attachments)?; + // Attachments come along with all of a file's pages, not with a selection of them. + if file.pages.iter().collect::>().len() == crate::page_count(file.src)? { + collect_attachments(&mut out, file.src, &mut attachments)?; + } let declared = crate::pdfx::PrintStandard::of(file.src); match &standard { None => standard = Some((declared, file.src, imported.objects)), diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index d5ff2116ff..768b22e21d 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -1591,13 +1591,12 @@ fn inserting_a_file_brings_its_bookmarks_under_one_named_after_it() { let page_of = |title: &str| marks.iter().find(|b| b.title == title).and_then(|b| b.page); assert_eq!((page_of("annex"), page_of("Chapter one"), page_of("Section"), page_of("End")), (Some(1), Some(1), Some(2), Some(4))); assert!(!marks.iter().find(|b| b.title == "annex").unwrap().open, "the file's bookmarks start collapsed"); - // Inserted last, after every bookmark; only the pages chosen keep a destination. + // Inserted last, after every bookmark; a bookmark to a page left out stays behind with it. let mut host = open(fixture()); add_bookmark(&mut host, &[], 0, "Start", 0).unwrap(); insert_file_pages(&mut host, &doc_c(), "annex", &[0], 9, FieldConflicts::Rename).unwrap(); let marks = bookmarks(&host); - assert_eq!(titles(&marks), ["Start", "annex[Chapter one[Section]]"]); - assert_eq!(marks.iter().find(|b| b.title == "Section").unwrap().page, None, "its page was left out"); + assert_eq!(titles(&marks), ["Start", "annex[Chapter one]"], "Section's page was left out"); // A file without bookmarks adds none, and a host without an outline gets one only when needed. let mut host = open(fixture()); insert_file_pages(&mut host, &doc_b(), "plain", &[0], 0, FieldConflicts::Rename).unwrap(); @@ -3985,3 +3984,42 @@ fn an_outline_deeper_than_the_listing_is_exported_as_truncated() { crate::import_outline(&mut target, &[], 0, &tree.nodes, false).unwrap(); assert_eq!(crate::bookmarks(&target).len(), 33); } + +/// The titles of a bookmark's children, each with its own children's titles. +fn bookmark_titles(doc: &Document, first: Option) -> Vec<(String, Vec)> { + let mut out = Vec::new(); + let mut at = first; + while let Some(r) = at { + let item = doc.get(r).as_dict().cloned().unwrap(); + let title = item.get(b"Title").and_then(|t| t.as_string()).map(|t| t.to_text()).unwrap_or_default(); + let children = bookmark_titles(doc, item.reference(b"First")).into_iter().map(|(t, _)| t).collect(); + out.push((title, children)); + at = item.reference(b"Next"); + } + out +} + +/// Combining some of a file's pages brings only the bookmarks that lead to them (and those above +/// them), and none of the file's attachments. +#[test] +fn combining_some_pages_brings_only_their_bookmarks_and_no_attachments() { + let source_bookmarks = |out: &Document| { + let outlines = out.get(catalog(out).reference(b"Outlines").unwrap()).as_dict().cloned().unwrap(); + let file = out.get(outlines.reference(b"First").unwrap()).as_dict().cloned().unwrap(); + bookmark_titles(out, file.reference(b"First")) + }; + let attachments = |out: &Document| { + catalog(out).get(b"Names").map(|n| out.resolve(n)).is_some_and(|n| n.as_dict().is_some_and(|d| d.contains(b"EmbeddedFiles"))) + }; + // Page 1 only: "Section" leads to page 2. + let first = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[0]))]).unwrap()); + assert_eq!(source_bookmarks(&first), [("Chapter one".to_string(), vec![])]); + assert!(!attachments(&first), "an attachment came along with a selection of pages"); + // Page 2 only: "Chapter one" stays, as the way to "Section". + let second = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[1]))]).unwrap()); + assert_eq!(source_bookmarks(&second), [("Chapter one".to_string(), vec!["Section".to_string()])]); + // Every page: all of it, attachments too. + let all = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[0, 1]))]).unwrap()); + assert_eq!(source_bookmarks(&all), [("Chapter one".to_string(), vec!["Section".to_string()])]); + assert!(attachments(&all)); +} From 6a56476a075a5d8d8e87bdff6c03cb4f0f17b775 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:05:12 +0200 Subject: [PATCH 021/107] Remove Hidden Information unlinks images nothing draws Delete Image takes only the Do out of the page, and Replace Image adds a new name and leaves the old image listed. Remove Hidden Information only pruned the names its own content pass replaced, so a deleted image stayed in the page's resources and in the full-rewrite save. Every run now unlinks the image and form XObjects that nothing draws from the page-tree resource dictionaries that name them. A name stays when anything drawing through the dictionary uses it with Do: the pages, their annotations' appearances, and every form, tiling pattern, Type 3 glyph and soft mask they reach, with /Resources of their own or not, since a name those don't define is looked up in the page's. Going by name keeps more than it must, never less. Each stream is read once against one decode budget; a page or stream that won't decode, or a spent budget, leaves the dictionary as it is. The page-tree walk is shared with unlink_doomed. --- crates/redact/src/interp.rs | 2 +- crates/redact/src/interp/prune.rs | 177 ++++++++++++++++++++++++++---- crates/redact/src/sanitize.rs | 3 + crates/redact/src/tests.rs | 100 +++++++++++++++++ 4 files changed, 257 insertions(+), 25 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index b0a7257cb3..71ac31b59c 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -31,7 +31,7 @@ mod prune; use layers::strip_hidden_layers; use prune::{clip_out, extgstate_font, prune_form_resources}; -pub(crate) use prune::{prune, prune_patterns, prune_properties, resource_ops, unlink_doomed, unlink_pruned_patterns}; +pub(crate) use prune::{prune, prune_patterns, prune_properties, resource_ops, unlink_doomed, unlink_pruned_patterns, unlink_undrawn}; /// How deep this pass follows form XObjects. A form past it is removed rather than walked, which /// is the fail-safe choice for a redaction -- marked content inside a form we cannot read would diff --git a/crates/redact/src/interp/prune.rs b/crates/redact/src/interp/prune.rs index 188af03abe..eff76b50a9 100644 --- a/crates/redact/src/interp/prune.rs +++ b/crates/redact/src/interp/prune.rs @@ -1,6 +1,6 @@ use super::{ - CosError, Dict, Document, HashMap, HashSet, MAX_DEPTH, Matrix, Metrics, ObjRef, Object, Op, Pieces, Rc, RedactError, Scope, is_vertical, num, - overlaps, page_streams, res_dict, + CosError, DecodeBudget, Dict, Document, HashMap, HashSet, MAX_DEPTH, Matrix, Metrics, ObjRef, Object, Op, Pieces, Rc, RedactError, Scope, + is_vertical, num, overlaps, page_streams, res_dict, }; /// Drop every `replaced` resource name whose `Do` no longer appears in the rewritten `content`, @@ -49,29 +49,12 @@ fn content_draws(doc: &Document, content: &[Vec], xo: &Dict, name: &[u8], de Some(false) } -/// Unlink the XObjects a rewrite dropped or replaced (`doomed`) from every page-tree resource -/// dictionary that still names them. The rewritten page has its own pruned /Resources, but the -/// originals stay reachable, and so stay in a full-rewrite save, when: -/// - the page inherited its /Resources from a /Pages node, which still lists them; -/// - several pages share one resource dictionary, and another page still refers to it; -/// - a form without /Resources drew them through the page's dictionary. -/// -/// An entry goes unless a page that draws through that dictionary still draws the name (the same -/// image shown on another page, or again outside a mark). A dictionary that loses an entry is -/// replaced by a private copy on its node, so a dictionary object shared with other nodes is -/// left as it is for them. Retired properties are likewise unlinked only when none of the -/// holder's pages or inheriting forms names them. Fails closed when use cannot be determined. -pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_properties: &[Object]) -> Result<(), RedactError> { - crate::type3::unlink_type3(doc)?; - if doomed.is_empty() && retired_properties.is_empty() { - return Ok(()); - } - let doomed: HashSet = doomed.iter().copied().collect(); - let Some(top) = doc.root().and_then(|root| doc.get(root).as_dict().and_then(|d| d.reference(b"Pages"))) else { return Ok(()) }; - // Every page-tree node with a /Resources of its own, and the pages that draw through it - // (the nearest such node above a page, or the page itself). Cycles are cut as in - // `pdfcraft_model::pages`. +/// Every page-tree node with a /Resources of its own, and the pages that draw through it (the +/// nearest such node above a page, or the page itself). Cycles are cut as in +/// `pdfcraft_model::pages`. +fn resource_holders(doc: &Document) -> Vec<(ObjRef, Vec)> { let mut holders: Vec<(ObjRef, Vec)> = Vec::new(); + let Some(top) = doc.root().and_then(|root| doc.get(root).as_dict().and_then(|d| d.reference(b"Pages"))) else { return holders }; let mut seen = HashSet::new(); let mut stack: Vec<(ObjRef, Option)> = vec![(top, None)]; while let Some((node, mut holder)) = stack.pop() { @@ -92,6 +75,152 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope h.1.push(node); } } + holders +} + +/// Unlink the image and form XObjects nothing draws from the page-tree resource dictionaries +/// that still name them: Delete Image and Replace Image take an image off the page but leave it +/// listed there, and so in a full-rewrite save. A name stays when anything that draws through the +/// dictionary uses it ([`names_drawn`]), and a dictionary whose use can't be told is left as it +/// is. A dictionary that loses an entry is replaced by a private copy on its node, as in +/// [`unlink_doomed`]. Returns how many names went. +pub(crate) fn unlink_undrawn(doc: &mut Document) -> Result { + let index: HashMap = pdfcraft_model::pages(doc).iter().enumerate().map(|(i, p)| (p.obj, i)).collect(); + let mut n = 0; + for (node, users) in resource_holders(doc) { + let Some(mut res) = doc.get(node).as_dict().and_then(|d| d.get(b"Resources")).and_then(|r| doc.resolve(r).as_dict().cloned()) else { + continue; + }; + let mut xo = res_dict(doc, &res, b"XObject"); + let listed: Vec> = xo + .iter() + .filter(|(_, v)| { + v.as_ref().is_some_and(|r| matches!(&*doc.get(r), Object::Stream(s) if matches!(s.dict.name(b"Subtype"), Some(b"Image" | b"Form")))) + }) + .map(|(name, _)| name.to_vec()) + .collect(); + if listed.is_empty() { + continue; + } + let Some(drawn) = names_drawn(doc, &res, &users, &index) else { continue }; + let mut pruned = false; + for name in listed.iter().filter(|name| !drawn.contains(*name)) { + xo.remove(name); + pruned = true; + n += 1; + } + if pruned { + res.set(b"XObject".to_vec(), Object::Dict(xo)); + doc.update_dict(node, |d| d.set(b"Resources".to_vec(), Object::Dict(res)))?; + } + } + Ok(n) +} + +/// Every name used with `Do` by what draws through `res`, the resource dictionary of `pages`: +/// their content, their annotations' appearances, and every form, tiling pattern, Type 3 glyph +/// and soft mask those can reach, with /Resources of their own or not. A name such a stream +/// doesn't define is looked up in the resources of whatever draws it, up to the page's, so a `Do` +/// anywhere in there may draw from `res`. Going by name keeps more than it must, never less. +/// Each stream is read once, against one budget. `None` when that can't be told: a page or +/// stream that won't decode, or the budget spent. +fn names_drawn(doc: &Document, res: &Dict, pages: &[ObjRef], index: &HashMap) -> Option>> { + let mut drawn = HashSet::new(); + let mut note = |data: &[u8]| { + for op in pdfcraft_content::parse(data).ops.iter().filter(|op| op.is("Do")) { + if let Some(name) = op.last_name() { + drawn.insert(name.to_vec()); + } + } + }; + // Streams still to read, and resource dictionaries still to look through. + let mut streams: Vec = Vec::new(); + let mut dicts: Vec = vec![res.clone()]; + for page in pages { + let dict = doc.get(*page).as_dict().cloned().unwrap_or_default(); + let (_, content) = page_streams(doc, &dict, index.get(page).copied().unwrap_or(0)).ok()?; + content.iter().for_each(|c| note(c)); + let annots = dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default(); + for annot in annots { + let ap = doc.resolve(&annot).as_dict().and_then(|a| a.get(b"AP")).map(|ap| doc.resolve(ap)).and_then(|ap| ap.as_dict().cloned()); + // Each of /N, /R and /D is an appearance, or a dictionary of them by state. + for (_, look) in ap.unwrap_or_default().iter() { + match look.as_ref() { + Some(r) if matches!(&*doc.get(r), Object::Stream(_)) => streams.push(r), + _ => streams.extend(doc.resolve(look).as_dict().into_iter().flat_map(|states| states.iter().filter_map(|(_, v)| v.as_ref()))), + } + } + } + } + let mut budget = DecodeBudget::page(); + let mut read = HashSet::new(); + let mut looked = 0; + loop { + while let Some(dict) = dicts.get(looked) { + streams.extend(drawn_through(doc, dict)); + looked += 1; + } + let Some(r) = streams.pop() else { break }; + if !read.insert(r) { + continue; + } + let obj = doc.get(r); + let Object::Stream(s) = &*obj else { continue }; + if !budget.enter_form() { + return None; + } + note(&budget.decode_strict(s).ok()?); + if let Some(own) = s.dict.get(b"Resources").map(|o| doc.resolve(o)).and_then(|o| o.as_dict().cloned()) { + dicts.push(own); + } + } + Some(drawn) +} + +/// The content streams the resource dictionary `res` lists: its form XObjects, its tiling +/// patterns, its Type 3 fonts' glyph procedures and its soft masks' groups. +fn drawn_through(doc: &Document, res: &Dict) -> Vec { + let is_stream = |r: &ObjRef| matches!(&*doc.get(*r), Object::Stream(_)); + let mut found: Vec = Vec::new(); + for (_, x) in res_dict(doc, res, b"XObject").iter() { + found.extend(x.as_ref().filter(|r| matches!(&*doc.get(*r), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Form")))); + } + for (_, p) in res_dict(doc, res, b"Pattern").iter() { + found.extend(p.as_ref().filter(|r| matches!(&*doc.get(*r), Object::Stream(s) if s.dict.int(b"PatternType") == Some(1)))); + } + for (_, f) in res_dict(doc, res, b"Font").iter() { + let font = doc.resolve(f); + let Some(font) = font.as_dict().filter(|f| f.name(b"Subtype") == Some(b"Type3")) else { continue }; + let procs = font.get(b"CharProcs").map(|c| doc.resolve(c)).and_then(|c| c.as_dict().cloned()).unwrap_or_default(); + found.extend(procs.iter().filter_map(|(_, v)| v.as_ref()).filter(is_stream)); + } + for (_, g) in res_dict(doc, res, b"ExtGState").iter() { + let gs = doc.resolve(g); + let mask = gs.as_dict().and_then(|g| g.get(b"SMask")).map(|m| doc.resolve(m)); + found.extend(mask.as_ref().and_then(|m| m.as_dict()).and_then(|m| m.reference(b"G")).filter(is_stream)); + } + found +} + +/// Unlink the XObjects a rewrite dropped or replaced (`doomed`) from every page-tree resource +/// dictionary that still names them. The rewritten page has its own pruned /Resources, but the +/// originals stay reachable, and so stay in a full-rewrite save, when: +/// - the page inherited its /Resources from a /Pages node, which still lists them; +/// - several pages share one resource dictionary, and another page still refers to it; +/// - a form without /Resources drew them through the page's dictionary. +/// +/// An entry goes unless a page that draws through that dictionary still draws the name (the same +/// image shown on another page, or again outside a mark). A dictionary that loses an entry is +/// replaced by a private copy on its node, so a dictionary object shared with other nodes is +/// left as it is for them. Retired properties are likewise unlinked only when none of the +/// holder's pages or inheriting forms names them. Fails closed when use cannot be determined. +pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_properties: &[Object]) -> Result<(), RedactError> { + crate::type3::unlink_type3(doc)?; + if doomed.is_empty() && retired_properties.is_empty() { + return Ok(()); + } + let doomed: HashSet = doomed.iter().copied().collect(); + let holders = resource_holders(doc); let index: HashMap = pdfcraft_model::pages(doc).iter().enumerate().map(|(i, p)| (p.obj, i)).collect(); let holder_nodes: Vec = holders.iter().map(|(node, _)| *node).collect(); for (node, users) in holders { diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index f8088e0bba..49cda651b6 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -671,6 +671,9 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result