From 89a5b2cf2eebdc007beba80a9b1c6582359d3c6a Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 07:46:17 +0000 Subject: [PATCH] hayro-interpret: port PdfCraft's ICC profile cache key (#3020) Cache the color space of an /ICCBased profile by its stream object, and on a miss by its dictionary key and raw bytes, instead of the dictionary's bytes alone, so two profiles with the same /N and /Length each draw through their own profile. InterpreterCache::icc_profile_reads counts profile reads. PdfCraft's two raster tests are adapted in tests/upstream_raster.rs. --- crates/pdf/tests/upstream_raster.rs | 125 ++++++++++++++++++++++++-- vendor/README.craft-reader.md | 13 +++ vendor/hayro-interpret/SOURCE.json | 6 +- vendor/hayro-interpret/src/cache.rs | 18 +++- vendor/hayro-interpret/src/color.rs | 98 ++++++++++++++------ vendor/hayro-interpret/src/context.rs | 7 ++ 6 files changed, 226 insertions(+), 41 deletions(-) diff --git a/crates/pdf/tests/upstream_raster.rs b/crates/pdf/tests/upstream_raster.rs index 7c899a73..44170098 100644 --- a/crates/pdf/tests/upstream_raster.rs +++ b/crates/pdf/tests/upstream_raster.rs @@ -662,9 +662,9 @@ mod default_color_spaces { use super::{Pixels, render}; const WIDTH: usize = 130; - const RED: [u8; 3] = [255, 0, 0]; + pub(super) const RED: [u8; 3] = [255, 0, 0]; const GREEN: [u8; 3] = [0, 255, 0]; - const BLUE: [u8; 3] = [0, 0, 255]; + pub(super) const BLUE: [u8; 3] = [0, 0, 255]; const BLACK: [u8; 3] = [0, 0, 0]; /// Device black through `gray_profile`: half the luminance of white, sRGB-encoded. const MID_GRAY: [u8; 3] = [188, 188, 188]; @@ -724,7 +724,7 @@ mod default_color_spaces { /// A linear RGB profile whose red, green and blue take the sRGB colorants `primaries` names: /// with `[1, 2, 0]` red is drawn as sRGB green. - fn rgb_profile(primaries: [usize; 3]) -> Vec { + pub(super) fn rgb_profile(primaries: [usize; 3]) -> Vec { let mut tags = Vec::new(); for (signature, primary) in [*b"rXYZ", *b"gXYZ", *b"bXYZ"].into_iter().zip(primaries) { tags.push((signature, xyz(SRGB_COLORANTS[primary]))); @@ -760,7 +760,7 @@ mod default_color_spaces { icc(b"prtr", b"CMYK", b"Lab ", &[(*b"A2B0", lut)]) } - fn profile_stream(n: u8, profile: Vec) -> (String, Vec) { + pub(super) fn profile_stream(n: u8, profile: Vec) -> (String, Vec) { (format!("<< /N {n} /Length {} >>", profile.len()), profile) } @@ -769,9 +769,9 @@ mod default_color_spaces { (format!("<< /Type /XObject /Subtype /Form /BBox [0 0 130 70] {extra} /Length {} >>", content.len()), content.as_bytes().to_vec()) } - /// Draw `content` with `resources` on a 130 × 70 pt page whose objects 5, 6, … are the - /// streams `objects`, and give the color at each of `points` (from the top left). - fn colors(content: &str, resources: &str, objects: &[(String, Vec)], points: &[(usize, usize)]) -> Vec<[u8; 3]> { + /// A 130 × 70 pt page that draws `content` with `resources`, whose objects 5, 6, … are the + /// streams `objects`. + pub(super) fn page_pdf(content: &str, resources: &str, objects: &[(String, Vec)]) -> Vec { let mut pdf = format!( "%PDF-1.4\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ @@ -786,13 +786,19 @@ mod default_color_spaces { pdf.extend_from_slice(b"\nendstream endobj\n"); } pdf.extend_from_slice(b"trailer << /Root 1 0 R >>\n%%EOF"); - let Pixels { width, height, rgba } = render(&pdf); + pdf + } + + /// Draw `content` with `resources` on a 130 × 70 pt page whose objects 5, 6, … are the + /// streams `objects`, and give the color at each of `points` (from the top left). + pub(super) fn colors(content: &str, resources: &str, objects: &[(String, Vec)], points: &[(usize, usize)]) -> Vec<[u8; 3]> { + let Pixels { width, height, rgba } = render(&page_pdf(content, resources, objects)); assert_eq!((width as usize, height), (WIDTH, 70)); points.iter().map(|&(x, y)| rgba[(y * WIDTH + x) * 4..][..3].try_into().unwrap()).collect() } #[track_caller] - fn assert_near(got: [u8; 3], want: [u8; 3], what: &str) { + pub(super) 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:?}"); } @@ -1044,3 +1050,104 @@ mod default_color_spaces { } } } + +/// PdfCraft ledger hayro-interpret (48) and the ICC profile cache key (#3020): two `/ICCBased` +/// profiles whose stream dictionaries are byte for byte the same (`/N 3` and one `/Length`) +/// each draw through their own profile. hayro cached the color 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 linear sRGB and the same profile with red and blue swapped, +/// which draws red as blue; they have the same length. +mod icc_profile_cache { + use super::default_color_spaces::{BLUE, RED, assert_near, colors, page_pdf, profile_stream, rgb_profile}; + use craft_pdf::interpret::{ + BlendMode, ClipPath, Context, Device, GlyphDrawMode, Image, InterpreterCache, InterpreterSettings, Paint, PathDrawMode, SoftMask, + TransformExt, font::Glyph, + }; + use kurbo::{Affine, BezPath, Rect}; + + /// An `/ICCBased` space whose profile stream is written directly in the space's array, not + /// as an object of its own: the profile as ASCIIHex digits, each a token of its own, in + /// upper case (a lone `f` reads as a broken `false`). 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()) + } + + /// Six 20 pt squares, side by side, filled with `1 0 0 sc` (red) in `/A` (object 5, linear + /// sRGB), `/B` (object 6, red and blue swapped), `/A2` (object 5 again, through an array of + /// other bytes), `/A3` (object 7, a copy of object 5), `/C` and `/D` (the two profiles + /// again, each written directly in its array). Objects 5, 6 and 7 have the same dictionary, + /// and so do the two direct streams. + fn page() -> (String, String, Vec<(String, Vec)>) { + 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 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::>() + .join("\n"); + (content, resources, vec![profile_stream(3, srgb.clone()), profile_stream(3, swapped), profile_stream(3, srgb)]) + } + + /// Each square is drawn through its own profile, whether the profile is an object or + /// written in its array, and object 5 looked up again and its copy still draw red. + #[cfg_attr(not(target_arch = "wasm32"), test)] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] + fn icc_profiles_with_identical_dictionaries_draw_their_own_colors() { + let (content, resources, objects) = page(); + let points: Vec<_> = (0..6).map(|i| (12 + 21 * i, 35)).collect(); + let got = colors(&content, &resources, &objects, &points); + 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 (got, (what, want)) in got.into_iter().zip(expected) { + assert_near(got, want, what); + } + } + + /// Draws nothing: only the color spaces the page selects matter here. + struct Nothing; + impl<'a> Device<'a> for Nothing { + fn set_soft_mask(&mut self, _: Option>) {} + 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>, _: 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) {} + } + + /// 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 pass through the same cache reads none. + #[cfg_attr(not(target_arch = "wasm32"), test)] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] + fn icc_profiles_used_again_are_read_once() { + let (content, resources, objects) = page(); + let bytes = page_pdf(&content, &resources, &objects); + let pdf = craft_pdf::open_slice(&bytes, None).unwrap(); + let page = &pdf.pages()[0]; + let cache = InterpreterCache::new(); + for pass 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()); + craft_pdf::interpret_page(page, &mut ctx, &mut Nothing).unwrap(); + assert_eq!(cache.icc_profile_reads(), 4, "after the {pass} pass: each different profile is read once"); + } + } +} diff --git a/vendor/README.craft-reader.md b/vendor/README.craft-reader.md index 5058fbca..436295fb 100644 --- a/vendor/README.craft-reader.md +++ b/vendor/README.craft-reader.md @@ -280,6 +280,19 @@ returns before the extent is used, so it still clears under the whole path. Test `mesh_extent_tests::{a_mesh_paints_its_triangles_alone, a_mesh_keeps_the_graphics_state_alpha, a_mesh_off_the_canvas_paints_nothing}` in hayro. +An ICC profile cache fix from the [PdfCraft revision 0b670caf4a090e9d83a7f41a84b4cb2144a2d33e ledger](https://github.com/storytold/pdfcraft/blob/0b670caf4a090e9d83a7f41a84b4cb2144a2d33e/vendor/README.md): +hayro-interpret (48) (PdfCraft #3020), marked `PdfCraft patch (#3020)`. The color space read +from an `/ICCBased` profile stream is cached by `IccProfileKey`, not by the stream +dictionary's bytes alone, so two profiles with the same `/N` and `/Length` no longer share +whichever was read first. A stream object is looked up by its object identifier without +reading it, and on a miss by its dictionary key and raw (decrypted, undecoded) bytes, so +copies of a profile are read once; a stream written directly in its array has only the +content key. `InterpreterCache::icc_profile_reads` counts the reads. The CMYK simulation of +patch 36 doesn't use this cache: it is built from the output intent's bytes by the caller, +and the digest that tells its profile from a document profile is still a hash of the +decoded profile bytes. PdfCraft's two raster tests are adapted in `tests/upstream_raster.rs` +(`icc_profile_cache`). + This remains the patched 0.7 backend. It retains the existing Bradford Lab conversion, floating component precision and codec versions, rather than claiming full 0.8 equivalence. The independent color oracles, backend tests and facade contracts cover diff --git a/vendor/hayro-interpret/SOURCE.json b/vendor/hayro-interpret/SOURCE.json index 5b2626fb..4b565798 100644 --- a/vendor/hayro-interpret/SOURCE.json +++ b/vendor/hayro-interpret/SOURCE.json @@ -176,19 +176,19 @@ "path": "src/cache.rs", "source_sha256": "76b5dc066330666a50a32124856306b72c98e646346263bd8a269bc99e5a0d42", "source_git_blob": "e253a7b8c7dbfbfa2f7dc1f6c582559962060b94", - "sha256": "137d210732f88f45716e72cd5be654dc25ce83b39dcec697a5715f98831781bd" + "sha256": "c63ed890ead39cc6205746063f90689b7a617e8c14a35e6ee1b955550d1230a5" }, { "path": "src/color.rs", "source_sha256": "799ce29bb5e59c8fc94db202873e48c2047dd35962e6f26efd215156c47158c1", "source_git_blob": "56bb04f24c04ba33ab6bf99c02a24735670959ad", - "sha256": "190013651fdeb71cb16345e6766e29ce437a682d1efd265ea3e7a567a8eac555" + "sha256": "85391b386b0c34b6bb1d6757c3d083ca07764062d085e0324456a377d6d520fc" }, { "path": "src/context.rs", "source_sha256": "dc2be553958ee3f5e5e644a77a0301723bae9ca027a352c13b9caaf549cc0e56", "source_git_blob": "d6772baa15ebb95d3ba2d1f54ce5c250ad1e8137", - "sha256": "e6b4dd369ea6b9c3b60374e46b775511a2542cd46d9da31957381a7361528d82" + "sha256": "98ed66aab25f1e3b8b9dfd9b6837dc0d9519a9fd7f5429b695865879abc24f6d" }, { "path": "src/convert.rs", diff --git a/vendor/hayro-interpret/src/cache.rs b/vendor/hayro-interpret/src/cache.rs index 7914ce32..d2be7543 100644 --- a/vendor/hayro-interpret/src/cache.rs +++ b/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>>; +/// The objects read from a document, by key. PdfCraft patch (#3020): the second field counts +/// the ICC profiles read through it ([`Cache::icc_profile_reads`]). #[derive(Clone)] -pub(crate) struct Cache(Arc>); +pub(crate) struct Cache(Arc>, Arc); 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 (#3020): count one ICC profile read, the color 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 (#3020): 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( diff --git a/vendor/hayro-interpret/src/color.rs b/vendor/hayro-interpret/src/color.rs index 3123974f..e85fd17e 100644 --- a/vendor/hayro-interpret/src/color.rs +++ b/vendor/hayro-interpret/src/color.rs @@ -124,6 +124,34 @@ pub(crate) enum ColorSpaceType { Plate(PlateSpace), } +/// PdfCraft patch (#3020): what the color 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 colors 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 (#3020): 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 { + 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::new_inner(object, cache) @@ -142,34 +170,50 @@ impl ColorSpaceType { let dict = icc_stream.dict(); let num_components = dict.get::(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::>(ALTERNATE) - .and_then(|o| Self::new(o, cache)) - }) - .or_else(|| match dict.get::(N) { - Some(1) => Some(Self::DeviceGray), - Some(3) => Some(Self::DeviceRgb), - Some(4) => Some(Self::DeviceCmyk), - _ => None, - }) - } else { - None + // PdfCraft patch (#3020): 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::>(ALTERNATE) + .and_then(|o| Self::new(o, cache)) + }) + .or_else(|| match dict.get::(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 => { diff --git a/vendor/hayro-interpret/src/context.rs b/vendor/hayro-interpret/src/context.rs index 99d80e5a..e4d84130 100644 --- a/vendor/hayro-interpret/src/context.rs +++ b/vendor/hayro-interpret/src/context.rs @@ -156,6 +156,13 @@ impl<'a> InterpreterCache<'a> { pub fn decoded_image_decodes(&self) -> usize { self.decoded_cache.decodes() } + + /// PdfCraft patch (#3020): 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`].