fix(render): draw each ICC profile through itself when two have the same stream dictionary (vendored hayro-interpret patch 40)

hayro-interpret cached the colour space built from an /ICCBased profile
stream under the hash of the stream's dictionary bytes alone. Two
different profiles with the same /N and /Length (any two uncompressed
RGB profiles of one size) shared whichever was read first, so colours in
the second space were drawn through the first profile for the rest of
the document. Found while fixing #1201.

The space is now keyed by what its profile is read from (IccProfileKey):
the stream object a space names, looked up without reading the stream,
and on a miss the old dictionary key plus the stream's own bytes
(decrypted, not decoded), so byte-for-byte copies in other objects still
share one parsed profile and a lookup never decodes. A stream written
directly in the colour space array reports the object holding the array,
as every other stream written there does; it is recognised by its
dictionary lying within the array's bytes and keyed by content only.

InterpreterCache::icc_profile_reads counts the profiles read, so the
tests can check that the cache still hits. Tests: two synthetic RGB
profiles of one length (linear sRGB, and red and blue swapped) as
objects, a reused object, a copy and direct streams on one page.

crates/render/src/raster.rs

@@ -6172,6 +6172,9 @@ endstream endobj
         assert_eq!(decodes("B"), 5, "a JPX soft mask decodes for the size it is drawn at");
     }
 
