From 59dbdee4286101c398271a28faf34c73fb1b44e8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:05 +0000 Subject: [PATCH] fix(organize): copy annotation actions with one shared reference map Fixing up copied annotations cloned the whole source-to-destination map for every non-GoTo action and dropped the copies it made, so many annotations cost quadratic memory and time and an object shared by their actions was copied once per action. The copier now lends its map to the action copy and takes it back with the new entries. --- crates/organize/src/import.rs | 15 +++++++++------ crates/organize/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index eb617ffed..5670f0859 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -154,8 +154,8 @@ impl<'a> Copier<'a> { } /// Rewrite `/P`, `/Dest` and GoTo actions on copied annotations. - fn fix_annotations(&self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { - for &a in &self.annots { + fn fix_annotations(&mut self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { + for &a in &std::mem::take(&mut self.annots) { let src_dict = dst.get(a).as_dict().cloned(); let Some(d) = src_dict else { continue }; let mut d = d; @@ -188,10 +188,13 @@ impl<'a> Copier<'a> { } } } else { - // Other actions (URI, Launch, JavaScript…) carry no page references. - let mut c = Copier::new(self.src, self.map.clone(), HashMap::new()); - let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None)?; - d.set(b"A".to_vec(), copied); + // Other actions (URI, Launch, JavaScript…) carry no page references. The + // copier lends its map out and takes it back with the new copies in it, + // so actions sharing an object share its one copy. + let mut c = Copier::new(self.src, std::mem::take(&mut self.map), HashMap::new()); + let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None); + self.map = c.map; + d.set(b"A".to_vec(), copied?); } } } diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index dcab68a49..c2bb725d7 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3175,3 +3175,19 @@ fn renaming_many_same_named_fields_resumes_the_suffix_search() { assert_eq!(renames.first(), Some(&("x".to_string(), "x-1".to_string()))); assert_eq!(renames.last(), Some(&("x".to_string(), format!("x-{}", n - 1)))); } + +#[test] +fn annotation_actions_share_the_objects_they_copy() { + // Fifty links whose URI actions all point at one string object. + let n = 50; + let links: Vec = + (0..n).map(|_| format!("<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI {} 0 R >> >>", n + 5)).collect(); + let src = page_with_annots(&links, &["(https://example.com/shared)".into()], ""); + let out = extract_pages(&src, &[0]).unwrap(); + let copies = out + .object_numbers() + .into_iter() + .filter(|&o| out.get(ObjRef::new(o, out.generation(o))).as_string().is_some_and(|s| s.bytes == b"https://example.com/shared")) + .count(); + assert_eq!(copies, 1, "the shared URI is copied once, not once per action"); +}