From 509b7afc92a4d3cc910f66e38ecf5dbab324c282 Mon Sep 17 00:00:00 2001 From: Velli20 Date: Sun, 25 Oct 2026 15:59:49 +0200 Subject: [PATCH] Inherit page-tree attributes in one top-down pass Decoding the page tree applied each /Pages node's inherited attributes to every descendant at every level, so a page with its own /Resources was merged once per resource-carrying ancestor, and pages sharing one /Resources dictionary each got their own deep copy. The tree is now decoded as written and flattened once from the root: each node combines with its ancestors once, each page is merged at most once, and merges are shared between pages that point at the same resources below the same ancestors. Resources::merged_with_parent returns None when the parent adds nothing, so the child's resources are kept as they are. Co-Authored-By: Claude Opus 5.5 --- crates/pdf-document/src/page_tree.rs | 157 ++++++++++++++++++++------ crates/pdf-resources/src/resources.rs | 66 ++++++----- 2 files changed, 164 insertions(+), 59 deletions(-) diff --git a/crates/pdf-document/src/page_tree.rs b/crates/pdf-document/src/page_tree.rs index a4e6f8db..550473aa 100644 --- a/crates/pdf-document/src/page_tree.rs +++ b/crates/pdf-document/src/page_tree.rs @@ -3,7 +3,7 @@ use pdf_object_reader::{ FromPdfObject, ObjectAccess, ObjectContext, ObjectReadError, ReadResult, pdf_array::PdfArray, }; -use std::sync::Arc; +use std::{collections::HashMap, sync::Arc}; use crate::page::PdfPage; @@ -21,12 +21,29 @@ impl PageTree { pub const KEY: &'static [u8] = b"Pages"; } +/// Decodes the tree, then passes inherited attributes down it once, so each page and +/// each `/Pages` node combines with its ancestors a single time. +impl FromPdfObject for PageTree { + fn from_pdf_object( + mut context: ObjectContext<'_, impl ObjectAccess + ?Sized>, + ) -> ReadResult { + let root: PageTreeBranch = context.read(context.object())?; + let mut pages = Vec::new(); + root.flatten_into( + &InheritedAttributes::default(), + &mut pages, + &mut ResourceMerges::default(), + ); + Ok(Self { pages }) + } +} + /// A `/Kids` entry of a page-tree node, dispatched on its `/Type`. enum PageTreeNode { /// A leaf `/Page` object. Page(PdfPage), /// An intermediate `/Pages` node. - Pages(PageTree), + Pages(PageTreeBranch), } impl FromPdfObject for PageTreeNode { @@ -37,7 +54,7 @@ impl FromPdfObject for PageTreeNode { context.read(context.object().dictionary()?.get_or_err(b"Type")?)?; match &*node_type { PdfPage::KEY => PdfPage::from_pdf_object(context).map(Self::Page), - PageTree::KEY => PageTree::from_pdf_object(context).map(Self::Pages), + PageTree::KEY => PageTreeBranch::from_pdf_object(context).map(Self::Pages), other => Err(PdfPagesError::InvalidKidsEntryType { found_type: String::from_utf8_lossy(other).into_owned(), } @@ -46,23 +63,53 @@ impl FromPdfObject for PageTreeNode { } } -impl FromPdfObject for PageTree { +/// A `/Pages` node as written: its own inheritable attributes and its kids, with +/// nothing inherited yet. +struct PageTreeBranch { + /// The attributes this node defines for its descendants. + inherited: InheritedAttributes, + /// The readable `/Kids` entries, in source order. + kids: Vec, +} + +impl FromPdfObject for PageTreeBranch { fn from_pdf_object(context: ObjectContext<'_, impl ObjectAccess + ?Sized>) -> ReadResult { let object = context.object(); let mut context = context.dictionary()?; - let kids = context.required::(b"Kids")?; - let inherited: InheritedAttributes = context.read(object)?; - let mut pages = Vec::new(); - for value in kids.iter() { + let kids_array = context.required::(b"Kids")?; + let inherited = context.read(object)?; + let mut kids = Vec::with_capacity(kids_array.len()); + for value in kids_array.iter() { match context.read::(value) { - Ok(PageTreeNode::Page(page)) => pages.push(page), - Ok(PageTreeNode::Pages(child)) => pages.extend(child.pages), + Ok(kid) => kids.push(kid), Err(ObjectReadError::CyclicReference { .. }) => continue, Err(error) => return Err(error), } } - pages.iter_mut().for_each(|page| inherited.apply_to(page)); - Ok(Self { pages }) + Ok(Self { inherited, kids }) + } +} + +impl PageTreeBranch { + /// Appends this node's pages to `pages` in source order, each with the attributes + /// it inherits from this node and from `parent`, the combined attributes of its + /// ancestors. + fn flatten_into( + self, + parent: &InheritedAttributes, + pages: &mut Vec, + merges: &mut ResourceMerges, + ) { + let inherited = self.inherited.under(parent, merges); + for kid in self.kids { + match kid { + PageTreeNode::Page(mut page) => { + inherited.apply_to(&mut page, merges); + pages.push(page); + } + PageTreeNode::Pages(branch) => branch.flatten_into(&inherited, pages, merges), + } + } } } @@ -92,27 +139,70 @@ impl FromPdfObject for InheritedAttributes { } impl InheritedAttributes { + /// Combines a node's own attributes with those it inherits from `parent`. + fn under(self, parent: &Self, merges: &mut ResourceMerges) -> Self { + Self { + resources: merges.inherit(self.resources, parent.resources.as_ref()), + media_box: self.media_box.or(parent.media_box), + crop_box: self.crop_box.or(parent.crop_box), + rotation: self.rotation.or(parent.rotation), + } + } + /// Fills in the attributes a descendant page does not define itself. - /// - /// Per PDF spec §7.7.4, a page with no `/Resources` inherits the ancestor's resources - /// directly, while a page with its own `/Resources` keeps its entries and fills in any - /// categories or names it lacks from the ancestor. - fn apply_to(&self, page: &mut PdfPage) { + fn apply_to(&self, page: &mut PdfPage, merges: &mut ResourceMerges) { page.media_box = page.media_box.or(self.media_box); page.crop_box = page.crop_box.or(self.crop_box); page.rotation = page.rotation.or(self.rotation); - let Some(inherited) = &self.resources else { - return; - }; - match &page.resources { + page.resources = merges.inherit(page.resources.take(), self.resources.as_ref()); + } +} + +/// The resource merges made while flattening a page tree, so nodes and pages that +/// share a `/Resources` dictionary below the same ancestors share one merged copy. +#[derive(Default)] +struct ResourceMerges { + /// Merges keyed by the addresses of the own and the inherited resources. + merged: HashMap<(usize, usize), ResourceMerge>, +} + +/// One merge of a node's own resources with the resources it inherits. +struct ResourceMerge { + /// The own resources, held so their address is not reused while it keys a merge. + _own: Arc, + /// The inherited resources, held for the same reason. + _parent: Arc, + /// The merged resources, or the own resources when the parent adds nothing. + merged: Arc, +} + +impl ResourceMerges { + /// Returns the resources of a node or page given its own and the inherited ones. + /// + /// Per PDF spec §7.7.4, a node with no `/Resources` inherits the ancestor's resources + /// directly, while a node with its own `/Resources` keeps its entries and fills in any + /// categories or names it lacks from the ancestor. + fn inherit( + &mut self, + own: Option>, + parent: Option<&Arc>, + ) -> Option> { + match (own, parent) { + (own, None) => own, + (None, Some(parent)) => Some(Arc::clone(parent)), // The same shared resources object: nothing to merge. - Some(own) if Arc::ptr_eq(own, inherited) => {} - Some(own) => { - if let Some(merged) = own.merged_with_parent(inherited) { - page.resources = Some(Arc::new(merged)); - } + (Some(own), Some(parent)) if Arc::ptr_eq(&own, parent) => Some(own), + (Some(own), Some(parent)) => { + let key = (Arc::as_ptr(&own).addr(), Arc::as_ptr(parent).addr()); + let merge = self.merged.entry(key).or_insert_with(|| ResourceMerge { + merged: own + .merged_with_parent(parent) + .map_or_else(|| Arc::clone(&own), Arc::new), + _own: own, + _parent: Arc::clone(parent), + }); + Some(Arc::clone(&merge.merged)) } - None => page.resources = Some(Arc::clone(inherited)), } } } @@ -156,7 +246,7 @@ mod tests { resources: Some(Arc::clone(&parent)), ..Default::default() } - .apply_to(&mut page); + .apply_to(&mut page, &mut ResourceMerges::default()); let inherited = page .resources @@ -201,13 +291,14 @@ mod tests { resources: Some(Arc::clone(&parent)), ..Default::default() } - .apply_to(&mut page); + .apply_to(&mut page, &mut ResourceMerges::default()); let result = page .resources .as_ref() .expect("page should still have resources"); - assert!(!Arc::ptr_eq(result, &child_res)); + // The parent adds nothing the child lacks, so the child's resources are kept as is. + assert!(Arc::ptr_eq(result, &child_res)); // "CS1" must remain the child's version (DeviceRGB), not replaced by the parent's (DeviceGray). assert!( @@ -256,7 +347,7 @@ mod tests { resources: Some(Arc::clone(&parent)), ..Default::default() } - .apply_to(&mut page); + .apply_to(&mut page, &mut ResourceMerges::default()); let result = page.resources.as_ref().expect("page should have resources"); @@ -299,7 +390,7 @@ mod tests { media_box: Some(parent_mb), ..Default::default() } - .apply_to(&mut page); + .apply_to(&mut page, &mut ResourceMerges::default()); let inherited = page .media_box @@ -338,7 +429,7 @@ mod tests { media_box: Some(parent_mb), ..Default::default() } - .apply_to(&mut page); + .apply_to(&mut page, &mut ResourceMerges::default()); let result = page.media_box.expect("page should still have media box"); assert_eq!( diff --git a/crates/pdf-resources/src/resources.rs b/crates/pdf-resources/src/resources.rs index 5a1fefc1..b01c07fb 100644 --- a/crates/pdf-resources/src/resources.rs +++ b/crates/pdf-resources/src/resources.rs @@ -172,37 +172,51 @@ impl Resources { /// in one category (e.g. a font named `"F1"`) never blocks inheritance of a /// parent entry of a different category with the same name (e.g. an XObject /// named `"F1"`). + /// + /// Returns `None` when `parent` defines no entry that `self` lacks, so `self` + /// can be used unchanged. pub fn merged_with_parent(&self, parent: &Self) -> Option { let child = self; - - Some(Self { - fonts: Self::merged_category(&child.fonts, &parent.fonts), - ext_g_states: Self::merged_category(&child.ext_g_states, &parent.ext_g_states), - patterns: Self::merged_category(&child.patterns, &parent.patterns), - xobjects: Self::merged_category(&child.xobjects, &parent.xobjects), - shadings: Self::merged_category(&child.shadings, &parent.shadings), - color_spaces: Self::merged_category(&child.color_spaces, &parent.color_spaces), - optional_content: Self::merged_category( - &child.optional_content, - &parent.optional_content, - ), + let inherits_any = child.fonts.lacks_any_of(&parent.fonts) + || child.ext_g_states.lacks_any_of(&parent.ext_g_states) + || child.patterns.lacks_any_of(&parent.patterns) + || child.xobjects.lacks_any_of(&parent.xobjects) + || child.shadings.lacks_any_of(&parent.shadings) + || child.color_spaces.lacks_any_of(&parent.color_spaces) + || child + .optional_content + .lacks_any_of(&parent.optional_content); + inherits_any.then(|| Self { + fonts: child.fonts.merged_with(&parent.fonts), + ext_g_states: child.ext_g_states.merged_with(&parent.ext_g_states), + patterns: child.patterns.merged_with(&parent.patterns), + xobjects: child.xobjects.merged_with(&parent.xobjects), + shadings: child.shadings.merged_with(&parent.shadings), + color_spaces: child.color_spaces.merged_with(&parent.color_spaces), + optional_content: child.optional_content.merged_with(&parent.optional_content), }) } +} + +/// One named resource category, merged with the same category of an ancestor. +trait ResourceCategory { + /// Returns whether `parent` defines a name that `self` does not. + fn lacks_any_of(&self, parent: &Self) -> bool; + + /// Returns `self` with the entries only `parent` defines added. + fn merged_with(&self, parent: &Self) -> Self; +} + +impl ResourceCategory for HashMap, T> { + fn lacks_any_of(&self, parent: &Self) -> bool { + parent.keys().any(|name| !self.contains_key(name)) + } - /// Returns one merged resource category with child entries taking precedence. - fn merged_category( - child: &HashMap, T>, - parent: &HashMap, T>, - ) -> HashMap, T> { - let mut merged = HashMap::with_capacity(child.len().saturating_add(parent.len())); - merged.extend( - child - .iter() - .map(|(name, value)| (name.clone(), value.clone())), - ); - for (k, v) in parent { - if !merged.contains_key(k) { - merged.insert(k.clone(), v.clone()); + fn merged_with(&self, parent: &Self) -> Self { + let mut merged = self.clone(); + for (name, value) in parent { + if !merged.contains_key(name) { + merged.insert(name.clone(), value.clone()); } } merged