+    // ICC profiles whose stream dictionaries are the same (vendored hayro-interpret patch 40).
+    mod icc_profile_cache;
+
     /// Render `pdf`'s first page at 72 dpi with no time limit beyond the test's own.
     fn render_first_page(pdf: Vec<u8>) -> RenderedPage {
         let mut r = PageRenderer::new(Arc::new(pdf), RenderConfig::default());

crates/render/src/raster/tests/icc_profile_cache.rs

@@ -0,0 +1,160 @@
+//! Two `/ICCBased` profiles whose stream dictionaries are byte for byte the same (`/N 3` and one
+//! `/Length`) each draw through their own profile (vendored hayro-interpret patch 40). hayro
+//! cached the colour space it read from a profile stream by the dictionary's bytes alone, so
+//! the second profile drew through whichever was read first.
+//!
+//! The profiles are synthetic ones built here: a linear profile with the sRGB colorants, and the
+//! same profile with red and blue swapped, which draws red as blue. They have the same length.
+
+use super::{image_cache_pdf, render_first_page, rgb_at};
+
+/// The width in pixels of the 130 × 70 pt page `image_cache_pdf` makes, rendered at 72 dpi.
+const WIDTH: usize = 130;
+
+const RED: [u8; 3] = [255, 0, 0];
+const BLUE: [u8; 3] = [0, 0, 255];
+
+/// An s15Fixed16Number (ICC.1 §4.6).
+fn s15f16(value: f64) -> [u8; 4] {
+    ((value * 65536.0).round() as i32).to_be_bytes()
+}
+
+/// sRGB's red, green and blue colorants, adapted to D50 as sRGB profiles record them.
+const SRGB_COLORANTS: [[f64; 3]; 3] = [[0.4361, 0.2225, 0.0139], [0.3851, 0.7169, 0.0971], [0.1431, 0.0606, 0.7141]];
+
+/// An ICC v2 RGB display profile (ICC.1:2001-04) with linear tone curves, whose red, green and
+/// blue take the sRGB colorants `primaries` names: `[0, 1, 2]` is linear sRGB, and `[2, 1, 0]`
+/// draws red as blue. The device model is left blank: hayro takes a profile whose model reads
+/// `sRGB` for standard sRGB and skips its transform. Every profile is 300 bytes long.
+fn rgb_profile(primaries: [usize; 3]) -> Vec<u8> {
+    let mut tags: Vec<([u8; 4], Vec<u8>)> = Vec::new();
+    for (signature, primary) in [*b"rXYZ", *b"gXYZ", *b"bXYZ"].into_iter().zip(primaries) {
+        let [x, y, z] = SRGB_COLORANTS[primary];
+        tags.push((signature, [b"XYZ ".as_slice(), &[0; 4], &s15f16(x), &s15f16(y), &s15f16(z)].concat()));
+    }
+    for signature in [*b"rTRC", *b"gTRC", *b"bTRC"] {
+        // A `curveType` with no entries: the identity.
+        tags.push((signature, [b"curv".as_slice(), &[0; 8]].concat()));
+    }
+    let start = 128 + 4 + 12 * tags.len();
+    let (mut table, mut data) = (Vec::new(), Vec::new());
+    for (signature, tag) in &tags {
+        table.extend_from_slice(signature);
+        table.extend_from_slice(&u32::try_from(start + data.len()).unwrap().to_be_bytes());
+        table.extend_from_slice(&u32::try_from(tag.len()).unwrap().to_be_bytes());
+        data.extend_from_slice(tag);
+    }
+    let mut profile = vec![0; 128];
+    profile[0..4].copy_from_slice(&u32::try_from(start + data.len()).unwrap().to_be_bytes());
+    profile[8..12].copy_from_slice(&[2, 0x10, 0, 0]);
+    profile[12..16].copy_from_slice(b"mntr");
+    profile[16..20].copy_from_slice(b"RGB ");
+    profile[20..24].copy_from_slice(b"XYZ ");
+    profile[36..40].copy_from_slice(b"acsp");
+    // The illuminant of the profile connection space, D50.
+    for (i, value) in [0.9642, 1.0, 0.8249].into_iter().enumerate() {
+        profile[68 + 4 * i..72 + 4 * i].copy_from_slice(&s15f16(value));
+    }
+    profile.extend_from_slice(&u32::try_from(tags.len()).unwrap().to_be_bytes());
+    profile.extend(table);
+    profile.extend(data);
+    profile
+}
+
+/// An `/ICCBased` space whose profile stream is written directly in the space's array, not as
+/// an object of its own (hayro reads such a stream when its data is plain tokens): the profile
+/// as ASCIIHex digits, each a token of its own, in upper case (a lone `f` reads as a broken
+/// `false`, and the page's dictionary with it). Its dictionary reports the object holding the
+/// array, which is the page for every one of them.
+fn direct_space(profile: &[u8]) -> String {
+    let hex: String = profile.iter().flat_map(|b| [b >> 4, b & 0xf]).map(|d| format!("{d:X} ")).collect();
+    format!("[/ICCBased << /N 3 /Filter /ASCIIHexDecode /Length {} >> stream\n{hex}\nendstream]", hex.len())
+}
+
+/// A page that fills six 20 pt squares, side by side, with `1 0 0 sc` (red) in the spaces `/A`
+/// (object 5, linear sRGB), `/B` (object 6, red and blue swapped), `/A2` (object 5 again,
+/// through an array of other bytes, so the profile is looked up again), `/A3` (object 7, a copy
+/// of object 5), `/C` and `/D` (the two profiles again, each a stream written directly in its
+/// array). Objects 5, 6 and 7 have the same dictionary, and so do the two direct streams.
+fn page() -> Vec<u8> {
+    let (srgb, swapped) = (rgb_profile([0, 1, 2]), rgb_profile([2, 1, 0]));
+    assert_eq!(srgb.len(), swapped.len(), "the profiles have the same length, so the same dictionary");
+    assert_ne!(srgb, swapped);
+    let dict = format!("<< /N 3 /Length {} >>", srgb.len());
+    let resources = format!(
+        "<< /ColorSpace << /A [/ICCBased 5 0 R] /B [/ICCBased 6 0 R] /A2 [ /ICCBased 5 0 R ] /A3 [/ICCBased 7 0 R] /C {} /D {} >> >>",
+        direct_space(&srgb),
+        direct_space(&swapped)
+    );
+    let content = ["A", "B", "A2", "A3", "C", "D"]
+        .iter()
+        .enumerate()
+        .map(|(i, name)| format!("/{name} cs 1 0 0 sc {} 20 20 30 re f", 2 + 21 * i))
+        .collect::<Vec<_>>()
+        .join("\n");
+    image_cache_pdf(&content, &resources, &[(&dict, &srgb), (&dict, &swapped), (&dict, &srgb)])
+}
+
+#[track_caller]
+fn assert_near(got: [u8; 3], want: [u8; 3], what: &str) {
+    assert!(got.iter().zip(want).all(|(got, want)| got.abs_diff(want) <= 6), "{what}: got {got:?}, expected about {want:?}");
+}
+
+/// Each square is drawn through its own profile: red through linear sRGB, blue through the
+/// swapped one, whether the profile is an object (5 and 6) or written in its array (`/C` and
+/// `/D`), and object 5 looked up again and its copy (object 7) still draw red.
+#[test]
+fn icc_profiles_with_identical_dictionaries_draw_their_own_colours() {
+    let rendered = render_first_page(page());
+    assert_eq!((rendered.width as usize, rendered.height), (WIDTH, 70));
+    let expected = [
+        ("/A, object 5, linear sRGB", RED),
+        ("/B, object 6, red and blue swapped", BLUE),
+        ("/A2, object 5 again", RED),
+        ("/A3, object 7, a copy of object 5", RED),
+        ("/C, linear sRGB written in its array", RED),
+        ("/D, red and blue swapped written in its array", BLUE),
+    ];
+    for (i, (what, want)) in expected.into_iter().enumerate() {
+        assert_near(rgb_at(&rendered.rgba, WIDTH, 12 + 21 * i, 35), want, what);
+    }
+}
+
+/// The profiles are still cached: the four different ones (objects 5 and 6, and the two written
+/// in their arrays) are read once each, and neither object 5 looked up again nor its copy, object
+/// 7, is read again. A second render through the same cache reads none.
+#[test]
+fn icc_profiles_used_again_are_read_once() {
+    use hayro::hayro_interpret::font::Glyph;
+    use hayro::hayro_interpret::hayro_syntax::Pdf;
+    use hayro::hayro_interpret::{
+        BlendMode, ClipPath, Context, Device, GlyphDrawMode, Image, InterpreterCache, InterpreterSettings, Paint, PathDrawMode, SoftMask,
+        TransformExt, interpret_page,
+    };
+    use kurbo::{Affine, BezPath, Rect};
+    use std::sync::Arc;
+
+    /// Draws nothing: only the colour spaces the page selects matter here.
+    struct Nothing;
+    impl<'a> Device<'a> for Nothing {
+        fn set_soft_mask(&mut self, _: Option<SoftMask<'a>>) {}
+        fn set_blend_mode(&mut self, _: BlendMode) {}
+        fn draw_path(&mut self, _: &BezPath, _: Affine, _: &Paint<'a>, _: &PathDrawMode) {}
+        fn push_clip_path(&mut self, _: &ClipPath) {}
+        fn push_transparency_group(&mut self, _: f32, _: Option<SoftMask<'a>>, _: BlendMode) {}
+        fn draw_glyph(&mut self, _: &Glyph<'a>, _: Affine, _: Affine, _: &Paint<'a>, _: &GlyphDrawMode) {}
+        fn draw_image(&mut self, _: Image<'a, '_>, _: Affine) {}
+        fn pop_clip_path(&mut self) {}
+        fn pop_transparency_group(&mut self) {}
+    }
+
+    let pdf = Pdf::new(Arc::new(page())).expect("parses");
+    let page = &pdf.pages()[0];
+    let cache = InterpreterCache::new();
+    for render in ["first", "second"] {
+        let initial = page.initial_transform(true).to_kurbo();
+        let mut ctx = Context::new(initial, Rect::new(0.0, 0.0, 130.0, 70.0), &cache, page.xref(), InterpreterSettings::default());
+        interpret_page(page, &mut ctx, &mut Nothing);
+        assert_eq!(cache.icc_profile_reads(), 4, "after the {render} render: each different profile is read once");
+    }
+}

vendor/hayro-interpret/src/cache.rs

@@ -4,11 +4,14 @@ use kurbo::Affine;
 use std::any::Any;
 use std::collections::HashMap;
 use std::collections::hash_map::Entry;
+use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};
 
 type CacheMap = HashMap<u128, Option<Box<dyn Any + Send + Sync>>>;
+/// The objects read from a document, by key. PdfCraft patch (40): the second field counts the
+/// ICC profiles read through it ([`Cache::icc_profile_reads`]).
 #[derive(Clone)]
-pub(crate) struct Cache(Arc<Mutex<CacheMap>>);
+pub(crate) struct Cache(Arc<Mutex<CacheMap>>, Arc<AtomicUsize>);
 impl Default for Cache {
     fn default() -> Self {
         Self::new()
@@ -17,7 +20,18 @@ impl Default for Cache {
 
 impl Cache {
     pub(crate) fn new() -> Self {
-        Self(Arc::new(Mutex::new(HashMap::new())))
+        Self(Arc::new(Mutex::new(HashMap::new())), Arc::default())
+    }
+
+    /// PdfCraft patch (40): count one ICC profile read, the colour space of an `/ICCBased`
+    /// profile no entry held yet being built from it.
+    pub(crate) fn count_icc_profile_read(&self) {
+        self.1.fetch_add(1, Ordering::Relaxed);
+    }
+
+    /// PdfCraft patch (40): how many ICC profiles were read through this cache.
+    pub(crate) fn icc_profile_reads(&self) -> usize {
+        self.1.load(Ordering::Relaxed)
     }
 
     pub(crate) fn get_or_insert_with<T: Clone + Send + Sync + 'static>(

vendor/hayro-interpret/src/color.rs

@@ -2,6 +2,7 @@
 
 use crate::cache::{Cache, CacheKey};
 use crate::function::Function;
+use crate::util::hash128;
 use hayro_syntax::object;
 use hayro_syntax::object::Array;
 use hayro_syntax::object::Dict;
@@ -100,6 +101,34 @@ pub(crate) enum ColorSpaceType {
     DeviceN(DeviceN),
 }
 
+/// PdfCraft patch (40): what the colour space read from an `/ICCBased` profile stream is cached
+/// by. hayro keyed it by the stream dictionary's bytes alone, so two different profiles with the
+/// same `/N` and `/Length` shared whichever was read first, and colours in the second were
+/// drawn through the first.
+#[derive(Hash)]
+enum IccProfileKey<'a> {
+    /// The stream object a space names, looked up without reading the stream: an object holds
+    /// one profile for as long as the document, and so the cache, lives.
+    Object(object::ObjectIdentifier),
+    /// What the space is built from: the stream dictionary's key (its bytes, the old key:
+    /// `/N`, `/Alternate`, the filters) and the stream's own bytes, decrypted but not decoded,
+    /// which decode to the profile. Copies of a profile share it, and a lookup never decodes:
+    /// decoding waits for a miss.
+    Content { dict: u128, data: &'a [u8] },
+}
+
+/// PdfCraft patch (40): the object of the `/ICCBased` profile `stream` that `array` names, or
+/// `None` for a stream written directly in the array (its dictionary lies within the array's
+/// bytes). Such a stream has no object of its own: its dictionary reports the object that
+/// holds the array, which every other stream written there reports too.
+fn icc_stream_object(array: &Array<'_>, stream: &Stream<'_>) -> Option<object::ObjectIdentifier> {
+    let dict = stream.dict();
+    if array.data().as_ptr_range().contains(&dict.data().as_ptr()) {
+        return None;
+    }
+    dict.obj_id()
+}
+
 impl ColorSpaceType {
     fn new(object: Object<'_>, cache: &Cache) -> Option<Self> {
         Self::new_inner(object, cache)
@@ -118,34 +147,50 @@ impl ColorSpaceType {
                     let dict = icc_stream.dict();
                     let num_components = dict.get::<usize>(N)?;
 
-                    return cache.get_or_insert_with(icc_stream.cache_key(), || {
-                        if let Some(decoded) = icc_stream.decoded().ok().as_ref() {
-                            ICCProfile::new(decoded, num_components)
-                                .map(|icc| {
-                                    // TODO: For SVG and PNG we can assume that the output color space is
-                                    // sRGB. If we ever implement PDF-to-PDF, we probably want to
-                                    // let the user pass the native color type and don't make this optimization
-                                    // if it's not sRGB.
-                                    if icc.is_srgb() {
-                                        Self::DeviceRgb
-                                    } else {
-                                        Self::ICCBased(icc)
-                                    }
-                                })
-                                .or_else(|| {
-                                    dict.get::<Object<'_>>(ALTERNATE)
-                                        .and_then(|o| Self::new(o, cache))
-                                })
-                                .or_else(|| match dict.get::<u8>(N) {
-                                    Some(1) => Some(Self::DeviceGray),
-                                    Some(3) => Some(Self::DeviceRgb),
-                                    Some(4) => Some(Self::DeviceCmyk),
-                                    _ => None,
-                                })
-                        } else {
-                            None
+                    // PdfCraft patch (40): cached by what the profile is read from (see
+                    // `IccProfileKey`), not by the stream dictionary's bytes alone.
+                    let read = || {
+                        let data = icc_stream.raw_data();
+                        let content = IccProfileKey::Content {
+                            dict: icc_stream.cache_key(),
+                            data: &data,
+                        };
+                        cache.get_or_insert_with(hash128(&content), || {
+                            cache.count_icc_profile_read();
+                            if let Some(decoded) = icc_stream.decoded().ok().as_ref() {
+                                ICCProfile::new(decoded, num_components)
+                                    .map(|icc| {
+                                        // TODO: For SVG and PNG we can assume that the output color space is
+                                        // sRGB. If we ever implement PDF-to-PDF, we probably want to
+                                        // let the user pass the native color type and don't make this optimization
+                                        // if it's not sRGB.
+                                        if icc.is_srgb() {
+                                            Self::DeviceRgb
+                                        } else {
+                                            Self::ICCBased(icc)
+                                        }
+                                    })
+                                    .or_else(|| {
+                                        dict.get::<Object<'_>>(ALTERNATE)
+                                            .and_then(|o| Self::new(o, cache))
+                                    })
+                                    .or_else(|| match dict.get::<u8>(N) {
+                                        Some(1) => Some(Self::DeviceGray),
+                                        Some(3) => Some(Self::DeviceRgb),
+                                        Some(4) => Some(Self::DeviceCmyk),
+                                        _ => None,
+                                    })
+                            } else {
+                                None
+                            }
+                        })
+                    };
+                    return match icc_stream_object(&color_array, &icc_stream) {
+                        Some(object) => {
+                            cache.get_or_insert_with(hash128(&IccProfileKey::Object(object)), read)
                         }
-                    });
+                        None => read(),
+                    };
                 }
                 CALCMYK => return Some(Self::DeviceCmyk),
                 CALGRAY => {

vendor/hayro-interpret/src/context.rs

@@ -146,6 +146,13 @@ impl<'a> InterpreterCache<'a> {
     pub fn decoded_image_decodes(&self) -> usize {
         self.decoded_cache.decodes()
     }
+
+    /// PdfCraft patch (40): how many `/ICCBased` profiles were read through this cache, for
+    /// diagnostics and tests. A profile used again, by the same stream object or by a copy with
+    /// the same dictionary and bytes, is read once.
+    pub fn icc_profile_reads(&self) -> usize {
+        self.object_cache.icc_profile_reads()
+    }
 }
 
 /// A per-page interpretation context that borrows shared data from an [`InterpreterCache`].