M11: the optimizer reads images through /DecodeParms given by reference

Stream::decoded reads only direct parameter dictionaries, so an image
whose /DecodeParms is a reference (whole, or as array entries) was
decoded without its predictor. For a 1-bit image with a PNG predictor
the rows came out shifted by their tag bytes, Group 4 of that was still
smaller, and the picture was replaced by a skewed one; resampling and
merging image strips read the same way.

pdfcraft_cos::Document::with_direct_parms gives the stream with the
parameters its filters use made direct: only the entries at the
filters' places, and none for a chain longer than any decoder takes,
so at most MAX_CHAIN objects are copied. The optimizer's pixels() and
the Group 4 step decode through it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BfMGHQNgdq5yf5tKZHGR3z

crates/cos/README.md

@@ -24,6 +24,7 @@ let obj  = doc.get(ObjRef { num: 12, gen: 0 });      // load (and cache): Arc<Ob
 let real = doc.resolve(&obj);                        // follow one indirect reference
 let d    = doc.dict(&obj);                           // Option<Dict>, resolving first
 let data = stream.decoded()?;                        // filters applied on demand
+let data = doc.with_direct_parms(&stream).decoded()?; // /DecodeParms given by reference too
 
 doc.set(r, Object::Dict(d));                         // edit: lands in the overlay
 let r = doc.add(obj);                                // append a new object

crates/cos/src/document.rs

@@ -5,11 +5,12 @@
 //! Edits never touch the original bytes; the writer appends them (incremental save) or rewrites
 //! the file (full save).
 
+use std::borrow::Cow;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::sync::{Arc, Mutex, Weak};
 
 use crate::CosError;
-use crate::object::{Dict, ObjRef, Object};
+use crate::object::{Dict, ObjRef, Object, Stream};
 use crate::parser::{EndstreamIndex, Lexer, is_whitespace, parse_indirect, parse_indirect_shared};
 
 mod objects;

crates/cos/src/document/objects.rs

@@ -1,7 +1,7 @@
 use super::reader::find;
 use super::state::{LENGTHS, LoadGuard, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, objstm_member, owned_bytes};
 use super::xref::{Revision, XrefEntry};
