address review: skip the image cache for resource-dependent names inside colour-space arrays

This commit is contained in:
Dylan M. Taylor
2026-11-10 22:57:28 +00:00
parent d8e46a0c63
commit 6bb41054e7
3 changed files with 52 additions and 5 deletions
+22
View File
@@ -94,6 +94,28 @@ pub fn color_space(file: &File, o: &Obj, res: Option<&Dict>) -> Cs {
color_space_at(file, o, res, 0)
}
/// Whether decoding colour space `o` looks a name up in the page resources: a name that is not a
/// device space, anywhere in the definition (an Indexed base, a Separation or DeviceN alternate, an
/// ICCBased alternate). The same image then decodes differently under different resources.
pub fn names_resource(file: &File, o: &Obj, depth: usize) -> bool {
if depth >= MAX_DEPTH {
return true;
}
match file.resolve(o) {
Obj::Name(n) => !matches!(n.as_str(), "DeviceGray" | "DeviceRGB" | "DeviceCMYK" | "G" | "RGB" | "CMYK"),
Obj::Array(a) => {
let inner = match a.first().map(|h| file.resolve(h)).and_then(Obj::name).unwrap_or("") {
"Indexed" | "I" => a.get(1).cloned(),
"Separation" | "DeviceN" => a.get(2).cloned(),
"ICCBased" => a.get(1).map(|s| file.resolve(s)).and_then(Obj::dict).and_then(|d| file.get(d, "Alternate")).cloned(),
_ => None,
};
inner.is_some_and(|i| names_resource(file, &i, depth + 1))
}
_ => false,
}
}
/// [`color_space`] nested `depth` levels deep: DeviceGray past [`MAX_DEPTH`].
fn color_space_at(file: &File, o: &Obj, res: Option<&Dict>, depth: usize) -> Cs {
if depth >= MAX_DEPTH {
+4 -5
View File
@@ -13,7 +13,7 @@ use effectcraft_svg::{
use kurbo::{Point, Shape as _};
use crate::build::{Builder, MAX_PATH};
use crate::color::{Cs, Func, color_space, text_string};
use crate::color::{Cs, Func, color_space, names_resource, text_string};
use crate::font::Font;
use crate::object::{Dict, File, Lexer, Obj, decode_stream};
@@ -1054,10 +1054,9 @@ impl<'a> Interp<'a> {
Fill::Color(c) => *c,
_ => [0.0; 3],
};
// A named colour space other than a device one is looked up in `res`, which differs
// between placements, so those images are decoded each time.
let named_space =
self.file.get(d, "ColorSpace").and_then(Obj::name).is_some_and(|n| !matches!(n, "DeviceGray" | "DeviceRGB" | "DeviceCMYK" | "G" | "RGB" | "CMYK"));
// A named colour space other than a device one, even inside an array, is looked up in
// `res`, which differs between placements, so those images are decoded each time.
let named_space = self.file.get(d, "ColorSpace").is_some_and(|c| names_resource(self.file, c, 0));
let cache_key = key.filter(|_| !named_space).map(|k| (k, fill.map(|x| (x.clamp(0.0, 1.0) * 255.0).round() as u8)));
let decoded = match cache_key.as_ref().and_then(|k| self.images.get(k)) {
Some(img) => img.clone(),
+26
View File
@@ -699,3 +699,29 @@ fn an_image_with_a_named_colour_space_is_not_shared_between_resource_dictionarie
assert_eq!(found.len(), 2, "{:?}", doc.skipped);
assert_ne!(found[0].rgba, found[1].rgba, "each placement is decoded with its own colour space");
}
#[test]
fn an_image_with_a_named_base_inside_an_array_is_not_shared_between_resource_dictionaries() {
fn images<'a>(g: &'a effectcraft_svg::Group, out: &mut Vec<&'a effectcraft_svg::Image>) {
for n in &g.children {
match n {
Node::Group(g) => images(g, out),
Node::Image(i) => out.push(i),
Node::Shape(_) => {}
}
}
}
let form = |space: &str| {
format!("<< /Type /XObject /Subtype /Form /BBox [0 0 200 100] /Resources << /XObject << /Im 10 0 R >> /ColorSpace << /CS1 /{space} >> >> >>")
};
let objs = vec![
(10, "<< /Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace [/Indexed /CS1 0 <0A141E>] /BitsPerComponent 8 >>".into(), Some(vec![0])),
(11, form("DeviceGray"), Some(b"/Im Do".to_vec())),
(12, form("DeviceRGB"), Some(b"/Im Do".to_vec())),
];
let doc = parse(&page_pdf("/FA Do /FB Do", "/XObject << /FA 11 0 R /FB 12 0 R >>", objs)).unwrap();
let mut found = vec![];
images(&doc.root, &mut found);
assert_eq!(found.len(), 2, "{:?}", doc.skipped);
assert_ne!(found[0].rgba, found[1].rgba, "each placement is decoded with its own colour space");
}