aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-10-24 10:08:00 -06:00
Code Issues Packages Projects Releases Wiki Activity

Issue #878: Combine stores an image once when its colour space is an indirect name (#883)

Browse Source 
ImageMagick writes an image's colour space as its own object (`/ColorSpace 10 0 R`,
`10 0 obj /DeviceRGB`). Each source's copy of that object was never merged, since it
isn't a resource, so identical images compared unequal and every copy was kept:
8 copies of an 80 MB ImageMagick PDF combined into 648 MB instead of 81 MB.

Dedupe now compares a reference to a plain value (name, number, boolean or string) as
that value. The value objects themselves are still never merged. Combine, Insert Pages
and Reduce File Size share this code.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mlkonrad
2026-10-10 15:46:38 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 7c386fa4ac
commit 1845d9260d
2 changed files with 113 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/organize/src/dedupe.rs
+62 -12
View File
@@ -10,7 +10,10 @@
//! encoding dictionaries, and colour-space arrays. Objects whose *identity* matters (pages,
//! annotations, form fields, layers, outline items, structure elements) are never merged.
//! - Two objects are equal when they are structurally identical with references compared after
//! merging, so a font dictionary merges once its font file has. This runs to a fixpoint.
//! merging, so a font dictionary merges once its font file has. This runs to a fixpoint. A
//! reference to a plain value (a name, number, boolean or string, such as ImageMagick's
//! `/ColorSpace 10 0 R` → `/DeviceRGB`) compares as that value; the value objects themselves
//! are never merged.
//! - Only `candidates` (objects created by the operation) are ever replaced or rewritten. Existing
//! objects can serve as the merge target but are not changed, so an incremental save stays
//! small.
@@ -84,7 +87,14 @@ fn canon(map: &HashMap<ObjRef, ObjRef>, mut r: ObjRef) -> ObjRef {
r
}
fn hash_obj(o: &Object, map: &HashMap<ObjRef, ObjRef>, h: &mut impl Hasher) {
/// The value behind `r` when it is a plain value (name, number, boolean or string): it then
/// means that value wherever it is used, whichever object holds it (#878).
fn plain_value(doc: &Document, r: ObjRef) -> Option<std::sync::Arc<Object>> {
let o = doc.get(r);
matches!(*o, Object::Bool(_) | Object::Int(_) | Object::Real(_) | Object::Name(_) | Object::String(_)).then_some(o)
}
fn hash_obj(o: &Object, doc: &Document, map: &HashMap<ObjRef, ObjRef>, h: &mut impl Hasher) {
match o {
Object::Null => 0u8.hash(h),
Object::Bool(b) => (1u8, b).hash(h),
@@ -95,7 +105,7 @@ fn hash_obj(o: &Object, map: &HashMap<ObjRef, ObjRef>, h: &mut impl Hasher) {
Object::Array(a) => {
(6u8, a.len()).hash(h);
for x in a {
hash_obj(x, map, h);
hash_obj(x, doc, map, h);
}
}
Object::Dict(d) => {
@@ -104,28 +114,39 @@ fn hash_obj(o: &Object, map: &HashMap<ObjRef, ObjRef>, h: &mut impl Hasher) {
for (k, v) in d.iter() {
let mut eh = std::collections::hash_map::DefaultHasher::new();
k.hash(&mut eh);
hash_obj(v, map, &mut eh);
hash_obj(v, doc, map, &mut eh);
acc = acc.wrapping_add(eh.finish());
}
(7u8, d.len(), acc).hash(h);
}
Object::Stream(s) => {
8u8.hash(h);
hash_obj(&Object::Dict(s.dict.clone()), map, h);
hash_obj(&Object::Dict(s.dict.clone()), doc, map, h);
s.raw[..].hash(h);
}
Object::Ref(r) => {
let c = canon(map, *r);
(9u8, c.num, c.generation).hash(h);
match plain_value(doc, c) {
Some(v) => hash_obj(&v, doc, map, h),
None => (9u8, c.num, c.generation).hash(h),
}
}
}
}
fn eq_obj(a: &Object, b: &Object, map: &HashMap<ObjRef, ObjRef>) -> bool {
fn eq_obj(a: &Object, b: &Object, doc: &Document, map: &HashMap<ObjRef, ObjRef>) -> bool {
// Plain values behind references compare as the values (as `hash_obj` hashes them).
let plain = |o: &Object| if let Object::Ref(r) = o { plain_value(doc, canon(map, *r)) } else { None };
match (plain(a), plain(b)) {
(Some(x), Some(y)) => return eq_obj(&x, &y, doc, map),
(Some(x), None) => return eq_obj(&x, b, doc, map),
(None, Some(y)) => return eq_obj(a, &y, doc, map),
(None, None) => {}
}
match (a, b) {
(Object::Array(x), Object::Array(y)) => x.len() == y.len() && x.iter().zip(y).all(|(p, q)| eq_obj(p, q, map)),
(Object::Dict(x), Object::Dict(y)) => x.len() == y.len() && x.iter().all(|(k, v)| y.get(k).is_some_and(|w| eq_obj(v, w, map))),
(Object::Stream(x), Object::Stream(y)) => x.raw == y.raw && eq_obj(&Object::Dict(x.dict.clone()), &Object::Dict(y.dict.clone()), map),
(Object::Array(x), Object::Array(y)) => x.len() == y.len() && x.iter().zip(y).all(|(p, q)| eq_obj(p, q, doc, map)),
(Object::Dict(x), Object::Dict(y)) => x.len() == y.len() && x.iter().all(|(k, v)| y.get(k).is_some_and(|w| eq_obj(v, w, doc, map))),
(Object::Stream(x), Object::Stream(y)) => x.raw == y.raw && eq_obj(&Object::Dict(x.dict.clone()), &Object::Dict(y.dict.clone()), doc, map),
(Object::Ref(x), Object::Ref(y)) => canon(map, *x) == canon(map, *y),
(Object::Real(x), Object::Real(y)) => x.to_bits() == y.to_bits(),
(Object::String(x), Object::String(y)) => x.bytes == y.bytes,
@@ -201,9 +222,9 @@ pub fn dedupe_resources(doc: &mut Document, candidates: &[ObjRef], index_existin
continue;
}
let mut h = std::collections::hash_map::DefaultHasher::new();
hash_obj(o, &map, &mut h);
hash_obj(o, doc, &map, &mut h);
let bucket = buckets.entry(h.finish()).or_default();
let target = bucket.iter().map(|j| &universe[*j]).find(|(_, other)| eq_obj(o, other, &map)).map(|(t, _)| *t);
let target = bucket.iter().map(|j| &universe[*j]).find(|(_, other)| eq_obj(o, other, doc, &map)).map(|(t, _)| *t);
match target {
Some(t) if cand.contains(r) => {
map.insert(*r, t);
@@ -304,6 +325,35 @@ mod tests {
assert_eq!(doc.get(p1), doc.get(p2), "both pages now use the one graphics state");
}
fn image(doc: &mut Document, colour_space: Object) -> ObjRef {
let mut d = Dict::new();
d.set(b"Subtype".to_vec(), Object::name("Image"));
d.set(b"ColorSpace".to_vec(), colour_space);
doc.add(Object::Stream(Stream { dict: d, raw: b"pixels".to_vec().into() }))
}
#[test]
fn references_to_plain_values_compare_as_the_values() {
// ImageMagick writes `/ColorSpace 10 0 R` with `10 0 obj /DeviceRGB`, one per file (#878).
let mut doc = Document::new_empty();
let rgb1 = doc.add(Object::name("DeviceRGB"));
let rgb2 = doc.add(Object::name("DeviceRGB"));
let grey = doc.add(Object::name("DeviceGray"));
let a = image(&mut doc, Object::Ref(rgb1));
let b = image(&mut doc, Object::Ref(rgb2));
let c = image(&mut doc, Object::name("DeviceRGB")); // written directly: the same image
let d = image(&mut doc, Object::Ref(grey)); // same pixels, another colour space
let mut page = Dict::new();
page.set(b"Images".to_vec(), Object::Array([a, b, c, d].map(Object::Ref).to_vec()));
let p = doc.add(Object::Dict(page));
assert_eq!(dedupe_resources(&mut doc, &[rgb1, rgb2, grey, a, b, c, d, p], false), 2);
let page = doc.get(p);
let images = page.as_dict().unwrap().get(b"Images").unwrap().as_array().unwrap().clone();
assert_eq!(images, [a, a, a, d].map(Object::Ref).to_vec());
let nums = doc.object_numbers();
assert!(nums.contains(&rgb1.num) && nums.contains(&rgb2.num), "the values themselves are never merged");
}
#[test]
fn existing_objects_are_targets_but_never_changed() {
let mut doc = Document::new_empty();
crates/organize/src/tests.rs
+51
View File
@@ -734,6 +734,57 @@ fn combine_and_insert_share_identical_resources() {
assert_eq!(labels(&d), ["A1", "A2", "A3", "A1", "A2"]);
}
/// A one-page document drawing one image, written the way ImageMagick writes it: the colour
/// space is an indirect `/DeviceRGB` object of its own (#878).
fn indirect_colour_space_doc() -> Document {
let objs: Vec<Vec<u8>> = 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 100 100] /Resources << /XObject << /Im0 5 0 R >> >> /Contents 4 0 R >>".to_vec(),
stream(b"q 100 0 0 100 0 0 cm /Im0 Do Q"),
[
b"<< /Type /XObject /Subtype /Image /Width 2 /Height 1 /ColorSpace 6 0 R /BitsPerComponent 8 /Length 6 >>\nstream\n".as_slice(),
&[255, 0, 0, 0, 0, 255],
b"\nendstream",
]
.concat(),
b"/DeviceRGB".to_vec(),
];
let mut out = b"%PDF-1.7\n".to_vec();
let mut offsets = Vec::new();
for (i, o) in objs.iter().enumerate() {
offsets.push(out.len());
out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes());
out.extend_from_slice(o);
out.extend_from_slice(b"\nendobj\n");
}
let xref = out.len();
out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes());
for o in offsets {
out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes());
}
out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
open(out)
}
#[test]
fn combine_stores_an_image_once_when_its_colour_space_is_an_indirect_name() {
let images = |d: &Document| {
d.object_numbers()
.into_iter()
.filter(|n| matches!(&*d.get(ObjRef::new(*n, d.generation(*n))), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Image")))
.count()
};
let src = indirect_colour_space_doc();
let out = full_roundtrip(&crate::combine(&[("one", &src), ("two", &src), ("three", &src)]).unwrap());
assert_eq!(images(&out), 1);
assert_eq!(pages(&out).unwrap().len(), 3);
// Inserting the same pages again reuses the image already there.
let mut d = indirect_colour_space_doc();
crate::import_pages(&mut d, &indirect_colour_space_doc(), &[0], 1).unwrap();
assert_eq!(images(&full_roundtrip(&d)), 1);
}
// ---- bookmarks (M4.6) --------------------------------------------------------------------------
fn titles(b: &[crate::Bookmark]) -> Vec<String> {