-use super::{Arc, CosError, Dict, Document, Lexer, ObjRef, Object, is_whitespace, parse_indirect_shared};
+use super::{Arc, CosError, Cow, Dict, Document, Lexer, ObjRef, Object, Stream, is_whitespace, parse_indirect_shared};
 
 impl Document {
     pub fn bytes(&self) -> &Arc<Vec<u8>> {
@@ -158,6 +158,39 @@ 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.
+    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") {
+            Some(Object::Array(a)) => a.len(),
+            Some(_) => 1,
+            None => 0,
+        };
+        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);
+                }
+                Object::Array(used.iter().map(|e| (*self.resolve(e)).clone()).collect())
+            }
+            other if whole => other.clone(),
+            _ => return Cow::Borrowed(s),
+        };
+        let mut s = s.clone();
+        s.dict.set(b"DecodeParms".to_vec(), direct);
+        Cow::Owned(s)
+    }
+
     /// Resolve and return a dictionary (a stream's dictionary counts).
     pub fn dict(&self, o: &Object) -> Option<Dict> {
         self.resolve(o).as_dict().cloned()

crates/cos/src/document/tests.rs

@@ -1,5 +1,41 @@
 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
+/// resolved, and nothing is for a chain longer than a decoder takes.
+#[test]
+fn decode_parms_given_by_reference_are_read_once_direct() {
+    let rows = [1u8, 2, 3, 4, 5, 6, 7, 8];
+    let params = pdfcraft_filters::Params { predictor: 12, colors: 1, bits_per_component: 8, columns: 4, early_change: 1 };
+    let coded = pdfcraft_filters::encode(&pdfcraft_filters::Filter::Flate, &params, &rows).unwrap();
+    let mut parms = Dict::new();
+    parms.set(b"Predictor".to_vec(), Object::Int(12));
+    parms.set(b"Columns".to_vec(), Object::Int(4));
+    let mut doc = Document::new_empty();
+    let r = doc.add(Object::Dict(parms.clone()));
+    let array = doc.add(Object::Array(vec![Object::Ref(r)]));
+    let stream = |filters: usize, p: Object| {
+        let mut d = Dict::new();
+        d.set(b"Filter".to_vec(), Object::Array(vec![Object::name("FlateDecode"); filters]));
+        d.set(b"DecodeParms".to_vec(), p);
+        Stream::from_raw(d, coded.clone())
+    };
+    for p in [Object::Ref(r), Object::Array(vec![Object::Ref(r)]), Object::Ref(array)] {
+        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:?}");
+    }
+    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);
+    // Entries past the filters' are left out, unresolved; so is everything past the chain limit.
+    let many = stream(1, Object::Array(vec![Object::Ref(r); 1000]));
+    let made = doc.with_direct_parms(&many);
+    assert_eq!(made.dict.get(b"DecodeParms").and_then(Object::as_array).map(Vec::len), Some(1));
+    let long = stream(pdfcraft_filters::MAX_CHAIN + 1, Object::Array(vec![Object::Ref(r); 9]));
+    assert!(matches!(doc.with_direct_parms(&long), Cow::Borrowed(_)));
+}
+
 #[test]
 fn a_read_state_matches_a_clone_and_no_changed_document() {
     let mut doc = Document::new_empty();

crates/optimize/README.md

@@ -31,7 +31,9 @@ let report = optimize_with_progress(&mut doc, &settings, &mut |stage| keep_going
   image masks, decode arrays, more than 8 bits, JPEG 2000, JBIG2 and CCITT. A JPEG is read with
   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.
+  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.
 - **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

crates/optimize/src/images.rs

@@ -258,7 +258,9 @@ fn count_decoded(_n: u64) {
     DECODED_PIXELS.with(|c| c.set(c.get().saturating_add(_n)));
 }
 
-/// 8-bit samples of an image (`n` components), decoded. `None` for a JPEG whose colour transform
+/// 8-bit samples of an image (`n` components), decoded, with `/DecodeParms` given by reference
+/// too ([`Document::with_direct_parms`]): read without them, a predictor's tag bytes would shift
+/// every row, and the image would be written back wrong. `None` for a JPEG whose colour transform
 /// readers may disagree about ([`ycc_encoded`]).
 pub(crate) fn pixels(doc: &Document, s: &Stream, n: usize) -> Option<Vec<u8>> {
     let (w, h) = (u32::try_from(s.dict.int(b"Width")?).ok()?, u32::try_from(s.dict.int(b"Height")?).ok()?);
@@ -275,7 +277,7 @@ pub(crate) fn pixels(doc: &Document, s: &Stream, n: usize) -> Option<Vec<u8>> {
         count_decoded(u64::from(w) * u64::from(h));
         (v.len() == bytes).then_some(v)
     } else {
-        let v = s.decoded().ok()?;
+        let v = doc.with_direct_parms(s).decoded().ok()?;
         Some(v.get(..bytes)?.to_vec())
     }
 }
@@ -573,7 +575,8 @@ fn mono(doc: &Document, s: &Stream, compression: MonoCompression) -> Option<Stre
     if w == 0 || h == 0 || w > pdfcraft_filters::MAX_G4_COLUMNS || u64::from(w) * u64::from(h) > MAX_PIXELS {
         return None;
     }
-    let g4 = pdfcraft_filters::encode_g4(&s.decoded().ok()?, w, h).ok()?;
+    // Through its parameters given by reference too, as `pixels` reads.
+    let g4 = pdfcraft_filters::encode_g4(&doc.with_direct_parms(s).decoded().ok()?, w, h).ok()?;
     let mut new = d.clone();
     for k in [&b"Filter"[..], b"DecodeParms", b"Length", b"DL"] {
         new.remove(k);

crates/optimize/tests/optimize.rs

@@ -413,6 +413,54 @@ 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.
+#[test]
+fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() {
+    let build = |by_reference: bool| {
+        let mut doc = Document::new_empty();
+        let (w, h) = (1200u32, 480u32);
+        let grey = [("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("DeviceGray"))];
+        let one_bit = mono_image(&mut doc, w, h, &bilevel(w, h, false), Some(15), &grey);
+        let shades: Vec<u8> = (0..w * h).map(|i| ((i % w) * 255 / w) as u8 ^ ((i / w) as u8 & 0x0F)).collect();
+        let params = pdfcraft_filters::Params { predictor: 15, colors: 1, bits_per_component: 8, columns: w as i64, early_change: 1 };
+        let mut parms = Dict::new();
+        parms.set(b"Predictor".to_vec(), Object::Int(15));
+        parms.set(b"Columns".to_vec(), Object::Int(w as i64));
+        let mut d = Dict::new();
+        d.set(b"Type".to_vec(), Object::name("XObject"));
+        d.set(b"Subtype".to_vec(), Object::name("Image"));
+        d.set(b"Width".to_vec(), Object::Int(w as i64));
+        d.set(b"Height".to_vec(), Object::Int(h as i64));
+        d.set(b"BitsPerComponent".to_vec(), Object::Int(8));
+        d.set(b"ColorSpace".to_vec(), Object::name("DeviceGray"));
+        d.set(b"Filter".to_vec(), Object::name("FlateDecode"));
+        d.set(b"DecodeParms".to_vec(), Object::Dict(parms));
+        let coded = pdfcraft_filters::encode(&pdfcraft_filters::Filter::Flate, &params, &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));
+            }
+        }
+        // 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");
+        let before = [one_bit, eight_bit].map(|r| stream(&doc, r).raw);
+        let r = optimize(&mut doc, &Settings::default()).unwrap();
+        assert_eq!((r.images_resampled, r.images_recompressed), (1, 1), "{r:?}");
+        let after = [one_bit, eight_bit].map(|r| stream(&doc, r));
+        assert!(before.iter().zip(&after).all(|(b, a)| *b != a.raw), "both stored again");
+        assert_eq!(after[0].dict.name(b"Filter"), Some(&b"CCITTFaxDecode"[..]));
+        after.map(|s| s.raw)
+    };
+    assert_eq!(build(true), build(false));
+}
+
 #[test]
 fn settings_choose_what_happens() {
     let mut doc = Document::new_empty();