diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 039f3dd101..e7d7255306 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -160,9 +160,12 @@ impl Document { /// `s` with the `/DecodeParms` its filters use made direct, so [`Stream::decoded`] reads them: /// an indirect dictionary resolved, and of an array (itself perhaps indirect), the entries at - /// the filters' places (`/Crypt` counts, ISO 32000-2 §7.4.1) resolved, the rest dropped. `s` - /// itself when none of those is indirect, or when the chain is longer than any decoder takes - /// ([`pdfcraft_filters::MAX_CHAIN`]), so at most that many objects are copied. + /// the filters' places (`/Crypt` counts, ISO 32000-2 §7.4.1) resolved, the rest dropped. Of + /// each dictionary only the entries the decoders read are kept, numbers and flags given by + /// reference (`/Predictor 7 0 R`) resolved; `/JBIG2Globals` stays a reference. `s` itself + /// when none of those is indirect, or when the chain is longer than any decoder takes + /// ([`pdfcraft_filters::MAX_CHAIN`]). Only small values are copied, so a file can't have a + /// large object copied once for each key and filter. pub fn with_direct_parms<'s>(&self, s: &'s Stream) -> Cow<'s, Stream> { let Some(p) = s.dict.get(b"DecodeParms") else { return Cow::Borrowed(s) }; let filters = match s.dict.get(b"Filter") { @@ -173,24 +176,45 @@ impl Document { if filters > pdfcraft_filters::MAX_CHAIN { return Cow::Borrowed(s); } - let whole = matches!(p, Object::Ref(_)); - let resolved = self.resolve(p); - let direct = match &*resolved { - Object::Array(a) => { - let used = a.get(..filters.min(a.len())).unwrap_or_default(); - if !whole && !used.iter().any(|e| matches!(e, Object::Ref(_))) { - return Cow::Borrowed(s); + let mut changed = matches!(p, Object::Ref(_)); + // Its numbers and flags kept, resolved; anything else is no filter's parameters (null). + let one = |e: &Object, changed: &mut bool| -> Object { + *changed |= matches!(e, Object::Ref(_)); + let Object::Dict(d) = &*self.held(e) else { return Object::Null }; + let mut made = Dict::new(); + for key in DECODE_PARMS_KEYS { + let Some(v) = d.get(key) else { continue }; + *changed |= matches!(v, Object::Ref(_)); + let v = self.held(v); + if matches!(*v, Object::Int(_) | Object::Real(_) | Object::Bool(_)) { + made.set(key.to_vec(), (*v).clone()); } - Object::Array(used.iter().map(|e| (*self.resolve(e)).clone()).collect()) } - other if whole => other.clone(), - _ => return Cow::Borrowed(s), + if let Some(g @ Object::Ref(_)) = d.get(b"JBIG2Globals") { + made.set(b"JBIG2Globals".to_vec(), g.clone()); + } + Object::Dict(made) }; + let made = match &*self.held(p) { + Object::Array(a) => Object::Array(a.get(..filters.min(a.len())).unwrap_or_default().iter().map(|e| one(e, &mut changed)).collect()), + _ => one(p, &mut changed), + }; + if !changed { + return Cow::Borrowed(s); + } let mut s = s.clone(); - s.dict.set(b"DecodeParms".to_vec(), direct); + s.dict.set(b"DecodeParms".to_vec(), made); Cow::Owned(s) } + /// `o`, or the object it refers to, without copying either. + fn held<'o>(&self, o: &'o Object) -> Held<'o> { + match o { + Object::Ref(_) => Held::Resolved(self.resolve(o)), + o => Held::Direct(o), + } + } + /// Resolve and return a dictionary (a stream's dictionary counts). pub fn dict(&self, o: &Object) -> Option { self.resolve(o).as_dict().cloned() @@ -603,3 +627,40 @@ impl Document { matches!(self.try_get(r.num).as_deref(), Ok(Object::Dict(d)) if d.name(b"Type") == Some(b"Catalog") || d.contains(b"Pages")) } } + +/// The `/DecodeParms` entries the decoders read (ISO 32000-2 Tables 8, 11 and 13: the Flate and +/// LZW predictors, CCITT, DCT's colour transform): numbers and flags a file may give by +/// reference, which [`Dict::int`] doesn't follow. +const DECODE_PARMS_KEYS: [&[u8]; 13] = [ + b"Predictor", + b"Colors", + b"BitsPerComponent", + b"Columns", + b"EarlyChange", + b"K", + b"Rows", + b"EndOfLine", + b"EncodedByteAlign", + b"EndOfBlock", + b"BlackIs1", + b"DamagedRowsBeforeError", + b"ColorTransform", +]; + +/// An object read in place, or the object a reference names as the document holds it: either way +/// nothing is copied ([`Document::held`]). +enum Held<'o> { + Direct(&'o Object), + Resolved(Arc), +} + +impl std::ops::Deref for Held<'_> { + type Target = Object; + + fn deref(&self) -> &Object { + match self { + Held::Direct(o) => o, + Held::Resolved(o) => o, + } + } +} diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 61a71e24d8..edbd68f562 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1,7 +1,7 @@ use super::*; -/// /DecodeParms given by reference, whole or as array entries, are read once made direct: here a -/// Flate predictor, which the stream alone doesn't see. Only the entries the filters use are +/// /DecodeParms given by reference, whole, as array entries or as the numbers inside, are read +/// once made direct: here a Flate predictor, which the stream alone doesn't see. Only the entries the filters use are /// resolved, and nothing is for a chain longer than a decoder takes. #[test] fn decode_parms_given_by_reference_are_read_once_direct() { @@ -25,6 +25,30 @@ fn decode_parms_given_by_reference_are_read_once_direct() { assert_ne!(s.decoded().unwrap(), rows, "{p:?} read as it is"); assert_eq!(doc.with_direct_parms(&s).decoded().unwrap(), rows, "{p:?}"); } + // A number inside given by reference, which `Dict::int` doesn't follow: in the dictionary + // itself, and in an array's entry. + let twelve = doc.add(Object::Int(12)); + let mut inner = parms.clone(); + inner.set(b"Predictor".to_vec(), Object::Ref(twelve)); + for p in [Object::Dict(inner.clone()), Object::Array(vec![Object::Dict(inner)])] { + let s = stream(1, p.clone()); + assert_ne!(s.decoded().unwrap(), rows, "{p:?} read as it is"); + assert_eq!(doc.with_direct_parms(&s).decoded().unwrap(), rows, "{p:?}"); + } + // Only numbers and flags are copied: a key naming a big array, or an entry that is no + // dictionary, is left out (a file could name one big object from every key and filter). + let big = doc.add(Object::Array(vec![Object::Int(1); 100_000])); + let mut hostile = Dict::new(); + hostile.set(b"Predictor".to_vec(), Object::Ref(big)); + hostile.set(b"Columns".to_vec(), Object::Ref(big)); + hostile.set(b"Colors".to_vec(), Object::Ref(twelve)); + let entry = doc.add(Object::Dict(hostile)); + let two = stream(2, Object::Array(vec![Object::Ref(entry), Object::Ref(big)])); + let made = doc.with_direct_parms(&two); + let made = made.dict.get(b"DecodeParms").and_then(Object::as_array).unwrap(); + let first = made[0].as_dict().unwrap(); + assert_eq!((first.contains(b"Predictor"), first.contains(b"Columns"), first.int(b"Colors")), (false, false, Some(12))); + assert!(matches!(made[1], Object::Null), "{:?}", made[1]); let direct = stream(1, Object::Dict(parms)); assert!(matches!(doc.with_direct_parms(&direct), Cow::Borrowed(_)), "nothing to resolve"); assert_eq!(direct.decoded().unwrap(), rows); diff --git a/crates/optimize/README.md b/crates/optimize/README.md index 4098adc3b0..3bb09038c6 100644 --- a/crates/optimize/README.md +++ b/crates/optimize/README.md @@ -32,8 +32,8 @@ let report = optimize_with_progress(&mut doc, &settings, &mut |stage| keep_going its colour transform (an Adobe marker's, else `/ColorTransform`, ISO 32000-2 §7.4.8), so one stored untransformed keeps its colours; one whose marker and `/ColorTransform` disagree is left alone, since readers differ on which wins. Samples are read with `/DecodeParms` given by - reference too (`Document::with_direct_parms`), here and below: without them a predictor's tag - bytes would shift every row of an image written back. + reference too, the numbers inside included (`Document::with_direct_parms`), here and below: + without them a predictor's tag bytes would shift every row of an image written back. - **Monochrome images** (`Settings::mono`, CCITT Group 4 by default): a 1-bit image with one colour component (grey, indexed, a separation) or an image mask, stored with no filter or with general-purpose ones, is stored again as CCITT Group 4 (`pdfcraft_filters::encode_g4`) when diff --git a/crates/optimize/tests/optimize.rs b/crates/optimize/tests/optimize.rs index 58699fb521..0336d73e9d 100644 --- a/crates/optimize/tests/optimize.rs +++ b/crates/optimize/tests/optimize.rs @@ -413,13 +413,15 @@ fn black_and_white_images_become_ccitt_group_4() { assert!(render(&doc) == before, "the page looks the same"); } -/// An image whose /DecodeParms are a reference is read through them, as one with them direct: a -/// predictor's tag bytes would otherwise shift every row, and the image would be stored again -/// wrong. Here a 1-bit image with a PNG predictor (stored again as Group 4) and an eight-bit grey -/// one with a PNG predictor at 600 ppi (resampled) come out byte for byte as with direct parameters. +/// An image whose /DecodeParms are a reference, or hold numbers given by reference +/// (`/Predictor 9 0 R`, which `Dict::int` doesn't follow), is read through them, as one with them +/// direct: a predictor's tag bytes would otherwise shift every row, and the image would be stored +/// again wrong. Here a 1-bit image with a PNG predictor (stored again as Group 4) and an eight-bit +/// grey one with a PNG predictor at 600 ppi (resampled) come out byte for byte as with direct +/// parameters. #[test] fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() { - let build = |by_reference: bool| { + let build = |how: &str| { let mut doc = Document::new_empty(); let (w, h) = (1200u32, 480u32); let grey = [("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("DeviceGray"))]; @@ -440,13 +442,22 @@ fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() { d.set(b"DecodeParms".to_vec(), Object::Dict(parms)); let coded = pdfcraft_filters::encode(&pdfcraft_filters::Filter::Flate, ¶ms, &shades).unwrap(); let eight_bit = doc.add(Object::Stream(Stream::from_raw(d, coded))); - if by_reference { - for r in [one_bit, eight_bit] { - let mut s = stream(&doc, r); - let parms = s.dict.get(b"DecodeParms").cloned().unwrap(); - s.dict.set(b"DecodeParms".to_vec(), Object::Ref(doc.add(parms))); - doc.set(r, Object::Stream(s)); - } + for r in [one_bit, eight_bit] { + let mut s = stream(&doc, r); + let Some(Object::Dict(mut parms)) = s.dict.get(b"DecodeParms").cloned() else { panic!("{:?}", s.dict) }; + let made = match how { + "whole" => Object::Ref(doc.add(Object::Dict(parms))), + "numbers" => { + for k in [&b"Predictor"[..], b"Columns"] { + let v = parms.get(k).cloned().unwrap(); + parms.set(k.to_vec(), Object::Ref(doc.add(v))); + } + Object::Dict(parms) + } + _ => continue, + }; + s.dict.set(b"DecodeParms".to_vec(), made); + doc.set(r, Object::Stream(s)); } // 300 × 120 pt is 288 ppi; 144 × 57.6 pt is 600 ppi. page(&mut doc, &[("A", one_bit), ("B", eight_bit)], "q 300 0 0 120 20 600 cm /A Do Q q 144 0 0 57.6 20 400 cm /B Do Q"); @@ -458,7 +469,9 @@ fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() { assert_eq!(after[0].dict.name(b"Filter"), Some(&b"CCITTFaxDecode"[..])); after.map(|s| s.raw) }; - assert_eq!(build(true), build(false)); + let direct = build("direct"); + assert_eq!(build("whole"), direct, "the dictionary by reference"); + assert_eq!(build("numbers"), direct, "the numbers inside by reference"); } #[test]