From 4b2a15101288029197e1c7dae1bfac0c6dc137a2 Mon Sep 17 00:00:00 2001 From: xullul Date: Sat, 7 Nov 2026 18:34:05 +0700 Subject: [PATCH] M7: count each grouped placement once across preparation passes Change: - Read and count only the placements a preparation pass newly inlines. The others were read on an earlier pass in the context they still have, so a group refused where it is drawn (translucent, for example) is no longer counted again on every pass: 33 such placements beside one text group used to reach the 64-placement limit on the second pass and prepare nothing. A form refused in one place is still read where an expanded parent draws it anew. If the page doesn't parse back to the operators written, every placement is read again, as before. - Read a group refused because its resources needed renaming again, without counting it, once names merged in for other groups make the renaming unnecessary (checked from its resource dictionaries alone, without decoding its content), and drop its earlier refusal when it is then prepared. Validation: edit 470 and the 2 automation text_prepare tests passed; formatting, strict Clippy for pdfcraft-edit, layers and assets passed. The wasm check was not run locally (the wasm32 target is not installed), nor were the engine suite and the workspace sweep. Each new regression failed with its fix disabled. --- crates/edit/src/tests.rs | 54 +++++++++++++++++++ crates/edit/src/text/forms.rs | 78 ++++++++++++++++++++++++---- crates/edit/src/text/forms/expand.rs | 28 +++++++++- 3 files changed, 147 insertions(+), 13 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index d3c0e4fdcd..5ae27517d3 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -11021,6 +11021,60 @@ fn a_placement_refusal_does_not_settle_the_form_for_later_passes() { assert_eq!(text::text_blocks(&doc, 0).unwrap().iter().filter(|b| b.text == "shared").count(), 1, "only the opaque one is the page's own text"); } +/// A placement refused where it is drawn is read once, not again on every pass, while the same +/// form drawn anew by an expanded parent is still read in its own place: 33 translucent +/// placements of a text group and a parent drawing it opaque are 35 reads, within the +/// 64-placement limit, where reading the 33 again on the second pass would pass it and prepare +/// nothing. +#[test] +fn a_group_refused_where_it_is_drawn_counts_once_against_the_placement_limit() { + let resources = "<< /Font << /F1 5 0 R >> /ExtGState << /Alpha << /ca 0.5 >> >> /XObject << /Shared 6 0 R /Parent 7 0 R >> >>"; + let form = |extra: &str, body: &str| stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 612 792] {extra} /Resources {resources}"), body); + let content = format!("{}/Parent Do", "q /Alpha gs /Shared Do Q\n".repeat(33)); + let objs = [ + "<< /Type /Catalog /Pages 2 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_string(), + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources {resources} /Contents 4 0 R >>"), + stream("", &content), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_string(), + form("/Group << /S /Transparency >>", "BT /F1 14 Tf 40 500 Td (shared) Tj ET"), + form("", "/Shared Do"), + ]; + let mut doc = build(&objs.iter().map(String::as_str).collect::>()); + let result = text::forms::prepare(&mut doc, 0).unwrap_or_else(|e| panic!("each placement is read once ({e})")); + assert_eq!(result.expanded, 2, "Parent, then the Shared it draws opaque: {:?}", result.refused); + assert!(result.refused.iter().any(|r| r.starts_with("Shared: ") && r.contains("translucent")), "{:?}", result.refused); + let drawn = pdfcraft_content::parse(&page_content_bytes(&doc, 0)).ops.iter().filter(|op| op.is("Do")).count(); + assert_eq!(drawn, 33, "the translucent placements stay grouped"); +} + +/// A group refused because its resources would need renaming (it draws a form that looks them +/// up by name) is prepared on a later pass once another group's merge has given the page those +/// very names. A lists the child as `/PCUg1`; B lists it as `/Other`, which merging B names +/// `/PCUg1` on the page. Its first refusal is then no longer reported. +#[test] +fn a_group_refused_for_renaming_is_prepared_once_the_page_has_its_names() { + let form = |resources: &str, body: &str| stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 612 792] /Resources {resources}"), body); + let objs = [ + "<< /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 612 792] /Resources << /Font << /F1 5 0 R >> /XObject << /A 6 0 R /B 7 0 R >> >> /Contents 4 0 R >>" + .to_string(), + stream("", "/A Do\n/B Do"), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_string(), + form("<< /Font << /F1 5 0 R >> /XObject << /PCUg1 8 0 R >> >>", "BT /F1 14 Tf 40 500 Td (alpha) Tj ET /PCUg1 Do"), + form("<< /Font << /F1 5 0 R >> /XObject << /Other 8 0 R >> >>", "BT /F1 14 Tf 40 400 Td (beta) Tj ET"), + // The child has no resources of its own: it looks names up in whatever draws it. + stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10]", "0 0 10 10 re f"), + ]; + let mut doc = build(&objs.iter().map(String::as_str).collect::>()); + let result = text::forms::prepare(&mut doc, 0).unwrap(); + assert_eq!(result.expanded, 2, "B, then A once the page names the child as A does: {:?}", result.refused); + assert!(result.refused.is_empty(), "A was prepared after all: {:?}", result.refused); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert!(["alpha", "beta"].iter().all(|t| blocks.iter().any(|b| b.text == *t)), "{blocks:?}"); +} + /// The `Do` is matched to its resource by the name's bytes as written, which need not be UTF-8. #[test] fn grouped_text_under_a_non_utf8_resource_name_is_prepared() { diff --git a/crates/edit/src/text/forms.rs b/crates/edit/src/text/forms.rs index da46a16d91..fcc7a31e13 100644 --- a/crates/edit/src/text/forms.rs +++ b/crates/edit/src/text/forms.rs @@ -24,6 +24,17 @@ pub struct Preparation { /// grouped and is reported in [`Preparation::refused`]. const MAX_LEVELS: usize = 8; +/// What one pass of [`prepare`] inlined, as positions in the next pass's parse of the page. +struct Inlined { + /// How many operators that parse has; with any other count the positions are wrong. + ops: usize, + /// Where each inlined form's operators are: the placements in them are new. + ranges: Vec>, + /// Earlier placements refused only because their resources needed renaming + /// ([`expand::Refusal::renaming`]), each with the reason it was given. + waiting: Vec<(usize, String)>, +} + /// Prepare grouped source text atomically, within eight levels and 64 placements per page. /// Shared forms and other pages are untouched. Preparation is a distinct undoable engine edit. pub fn prepare(doc: &mut Document, page: usize) -> Result { @@ -33,10 +44,17 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result = None; // One pass opens one level of every placement; the last pass only reports what is deeper. for pass in 0..=MAX_LEVELS { let p = page_dict(&next, page)?; @@ -47,11 +65,19 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result (Some(i.ranges), i.waiting), + _ => (None, Vec::new()), + }; + let mut still_waiting = Vec::new(); let resources = p.dict.get(b"Resources").map(|r| next.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let space = expand::page_blend_space(&next, &p.dict); let images: std::collections::BTreeMap<_, _> = crate::page_images(&next, page)?.into_iter().filter(|i| i.is_form).map(|i| (i.op, i)).collect(); - let mut replacements = std::collections::HashMap::new(); + let mut replacements = std::collections::BTreeMap::new(); let mut merged = resources; let mut pending_path = false; let mut in_text = false; @@ -68,6 +94,25 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result::to_vec); + // An earlier placement is read again only when it was refused for a rename that the + // names merged since have made unnecessary. The last pass opens nothing, so there it + // keeps its reason. + let earlier = new.as_ref().is_some_and(|ranges| !ranges.iter().any(|r| r.contains(&at))); + if earlier { + let Some(w) = waiting.iter().position(|(i, _)| *i == at) else { continue }; + let (_, reason) = waiting.swap_remove(w); + if pass == MAX_LEVELS || !expand::merges_without_renaming(&next, &merged, &name) { + still_waiting.push((at, reason)); + continue; + } + // What reading it again finds replaces the reason it was given. + if let Some(i) = result.refused.iter().position(|r| *r == reason) { + result.refused.remove(i); + } + } if pending_path || in_text { result.refused.push(format!("{}: the group is drawn inside an unfinished path or text object", image.name)); continue; @@ -78,14 +123,13 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result 64 { - return Err(EditError::Invalid("too many grouped text placements on this page (limit 64); the page was left unchanged".into())); + if !earlier { + visits = visits.saturating_add(1); + if visits > 64 { + return Err(EditError::Invalid("too many grouped text placements on this page (limit 64); the page was left unchanged".into())); + } } let translucent = expand::translucent_at(&next, &merged, &ops, at); - // The resource name exactly as the `Do` writes it (its only operand): a name need not - // be UTF-8, and `image.name` is only its display form. - let name = op.last_name().map_or_else(|| image.name.as_bytes().to_vec(), <[u8]>::to_vec); match expand::ungroup_ops(&next, &merged, &name, translucent, space.as_ref(), Matrix(image.matrix), &mut decode) { Ok(expand::Ungrouped { ops: inline, resources: mut res, patterns, shows_text }) => { // Vector and image artwork needs no preparation. Nested text groups are @@ -115,10 +159,14 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result { - result.refused.push(format!("{}: {}", image.name, refusal.error)); + let reason = format!("{}: {}", image.name, refusal.error); + if refusal.renaming { + still_waiting.push((at, reason.clone())); + } if refusal.intrinsic { refused_here.extend(image.object); } + result.refused.push(reason); } } } @@ -126,6 +174,14 @@ pub fn prepare(doc: &mut Document, page: usize) -> Result (ops, false), None => (Vec::new(), true), diff --git a/crates/edit/src/text/forms/expand.rs b/crates/edit/src/text/forms/expand.rs index 54495900cc..d674e1f473 100644 --- a/crates/edit/src/text/forms/expand.rs +++ b/crates/edit/src/text/forms/expand.rs @@ -67,6 +67,10 @@ pub(crate) struct Refusal { /// page so far is: its translucency or matrix, the resources it would merge into (which grow /// as other groups are prepared), or the decode budget left. pub(crate) intrinsic: bool, + /// The form's resources need renaming where it is drawn, and it draws something that would + /// look them up by their old names. Names merged in for other forms can make the renaming + /// unnecessary, which [`merges_without_renaming`] checks without reading the content. + pub(crate) renaming: bool, } /// Ungroup form `name` from the page's `resources`. `translucent` says whether the graphics state @@ -83,7 +87,11 @@ pub(crate) fn ungroup_ops( placement: Matrix, budget: &mut DecodeBudget, ) -> Result { - let refusal = |why: &str, intrinsic: bool| Refusal { error: EditError::Invalid(format!("this artwork can't be ungrouped: {why}")), intrinsic }; + let refusal = |why: &str, intrinsic: bool| Refusal { + error: EditError::Invalid(format!("this artwork can't be ungrouped: {why}")), + intrinsic, + renaming: false, + }; // Something about this placement or the page so far is the reason. let refuse = |why: &str| refusal(why, false); // The form itself is the reason: every placement of it is refused the same way. @@ -215,7 +223,7 @@ pub(crate) fn ungroup_ops( // would find the page's, which differ wherever a name was renamed (every form resource that // isn't already the page's, the nested item's own entry included). if !renames.is_empty() && ops.iter().any(|op| inherits_resources(doc, &form_res, op)) { - return Err(refuse("it draws a nested form, Type 3 font or pattern that uses its resources")); + return Err(Refusal { renaming: true, ..refuse("it draws a nested form, Type 3 font or pattern that uses its resources") }); } let mut depth = 0usize; // Marked content and text objects must balance within the form's own stream: an extra EMC @@ -383,6 +391,22 @@ pub(crate) fn ungroup_ops( Ok(Ungrouped { ops: out, resources: merged, patterns, shows_text }) } +/// Whether form `name` in `resources` would merge into them without a rename: it has resources +/// of its own, and each of their entries is already in `resources` under the same name, as +/// [`ungroup_ops`] compares them. Only dictionaries are read, never the form's content. +pub(crate) fn merges_without_renaming(doc: &Document, resources: &Dict, name: &[u8]) -> bool { + let category = |d: &Dict, cat: &[u8]| d.get(cat).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()); + let Some(form) = category(resources, b"XObject").and_then(|x| x.get(name).cloned()) else { return false }; + let Object::Stream(s) = &*doc.resolve(&form) else { return false }; + let Some(own) = s.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()) else { return false }; + CATEGORIES.into_iter().all(|cat| { + let Some(theirs) = category(&own, cat) else { return true }; + let mine = category(resources, cat).unwrap_or_default(); + // Past these sizes `ungroup_ops` refuses the form anyway. + theirs.len() <= 4096 && mine.len() <= 4096 && theirs.iter().all(|(key, value)| mine.get(key) == Some(value)) + }) +} + /// A copy of pattern `entry` whose `/Matrix` maps its space to the page's default space instead /// of the form's: its own matrix, then `placement`. `None` when it isn't a readable pattern, its /// matrix is unreadable, the result isn't finite or a tiling pattern is too large to copy.