From 1306afb5e843748eaf17a6fa3af39ea3052f869a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 13:49:50 -0500 Subject: [PATCH 01/20] Cut over roxmltree, png, tiff, psd, and trace codecs to craft-libs (#7638) --- Cargo.lock | 154 ++++++++++++++++-- Cargo.toml | 5 + crates/engine/Cargo.toml | 2 +- crates/engine/src/cmd/place/docx.rs | 152 ++++++----------- crates/engine/src/cmd/variablecmds/library.rs | 45 +++-- crates/pdf/Cargo.toml | 4 +- crates/pdf/src/file_info.rs | 125 +++++--------- crates/render/Cargo.toml | 8 +- crates/render/src/encode/tests.rs | 6 +- crates/render/src/encode/tests_psd.rs | 25 +-- crates/render/src/encode/tests_tiff.rs | 98 ++++++----- crates/render/src/paint.rs | 101 ++++++++---- crates/trace/Cargo.toml | 2 +- crates/trace/src/opaque.rs | 98 +++++++---- crates/trace/src/raster.rs | 112 ++++++++----- crates/trace/src/tests.rs | 18 +- 16 files changed, 562 insertions(+), 393 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 32fcfc62ad..7782fdbaa3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -508,6 +508,12 @@ dependencies = [ "serde", ] +[[package]] +name = "bit_field" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb2907cc03d08cdb1c6724ad0670ce67c4a71e1c45a6d7635224011714cfc4b1" + [[package]] name = "bitflags" version = "1.3.2" @@ -1033,6 +1039,36 @@ dependencies = [ "writeable", ] +[[package]] +name = "craft-image-codecs" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=e2dc681a3#e2dc681a3ad8a47038da2f11ddd2c5e1c0454991" +dependencies = [ + "craft-jpeg", + "exr", + "flate2", + "half", + "image", + "image-webp", + "jpeg-encoder", + "png 0.18.1", + "rayon", + "serde_json", + "thiserror 2.0.21", + "tiff", + "zune-core 0.5.3", + "zune-jpeg 0.5.15", + "zune-jpegxl", +] + +[[package]] +name = "craft-jpeg" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=e2dc681a3#e2dc681a3ad8a47038da2f11ddd2c5e1c0454991" +dependencies = [ + "thiserror 2.0.21", +] + [[package]] name = "craft-keymap" version = "0.1.0" @@ -1090,12 +1126,23 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "craft-psd" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=e2dc681a3#e2dc681a3ad8a47038da2f11ddd2c5e1c0454991" +dependencies = [ + "encoding_rs", + "flate2", + "rayon", + "thiserror 2.0.21", +] + [[package]] name = "craft-svg" version = "0.1.0" source = "git+https://github.com/storytold/craft-libs?rev=6d620e971e820b517b11a99cad040ebac670e1e2#6d620e971e820b517b11a99cad040ebac670e1e2" dependencies = [ - "craft-xml", + "craft-xml 0.1.0 (git+https://github.com/storytold/craft-libs?rev=6d620e971e820b517b11a99cad040ebac670e1e2)", "flate2", "kurbo 0.13.1", "roxmltree 0.21.2", @@ -1105,6 +1152,15 @@ dependencies = [ "vectorcraft-pathops 0.18.0", ] +[[package]] +name = "craft-tiff" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=e2dc681a3#e2dc681a3ad8a47038da2f11ddd2c5e1c0454991" +dependencies = [ + "serde", + "thiserror 2.0.21", +] + [[package]] name = "craft-ui" version = "0.1.0" @@ -1203,6 +1259,15 @@ dependencies = [ "thiserror 2.0.21", ] +[[package]] +name = "craft-xml" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=e2dc681a3#e2dc681a3ad8a47038da2f11ddd2c5e1c0454991" +dependencies = [ + "quick-xml 0.41.0", + "thiserror 2.0.21", +] + [[package]] name = "crc32fast" version = "1.5.3" @@ -1715,6 +1780,22 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "exr" +version = "1.74.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb29d0be702adf92c645d9b2f93472906ba83d400e59f98adb3770ce5c18c2f6" +dependencies = [ + "bit_field", + "half", + "lebe", + "miniz_oxide 0.8.9", + "multiversion", + "rayon-core", + "smallvec", + "zune-inflate", +] + [[package]] name = "fastrand" version = "2.5.0" @@ -2747,6 +2828,7 @@ dependencies = [ "moxcms", "num-traits", "png 0.18.1", + "qoi", "tiff", "zune-core 0.5.3", "zune-jpeg 0.5.15", @@ -3200,6 +3282,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" +[[package]] +name = "lebe" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a79a3332a6609480d7d0c9eab957bca6b455b91bb84e66d19f5ff66294b85b8" + [[package]] name = "libc" version = "0.2.191" @@ -3399,6 +3487,28 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "multiversion" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb8a68be21d0755f83bfe47e79b896cc732687cf1903f0e50585e8d6ecc53076" +dependencies = [ + "multiversion-macros", + "target-features", +] + +[[package]] +name = "multiversion-macros" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a05e0c3c3e4edaa21d87c645852ed0b0c3663752bcc6ec43353bd6189dce35e1" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", + "target-features", +] + [[package]] name = "multiversion_no_op" version = "1.0.0" @@ -4349,21 +4459,21 @@ dependencies = [ "unarray", ] -[[package]] -name = "psd" -version = "0.3.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a25f9b8cfffd65d911baf31a033239f7a0facb755faa2481575b70db5dc9195" -dependencies = [ - "thiserror 1.0.69", -] - [[package]] name = "pxfm" version = "0.1.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d55d956fa96f5ec02be2e13af0e20391a5aa83d6a074e3ad368959d0fab299ea" +[[package]] +name = "qoi" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f6d64c71eb498fe9eae14ce4ec935c555749aef511cca85b5568910d6e48001" +dependencies = [ + "bytemuck", +] + [[package]] name = "quick-error" version = "1.2.3" @@ -5734,6 +5844,12 @@ dependencies = [ "xattr", ] +[[package]] +name = "target-features" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1bbb9f3c5c463a01705937a24fdabc5047929ac764b2d5b9cf681c1f5041ed5" + [[package]] name = "tempfile" version = "3.27.0" @@ -6546,6 +6662,7 @@ dependencies = [ "chrono", "craft-docpath", "craft-proof", + "craft-xml 0.1.0 (git+https://github.com/storytold/craft-libs?rev=e2dc681a3)", "crc32fast", "flate2", "hayro-jpeg2000", @@ -6558,7 +6675,6 @@ dependencies = [ "log", "proptest", "rayon", - "roxmltree 0.21.2", "rustix 1.1.6", "ruzstd", "same-file", @@ -6733,6 +6849,7 @@ dependencies = [ "aes", "chrono", "craft-pdf", + "craft-xml 0.1.0 (git+https://github.com/storytold/craft-libs?rev=e2dc681a3)", "flate2", "getrandom 0.4.3", "image", @@ -6743,7 +6860,6 @@ dependencies = [ "md-5", "objc2-core-foundation", "objc2-core-graphics", - "roxmltree 0.21.2", "serde", "serde_json", "sha2", @@ -6784,6 +6900,9 @@ dependencies = [ name = "vectorcraft-render" version = "0.24.1" dependencies = [ + "craft-image-codecs", + "craft-psd", + "craft-tiff", "flate2", "gif 0.14.2", "image", @@ -6791,9 +6910,7 @@ dependencies = [ "log", "png 0.18.1", "proptest", - "psd", "serde_json", - "tiff", "unicode-general-category", "vectorcraft-brush", "vectorcraft-color", @@ -8368,6 +8485,15 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d56377fd46368984a170bc5aac5567e52ca5da874caa60bea39fcbca78fb658b" +[[package]] +name = "zune-inflate" +version = "0.2.54" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73ab332fe2f6680068f3582b16a24f90ad7096d5d39b974d1c0aff0125116f02" +dependencies = [ + "simd-adler32", +] + [[package]] name = "zune-jpeg" version = "0.4.21" diff --git a/Cargo.toml b/Cargo.toml index 610e2b462c..e77c950b1a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,11 @@ craft-eps = { git = "https://github.com/storytold/craft-libs", rev = "6d620e971e craft-dxf = { git = "https://github.com/storytold/craft-libs", rev = "6d620e971e820b517b11a99cad040ebac670e1e2" } # WMF, EMF and EMF+ reader (craft-libs#778). craft-metafile = { git = "https://github.com/storytold/craft-libs", rev = "6d620e971e820b517b11a99cad040ebac670e1e2" } +craft-xml = { git = "https://github.com/storytold/craft-libs", rev = "e2dc681a3" } +craft-image-codecs = { git = "https://github.com/storytold/craft-libs", rev = "e2dc681a3" } +craft-gif = { git = "https://github.com/storytold/craft-libs", rev = "e2dc681a3" } +craft-tiff = { git = "https://github.com/storytold/craft-libs", rev = "e2dc681a3" } +craft-psd = { git = "https://github.com/storytold/craft-libs", rev = "e2dc681a3" } craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "e98299546f5c5c3c22388d16507f8b5e61a5f280", features = ["egui"] } # internal vectorcraft-geom = { path = "crates/geom" } diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 0fccbcdecb..b46678804b 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -53,7 +53,7 @@ heic-rs = { version = "=0.1.4", default-features = false, features = ["std"], op flate2 = { workspace = true } chrono = { workspace = true } # The XML parts of Word documents placed as type (place/docx.rs). -roxmltree = "0.21" +craft-xml = { workspace = true } # The compound file that holds a Word 97-2003 document placed as type (place/doc.rs). cfb = "0.15" kurbo = { workspace = true } diff --git a/crates/engine/src/cmd/place/docx.rs b/crates/engine/src/cmd/place/docx.rs index 063444442e..ac528f3735 100644 --- a/crates/engine/src/cmd/place/docx.rs +++ b/crates/engine/src/cmd/place/docx.rs @@ -38,7 +38,7 @@ use std::collections::HashMap; -use roxmltree::Node; +use craft_xml::{Canonical, Doctype, El, Limits, Names, Options}; use vectorcraft_doc::text::Kashida; use vectorcraft_doc::{CharPosition, CharStyle, Justify, ParaDirection, ParaStyle, ScriptMetrics, TextRun}; @@ -88,6 +88,7 @@ const MAX_RUNS: usize = 100_000; /// WordprocessingML's namespaces: Transitional, then Strict. const W: [&str; 2] = ["http://schemas.openxmlformats.org/wordprocessingml/2006/main", "http://purl.oclc.org/ooxml/wordprocessingml/main"]; +const DOCX_NS: Canonical<'static> = Canonical::new(&[("w", W[0]), ("w", W[1])]); /// Is the file `name` with `bytes` a Word document: a ZIP archive named `.docx`, or one that holds /// `word/document.xml`? @@ -104,7 +105,7 @@ pub(super) fn import(bytes: &[u8], cmd: &str) -> Result { let main = relationship(&zip, "", "officeDocument", cmd)?.unwrap_or_else(|| "word/document.xml".into()); let Some(xml) = part(&zip, &main, cmd)? else { return Err(bad(cmd, "not a Word document: its package has no main document part")) }; let doc = parse(&xml, &main, cmd)?; - if local(doc.root_element()) != Some("document") { + if local(&doc) != Some("document") { return Err(bad(cmd, "not a Word document: its main part is not a WordprocessingML document")); } let folder = main.rfind('/').and_then(|i| main.get(..=i)).unwrap_or_default(); @@ -117,7 +118,7 @@ pub(super) fn import(bytes: &[u8], cmd: &str) -> Result { if let Some(xml) = part(&zip, &styles, cmd)? { r.styles(&parse(&xml, &styles, cmd)?); } - if let Some(body) = doc.root_element().children().find(|n| local(*n) == Some("body")) { + if let Some(body) = child(&doc, "body") { r.block(body, 0); } r.finish(cmd) @@ -158,57 +159,17 @@ fn part(zip: &Archive, name: &str, cmd: &str) -> Result> { } /// The XML of the part `name`. -fn parse<'a>(xml: &'a str, name: &str, cmd: &str) -> Result> { - // roxmltree parses nested elements recursively. - if nesting(xml) > MAX_NESTING { - return Err(bad(cmd, format!("`{name}` in the Word document nests elements more than {MAX_NESTING} deep"))); - } - let opts = roxmltree::ParsingOptions { nodes_limit: MAX_NODES, ..roxmltree::ParsingOptions::default() }; - roxmltree::Document::parse_with_options(xml, opts).map_err(|e| bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}"))) -} - -/// How deep the elements of `xml` nest: start tags less end tags, at most, outside comments, -/// CDATA sections, processing instructions, declarations and quoted attribute values. -pub(crate) fn nesting(xml: &str) -> usize { - let b = xml.as_bytes(); - let (mut depth, mut most, mut i) = (0usize, 0usize, 0usize); - // The index just past the first `end` at or after `from` (the end of the text if none). - let past = |from: usize, end: &[u8]| { - b.get(from..).and_then(|rest| rest.windows(end.len()).position(|w| w == end)).map_or(b.len(), |at| from + at + end.len()) +fn parse(xml: &str, name: &str, cmd: &str) -> Result { + let opts = Options { + names: Names::Canonical(&DOCX_NS), + doctype: Doctype::Reject, + limits: Limits { max_depth: MAX_NESTING, max_nodes: MAX_NODES as usize, ..Limits::DEFAULT }, + ..Options::default() }; - while let Some(at) = b.get(i..).and_then(|rest| rest.iter().position(|&c| c == b'<')) { - let start = i + at; - let rest = b.get(start..).unwrap_or_default(); - if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else { - // A start tag: up to its `>` outside quotes; `/>` closes it at once. - let mut j = start + 1; - let mut quote = None; - while let Some(&c) = b.get(j) { - match (quote, c) { - (None, b'"' | b'\'') => quote = Some(c), - (Some(q), _) if c == q => quote = None, - (None, b'>') => break, - _ => {} - } - j += 1; - } - if b.get(j.saturating_sub(1)) != Some(&b'/') { - depth += 1; - most = most.max(depth); - } - i = j + 1; - } - } - most + opts.parse(xml.as_bytes()).map_err(|e| match e { + craft_xml::Error::TooDeep(_) => bad(cmd, format!("`{name}` in the Word document nests elements more than {MAX_NESTING} deep")), + _ => bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")), + }) } /// The part that the first relationship of part `source` (`""`: the package) whose type ends in @@ -220,11 +181,10 @@ fn relationship(zip: &Archive, source: &str, kind: &str, cmd: &str) -> Result String { } /// `n`'s local name when it is a WordprocessingML element. -fn local<'a>(n: Node<'a, '_>) -> Option<&'a str> { - (n.is_element() && n.tag_name().namespace().is_some_and(|ns| W.contains(&ns))).then(|| n.tag_name().name()) +fn local(n: &El) -> Option<&str> { + n.name.strip_prefix("w:") } /// `n`'s first WordprocessingML child element named `name`. -fn child<'a, 'i>(n: Node<'a, 'i>, name: &str) -> Option> { - n.children().find(|c| local(*c) == Some(name)) +fn child<'a>(n: &'a El, name: &str) -> Option<&'a El> { + n.elements().find(|c| local(c) == Some(name)) } /// `n`'s WordprocessingML attribute `name`. -fn attr<'a>(n: Node<'a, '_>, name: &str) -> Option<&'a str> { - n.attributes().find(|a| a.name() == name && a.namespace().is_some_and(|ns| W.contains(&ns))).map(|a| a.value()) +fn attr<'a>(n: &'a El, name: &str) -> Option<&'a str> { + n.attrs.iter().find_map(|(k, v)| k.strip_prefix("w:").filter(|&l| l == name).map(|_| v.as_str())) } /// An on/off property (ST_OnOff): on unless `w:val` is `false`, `0` or `off`. -fn on(n: Node) -> bool { +fn on(n: &El) -> bool { !matches!(attr(n, "val"), Some("false" | "0" | "off")) } @@ -280,12 +240,12 @@ fn measure(v: &str, per_pt: f64) -> Option { } /// Twentieths of a point. -fn twips(n: Node, name: &str) -> Option { +fn twips(n: &El, name: &str) -> Option { attr(n, name).and_then(|v| measure(v, 20.0)) } /// Half points. -fn half_points(n: Node) -> Option { +fn half_points(n: &El) -> Option { attr(n, "val").and_then(|v| measure(v, 2.0)) } @@ -462,21 +422,21 @@ struct Reader { impl Reader { /// The major and minor fonts of a theme part's font scheme. - fn theme_fonts(&mut self, doc: &roxmltree::Document) { + fn theme_fonts(&mut self, doc: &El) { for (slot, scheme) in self.theme.iter_mut().zip(["majorFont", "minorFont"]) { - *slot = doc - .descendants() - .find(|n| n.tag_name().name() == scheme) - .and_then(|n| n.children().find(|c| c.tag_name().name() == "latin")) - .and_then(|n| n.attribute("typeface")) + *slot = std::iter::once(doc) + .chain(doc.descendants()) + .find(|n| n.local() == scheme) + .and_then(|n| n.elements().find(|c| c.local() == "latin")) + .and_then(|n| n.attr("typeface")) .map(font_name) .filter(|f| !f.is_empty()); } } /// The document defaults and the styles of the styles part. - fn styles(&mut self, doc: &roxmltree::Document) { - for n in doc.root_element().children() { + fn styles(&mut self, doc: &El) { + for n in doc.elements() { match local(n) { Some("docDefaults") => { let props = |outer: &str, inner: &str| child(n, outer).and_then(|d| child(d, inner)); @@ -526,7 +486,7 @@ impl Reader { } /// The number of the font a `w:rFonts` element names. - fn font(&mut self, n: Node) -> Option { + fn font(&mut self, n: &El) -> Option { let theme = |v: &str| self.theme.get(usize::from(v.starts_with("minor"))).and_then(Option::as_deref); let name = attr(n, "asciiTheme") .and_then(theme) @@ -549,9 +509,9 @@ impl Reader { } /// The run properties of a `w:rPr` element. - fn run_props(&mut self, rpr: Node) -> RunProps { + fn run_props(&mut self, rpr: &El) -> RunProps { let mut p = RunProps::default(); - for n in rpr.children() { + for n in rpr.elements() { match local(n) { Some("rFonts") => p.font = self.font(n), Some("b") => p.bold = Some(on(n)), @@ -594,11 +554,11 @@ impl Reader { } /// The paragraphs, tables and containers of them that are `n`'s children. - fn block(&mut self, n: Node, depth: usize) { + fn block(&mut self, n: &El, depth: usize) { if depth > MAX_DEPTH { return; } - for c in n.children() { + for c in n.elements() { if self.full { return; } @@ -617,7 +577,7 @@ impl Reader { } /// A table: each row a paragraph, its cells separated by tabs. - fn table(&mut self, tbl: Node, depth: usize) { + fn table(&mut self, tbl: &El, depth: usize) { for row in items(tbl, "tr", depth) { for cell in items(row, "tc", depth) { self.flush(); @@ -638,7 +598,7 @@ impl Reader { } /// A paragraph. - fn paragraph(&mut self, p: Node, depth: usize) { + fn paragraph(&mut self, p: &El, depth: usize) { self.flush(); let ppr = child(p, "pPr"); let style = ppr @@ -675,11 +635,11 @@ impl Reader { } /// The runs of a paragraph, and the containers of runs in it. - fn inline(&mut self, n: Node, depth: usize) { + fn inline(&mut self, n: &El, depth: usize) { if depth > MAX_DEPTH { return; } - for c in n.children() { + for c in n.elements() { match local(c) { Some("r") => self.run(c), Some("hyperlink" | "ins" | "moveTo" | "smartTag" | "customXml" | "fldSimple" | "dir" | "bdo") => self.inline(c, depth + 1), @@ -694,7 +654,7 @@ impl Reader { } /// A run. - fn run(&mut self, r: Node) { + fn run(&mut self, r: &El) { let rpr = child(r, "rPr"); let mut chars = self.base; let style = rpr @@ -709,11 +669,10 @@ impl Reader { let props = self.run_props(n); chars.apply(&props, false); } - for c in r.children() { + for c in r.elements() { match local(c) { Some("t") => { - // Comments and processing instructions may split the text. - for ch in c.children().filter(|n| n.is_text()).filter_map(|n| n.text()).flat_map(str::chars) { + for ch in c.text().chars() { self.put(ch, chars); } } @@ -907,9 +866,9 @@ fn font_name(name: &str) -> String { } /// The paragraph properties of a `w:pPr` element. -fn para_props(ppr: Node) -> ParaProps { +fn para_props(ppr: &El) -> ParaProps { let mut p = ParaProps::default(); - for n in ppr.children() { + for n in ppr.elements() { match local(n) { Some("jc") => { p.align = match attr(n, "val") { @@ -945,12 +904,12 @@ fn para_props(ppr: Node) -> ParaProps { } /// `n`'s children named `name`, and those inside content controls and custom XML among them. -fn items<'a, 'i>(n: Node<'a, 'i>, name: &str, depth: usize) -> Vec> { +fn items<'a>(n: &'a El, name: &str, depth: usize) -> Vec<&'a El> { let mut out = vec![]; if depth > MAX_DEPTH { return out; } - for c in n.children() { + for c in n.elements() { match local(c) { Some(l) if l == name => out.push(c), Some("sdt") => { @@ -979,15 +938,6 @@ mod tests { assert_eq!(measure("wide", 20.0), None); } - #[test] - fn nesting_counts_elements_outside_markup_that_is_not_an_element() { - assert_eq!(nesting(r#"]]>"#), 2); - assert_eq!(nesting(""), 3, "unclosed elements count"); - assert_eq!(nesting(""), 1); - assert_eq!(nesting(" Result { let xml = without_doctype(xml, cmd)?; - // roxmltree parses nested elements recursively. - if crate::cmd::place::docx::nesting(&xml) > MAX_LIBRARY_NESTING { - return Err(bad(cmd, format!("the variable library nests elements more than {MAX_LIBRARY_NESTING} deep"))); - } - let opts = roxmltree::ParsingOptions { nodes_limit: MAX_LIBRARY_NODES, ..roxmltree::ParsingOptions::default() }; - let doc = - roxmltree::Document::parse_with_options(&xml, opts).map_err(|e| bad(cmd, format!("the variable library is not well-formed XML: {e}")))?; - let is = |n: &roxmltree::Node, name: &str| n.is_element() && n.tag_name().name() == name; - let Some(root) = doc.descendants().find(|n| is(n, "variableSets")) else { + let opts = craft_xml::Options { + names: craft_xml::Names::Local, + limits: craft_xml::Limits { max_depth: MAX_LIBRARY_NESTING, max_nodes: MAX_LIBRARY_NODES as usize, ..craft_xml::Limits::DEFAULT }, + ..craft_xml::Options::default() + }; + let doc = opts.parse(xml.as_bytes()).map_err(|e| match e { + craft_xml::Error::TooDeep(_) => bad(cmd, format!("the variable library nests elements more than {MAX_LIBRARY_NESTING} deep")), + _ => bad(cmd, format!("the variable library is not well-formed XML: {e}")), + })?; + let Some(root) = std::iter::once(&doc).chain(doc.descendants()).find(|n| n.is("variableSets")) else { return Err(bad(cmd, "the file holds no `variableSets` element")); }; - let sets: Vec = root.children().filter(|n| is(n, "variableSet")).collect(); + let sets: Vec<&craft_xml::El> = root.children("variableSet").collect(); let (mut text, mut other, mut declared, mut refs) = (Vec::new(), Vec::new(), BTreeSet::new(), Vec::new()); - for var in sets.iter().flat_map(|s| s.children()).filter(|n| is(n, "variables")).flat_map(|v| v.children()).filter(|n| is(n, "variable")) { - let name = named(var.attribute("varName"), "varName", cmd)?; + for var in sets.iter().flat_map(|s| s.children("variables")).flat_map(|v| v.children("variable")) { + let name = named(var.attr("varName"), "varName", cmd)?; if !declared.insert(name.clone()) { return Err(bad(cmd, format!("variable `{name}` is declared twice"))); } if declared.len() > MAX_LIBRARY_VARIABLES { return Err(bad(cmd, format!("the variable library declares more than {MAX_LIBRARY_VARIABLES} variables"))); } - match var.attribute("trait") { + match var.attr("trait") { Some("textcontent") => { // `docRef="id('…')"` names the element of the variable's bound object. - let element = var.attribute("docRef").and_then(|r| r.trim().strip_prefix("id('")?.strip_suffix("')")).filter(|e| !e.is_empty()); + let element = var.attr("docRef").and_then(|r| r.trim().strip_prefix("id('")?.strip_suffix("')")).filter(|e| !e.is_empty()); if let Some(element) = element { refs.push((name.clone(), element.to_string())); } @@ -237,10 +238,8 @@ fn parse_library(xml: &str, cmd: &str) -> Result { } let text_names: BTreeSet<&str> = text.iter().map(String::as_str).collect(); let (mut datasets, mut seen) = (Vec::new(), BTreeSet::new()); - for row in - sets.iter().flat_map(|s| s.children()).filter(|n| is(n, "sampleDataSets")).flat_map(|d| d.children()).filter(|n| is(n, "sampleDataSet")) - { - let name = named(row.attribute("dataSetName"), "dataSetName", cmd)?; + for row in sets.iter().flat_map(|s| s.children("sampleDataSets")).flat_map(|d| d.children("sampleDataSet")) { + let name = named(row.attr("dataSetName"), "dataSetName", cmd)?; if !seen.insert(name.clone()) { return Err(bad(cmd, format!("data set `{name}` is declared twice"))); } @@ -248,8 +247,8 @@ fn parse_library(xml: &str, cmd: &str) -> Result { return Err(bad(cmd, format!("the variable library declares more than {MAX_LIBRARY_DATASETS} data sets"))); } let (mut values, mut held) = (BTreeMap::new(), BTreeSet::new()); - for el in row.children().filter(roxmltree::Node::is_element) { - let var = el.tag_name().name(); + for el in row.elements() { + let var = el.name.as_str(); if !declared.contains(var) { return Err(bad(cmd, format!("data set `{name}` holds a value for `{var}`, which the library doesn't declare"))); } @@ -259,11 +258,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result { if !text_names.contains(var) { continue; } - let paras: Vec = el - .children() - .filter(|n| is(n, "p")) - .map(|p| p.descendants().filter(roxmltree::Node::is_text).filter_map(|t| t.text()).collect()) - .collect(); + let paras: Vec = el.children("p").map(craft_xml::El::deep_text).collect(); if !paras.is_empty() { values.insert(var.to_string(), DataValue::Text(paras.join("\n"))); } diff --git a/crates/pdf/Cargo.toml b/crates/pdf/Cargo.toml index d4394dfefa..d59243eb75 100644 --- a/crates/pdf/Cargo.toml +++ b/crates/pdf/Cargo.toml @@ -33,8 +33,8 @@ smallvec = "1" siphasher = "1" weezl = "0.2" serde = { workspace = true } -# The XMP packet that File Info is read from (file_info.rs); the version usvg already uses. -roxmltree = "0.21" +# The XMP packet that File Info is read from (file_info.rs). +craft-xml = { workspace = true } # PDF security (encrypt.rs): pure Rust, MIT OR Apache-2.0, builds for wasm. aes = "0.8" md-5 = { version = "0.10", default-features = false } diff --git a/crates/pdf/src/file_info.rs b/crates/pdf/src/file_info.rs index 1cb0a45273..908212aabe 100644 --- a/crates/pdf/src/file_info.rs +++ b/crates/pdf/src/file_info.rs @@ -5,7 +5,7 @@ use craft_pdf::syntax::Pdf; use craft_pdf::syntax::object::{DateTime, Dict, Stream}; -use roxmltree::Node; +use craft_xml::{Canonical, Doctype, El, Limits, Names, Options}; use crate::import_scan::text_string; @@ -15,12 +15,14 @@ const MAX_XMP: usize = 8 << 20; const MAX_NESTING: usize = 256; const RDF: &str = "http://www.w3.org/1999/02/22-rdf-syntax-ns#"; -const XML: &str = "http://www.w3.org/XML/1998/namespace"; pub(crate) const DC: &str = "http://purl.org/dc/elements/1.1/"; const XMP: &str = "http://ns.adobe.com/xap/1.0/"; // brand-ok: the XMP namespace the files use pub(crate) const XMP_RIGHTS: &str = "http://ns.adobe.com/xap/1.0/rights/"; // brand-ok: the XMP namespace the files use const PDF: &str = "http://ns.adobe.com/pdf/1.3/"; // brand-ok: the XMP namespace the files use +const XMP_NAMESPACES: Canonical<'static> = + Canonical::new(&[("rdf", RDF), ("dc", DC), ("xmp", XMP), ("xmpRights", XMP_RIGHTS), ("pdf", PDF)]); + /// The File Info fields a PDF gives, each `None` when the file gives none (or only blank text). #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct FileInfo { @@ -73,39 +75,39 @@ fn packet(pdf: &Pdf) -> Option> { } /// The File Info fields of an XMP packet; `None` when it isn't XML in UTF-8 or UTF-16, or when its -/// elements nest more than [`MAX_NESTING`] deep. +/// [`xmp`] of `packet`; `None` when it isn't XML in UTF-8 or UTF-16, or when its elements nest +/// more than [`MAX_NESTING`] deep. fn xmp(packet: &[u8]) -> Option { let text = xml_text(packet)?; - // roxmltree parses nested elements recursively. - if nesting(&text) > MAX_NESTING { - return None; + let opts = Options { + names: Names::Canonical(&XMP_NAMESPACES), + doctype: Doctype::Reject, + limits: Limits { max_depth: MAX_NESTING, ..Limits::DEFAULT }, + ..Options::default() } - let xml = roxmltree::Document::parse(&text).ok()?; + .strict(); + let root = opts.parse(text.as_bytes()).ok()?; // The resource's own descriptions: those right under `rdf:RDF`, not the ones inside a value. - let descriptions: Vec> = xml - .descendants() - .filter(|n| n.has_tag_name((RDF, "RDF"))) - .flat_map(|rdf| rdf.children()) - .filter(|n| n.has_tag_name((RDF, "Description"))) - .collect(); - let get = |ns: &str, name: &str| property(&descriptions, ns, name); - let first = |ns: &str, name: &str| get(ns, name).into_iter().next(); - let creators = get(DC, "creator"); - let subjects = get(DC, "subject"); + let descriptions: Vec<&El> = + std::iter::once(&root).chain(root.descendants()).filter(|n| n.is("rdf:RDF")).flat_map(|rdf| rdf.children("rdf:Description")).collect(); + let get = |name: &str| property(&descriptions, name); + let first = |name: &str| get(name).into_iter().next(); + let creators = get("dc:creator"); + let subjects = get("dc:subject"); Some(FileInfo { - title: first(DC, "title"), + title: first("dc:title"), author: (!creators.is_empty()).then(|| creators.join(", ")), - description: first(DC, "description"), - keywords: if subjects.is_empty() { first(PDF, "Keywords").map(|k| split(&k)).filter(|k| !k.is_empty()) } else { Some(subjects) }, - copyright_notice: first(DC, "rights"), - copyright_url: first(XMP_RIGHTS, "WebStatement"), - copyrighted: first(XMP_RIGHTS, "Marked").and_then(|m| match m.to_ascii_lowercase().as_str() { + description: first("dc:description"), + keywords: if subjects.is_empty() { first("pdf:Keywords").map(|k| split(&k)).filter(|k| !k.is_empty()) } else { Some(subjects) }, + copyright_notice: first("dc:rights"), + copyright_url: first("xmpRights:WebStatement"), + copyrighted: first("xmpRights:Marked").and_then(|m| match m.to_ascii_lowercase().as_str() { "true" => Some(true), "false" => Some(false), _ => None, }), - created: first(XMP, "CreateDate").and_then(|d| xmp_date(&d)), - modified: first(XMP, "ModifyDate").and_then(|d| xmp_date(&d)), + created: first("xmp:CreateDate").and_then(|d| xmp_date(&d)), + modified: first("xmp:ModifyDate").and_then(|d| xmp_date(&d)), }) } @@ -123,58 +125,14 @@ fn xml_text(b: &[u8]) -> Option { } } -/// How deep the elements of `xml` nest: start tags less end tags, at most, outside comments, -/// CDATA sections, processing instructions, declarations and quoted attribute values. -fn nesting(xml: &str) -> usize { - let b = xml.as_bytes(); - let (mut depth, mut most, mut i) = (0usize, 0usize, 0usize); - // The index just past the first `end` at or after `from` (the end of the text if none). - let past = |from: usize, end: &[u8]| { - b.get(from..).and_then(|rest| rest.windows(end.len()).position(|w| w == end)).map_or(b.len(), |at| from + at + end.len()) - }; - while let Some(at) = b.get(i..).and_then(|rest| rest.iter().position(|&c| c == b'<')) { - let start = i + at; - let rest = b.get(start..).unwrap_or_default(); - if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else if rest.starts_with(b""); - } else { - // A start tag: up to its `>` outside quotes; `/>` closes it at once. - let mut j = start + 1; - let mut quote = None; - while let Some(&c) = b.get(j) { - match (quote, c) { - (None, b'"' | b'\'') => quote = Some(c), - (Some(q), _) if c == q => quote = None, - (None, b'>') => break, - _ => {} - } - j += 1; - } - if b.get(j.saturating_sub(1)) != Some(&b'/') { - depth += 1; - most = most.max(depth); - } - i = j + 1; - } - } - most -} - -/// The values of property `ns` `name` on the first of `descriptions` that gives it: an attribute +/// The values of property `name` on the first of `descriptions` that gives it: an attribute /// of the `rdf:Description`, or a property element's items ([`items`]), blank ones left out. -fn property(descriptions: &[Node<'_, '_>], ns: &str, name: &str) -> Vec { +fn property(descriptions: &[&El], name: &str) -> Vec { for d in descriptions { - if let Some(v) = d.attribute((ns, name)).map(str::trim).filter(|v| !v.is_empty()) { + if let Some(v) = d.attr(name).map(str::trim).filter(|v| !v.is_empty()) { return vec![v.to_string()]; } - let found = d.children().filter(|c| c.has_tag_name((ns, name))).map(items).find(|v| !v.is_empty()); + let found = d.children(name).map(items).find(|v| !v.is_empty()); if let Some(v) = found { return v; } @@ -185,13 +143,12 @@ fn property(descriptions: &[Node<'_, '_>], ns: &str, name: &str) -> Vec /// The items of property element `p`: the `rdf:li` of its `rdf:Seq`, `rdf:Bag` or `rdf:Alt` (the /// `x-default` language item first), else its own value ([`value_of`]); trimmed, blank ones left /// out. -fn items(p: Node<'_, '_>) -> Vec { - let array = p.children().find(|c| ["Seq", "Bag", "Alt"].into_iter().any(|t| c.has_tag_name((RDF, t)))); +fn items(p: &El) -> Vec { + let array = p.elements().find(|c| matches!(c.name.as_str(), "rdf:Seq" | "rdf:Bag" | "rdf:Alt")); let mut values: Vec<(bool, String)> = match array { Some(a) => a - .children() - .filter(|c| c.has_tag_name((RDF, "li"))) - .map(|li| (li.attribute((XML, "lang")).is_some_and(|l| l.eq_ignore_ascii_case("x-default")), value_of(li))) + .children("rdf:li") + .map(|li| (li.attr("xml:lang").is_some_and(|l| l.eq_ignore_ascii_case("x-default")), value_of(li))) .collect(), None => vec![(false, value_of(p))], }; @@ -203,19 +160,19 @@ fn items(p: Node<'_, '_>) -> Vec { /// The value of element `n`: the URI of its `rdf:resource` attribute (how RDF writes a URI value); /// for a qualified value, its `rdf:value` (an attribute or an element, on `n` or on an /// `rdf:Description` inside it), without the qualifiers; else the text inside it. -fn value_of(mut n: Node<'_, '_>) -> String { +fn value_of(mut n: &El) -> String { // Each step goes to an element inside `n`, so the loop ends. loop { - if let Some(uri) = n.attribute((RDF, "resource")) { + if let Some(uri) = n.attr("rdf:resource") { return uri.to_string(); } - let holder = n.children().find(|c| c.has_tag_name((RDF, "Description"))).unwrap_or(n); - if let Some(v) = holder.attribute((RDF, "value")) { + let holder = n.child("rdf:Description").unwrap_or(n); + if let Some(v) = holder.attr("rdf:value") { return v.to_string(); } - match holder.children().find(|c| c.has_tag_name((RDF, "value"))) { + match holder.child("rdf:value") { Some(v) => n = v, - None => return n.descendants().filter(|d| d.is_text()).filter_map(|d| d.text()).collect(), + None => return n.deep_text(), } } } diff --git a/crates/render/Cargo.toml b/crates/render/Cargo.toml index 4bd499b9d4..ecdaa60273 100644 --- a/crates/render/Cargo.toml +++ b/crates/render/Cargo.toml @@ -19,13 +19,12 @@ vello_cpu = { workspace = true } # Filter types (Gaussian blur, drop shadow) are not re-exported by vello_cpu 0.2. vello_common = { version = "0.2", default-features = false } image = { workspace = true } +craft-image-codecs = { workspace = true } flate2 = { workspace = true } jpeg-encoder = { workspace = true } gif = { workspace = true } # TIFF LZW compression (MIT OR Apache-2.0; already used by the image crate's TIFF reader). weezl = "0.2" -# PNG rows read one at a time (MIT OR Apache-2.0; already used by the image crate's PNG reader). -png = "0.18" log = { workspace = true } # Combining marks inside a lam-alef cluster (MIT OR Apache-2.0; already used by vectorcraft-text). unicode-general-category = "1" @@ -47,8 +46,7 @@ vectorcraft-plugins = { workspace = true } wat = { workspace = true } # Decoders the export tests read their files back with. image = { workspace = true, features = ["tga"] } -tiff = "0.11" -# Reads the PSD exports back (MIT OR Apache-2.0). -psd = "0.3" +craft-tiff = { workspace = true } +craft-psd = { workspace = true } proptest = { workspace = true } serde_json = { workspace = true } diff --git a/crates/render/src/encode/tests.rs b/crates/render/src/encode/tests.rs index a910480641..0a8227b75f 100644 --- a/crates/render/src/encode/tests.rs +++ b/crates/render/src/encode/tests.rs @@ -86,8 +86,10 @@ fn gray_and_bilevel_images_store_gray() { assert_eq!(gray(&file), levels); let (mime, file) = img.to_bilevel(150.0).unwrap(); assert_eq!(mime, "image/tiff"); - let mut d = ::tiff::decoder::Decoder::new(std::io::Cursor::new(&file)).unwrap(); - assert_eq!(d.colortype().unwrap(), ::tiff::ColorType::Gray(1)); + let tiff = craft_tiff::Tiff::parse(&file).unwrap(); + let ifd = &tiff.ifds[0]; + assert_eq!(ifd.u16(craft_tiff::tags::SAMPLES_PER_PIXEL), Some(1)); + assert_eq!(ifd.value(craft_tiff::tags::BITS_PER_SAMPLE).unwrap().to_u64_vec(), vec![1]); let bw: Vec = [0u8, 0, 255, 255, 255, 0].iter().flat_map(|&g| [g, 255]).collect(); assert_eq!(gray(&file), bw, "from 128 up is white"); // Half-covered white over the light grey; a clear pixel is white. diff --git a/crates/render/src/encode/tests_psd.rs b/crates/render/src/encode/tests_psd.rs index a488cceff2..7655fa16e7 100644 --- a/crates/render/src/encode/tests_psd.rs +++ b/crates/render/src/encode/tests_psd.rs @@ -146,7 +146,7 @@ fn unpack(mut src: &[u8], out: &mut Vec) { /// The composite as a reader sees it. fn merged(f: &[u8]) -> Vec { - ::psd::Psd::from_bytes(f).expect("a PSD a reader takes").rgba() + craft_psd::PsdFile::from_bytes(f).expect("a PSD a reader takes").composite_rgba8().expect("composite").data } fn pixel(px: &[u8], width: usize, x: usize, y: usize) -> [u8; 4] { @@ -168,11 +168,12 @@ fn layers_are_the_top_level_layers_and_the_merged_image_is_the_render() { assert_eq!(p.records[1].rect, [0, 0, 50, 60], "the blue square and the corner"); // A reader decodes it: the layers, in place, and the merged image. - let psd = ::psd::Psd::from_bytes(&file).unwrap(); - assert_eq!((psd.width(), psd.height(), psd.layers().len()), (100, 60, 2)); - let red = psd.layer_by_name("Layer 1").unwrap().rgba(); - assert_eq!(pixel(&red, 100, 20, 20), [255, 0, 0, 255]); - assert_eq!(pixel(&red, 100, 50, 45)[3], 0, "the blue square isn't on Layer 1"); + let psd = craft_psd::PsdFile::from_bytes(&file).unwrap(); + assert_eq!((psd.header.width, psd.header.height, psd.layers().len()), (100, 60, 2)); + let layer = psd.iter_layers().find(|l| l.name() == "Layer 1").unwrap(); + assert_eq!(layer.record.rect, craft_psd::layer::Rect { top: 10, left: 10, bottom: 40, right: 40 }); + let red = layer.rgba8().unwrap().data; + assert_eq!(pixel(&red, 30, 10, 10), [255, 0, 0, 255]); // The merged image is the flat render: colours on white where transparent, alpha apart. let img = Renderer::new().render_region_with(&doc, doc.artboards[0].rect, 1.0, &opts(PsdOptions::default()).render_options(RasterFormat::Psd)); @@ -200,7 +201,7 @@ fn flat_writes_the_merged_image_alone_on_white() { assert_eq!(o.encode(&img, RasterFormat::Psd).unwrap(), file); let file = export(&doc, &RasterExportOptions { ppi: 150.0, ..o }); assert_eq!(u32_at(parse(&file).resources, 12), 150 << 16); - assert_eq!(::psd::Psd::from_bytes(&file).unwrap().width(), 208); + assert_eq!(craft_psd::PsdFile::from_bytes(&file).unwrap().header.width, 208); } #[test] @@ -230,8 +231,10 @@ fn maximum_editability_writes_groups_and_an_object_per_layer() { assert_eq!((p.records[4].opacity, p.records[4].blend, p.records[4].rect), (128, *b"mul ", [20, 30, 50, 60])); assert_eq!((p.records[8].blend, p.records[8].opacity), (*b"pass", 255)); assert_eq!(p.records[6].rect, [0, 0, 6, 6]); - let psd = ::psd::Psd::from_bytes(&file).unwrap(); - assert_eq!((psd.layers().len(), psd.groups().len()), (3, 3)); + let psd = craft_psd::PsdFile::from_bytes(&file).unwrap(); + let content_layers = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::Other)).count(); + let groups = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::OpenFolder | craft_psd::layer::SectionType::ClosedFolder)).count(); + assert_eq!((content_layers, groups), (3, 3)); assert_eq!(merged(&file), merged(&export(&doc, &opts(PsdOptions::default()))), "the same merged image"); // Hidden layers: written hidden, drawn as if visible, instead of left out. @@ -300,8 +303,8 @@ fn a_background_colour_is_the_bottom_layer() { let p = parse(&file); assert_eq!((p.channels, p.count), (3, 3), "opaque: no merged alpha"); assert_eq!((p.records[0].name.as_str(), p.records[0].rect), ("Background", [0, 0, 60, 100])); - let psd = ::psd::Psd::from_bytes(&file).unwrap(); - assert!(psd.layer_by_name("Background").unwrap().rgba().chunks(4).all(|p| p == [0, 0, 255, 255])); + let psd = craft_psd::PsdFile::from_bytes(&file).unwrap(); + assert!(psd.iter_layers().find(|l| l.name() == "Background").unwrap().rgba8().unwrap().data.chunks(4).all(|p| p == [0, 0, 255, 255])); assert_eq!(pixel(&merged(&file), 100, 5, 50), [0, 0, 255, 255]); } diff --git a/crates/render/src/encode/tests_tiff.rs b/crates/render/src/encode/tests_tiff.rs index c70eacd2fc..380b6d155c 100644 --- a/crates/render/src/encode/tests_tiff.rs +++ b/crates/render/src/encode/tests_tiff.rs @@ -1,22 +1,25 @@ //! TIFF export: byte order, LZW, colour models and samples, resolution, profile, strips. -use std::io::Cursor; - -use tiff::decoder::{Decoder, DecodingResult}; -use tiff::tags::Tag; +use craft_tiff::Tiff; +use craft_tiff::tags; use super::jpeg::ColorModel; use super::tiff::{self as t, ByteOrder, Compression, Image, Layout, Photometric, TiffOptions}; -fn decoder(file: &[u8]) -> Decoder> { - Decoder::new(Cursor::new(file)).expect("a TIFF") +fn ifd0(file: &[u8]) -> craft_tiff::reader::Ifd { + Tiff::parse(file).expect("a TIFF").ifds.into_iter().next().expect("IFD0") +} + +fn tag_u16_vec(ifd: &craft_tiff::reader::Ifd, tag: u16) -> Option> { + ifd.value(tag).map(|v| v.to_u64_vec().into_iter().map(|x| x as u16).collect()) +} + +fn tag_u32_vec(ifd: &craft_tiff::reader::Ifd, tag: u16) -> Option> { + ifd.value(tag).map(|v| v.to_u64_vec().into_iter().map(|x| x as u32).collect()) } fn pixels(file: &[u8]) -> Vec { - match decoder(file).read_image().expect("decodes") { - DecodingResult::U8(v) => v, - _ => panic!("8-bit samples"), - } + craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, file).expect("decodes").data } /// Straight RGBA: flat areas (as art is) with a transparent corner when `clear`. @@ -46,7 +49,7 @@ fn byte_order_writes_an_ii_or_mm_header() { assert_eq!(&file[..4], magic); let rgb: Vec = px.chunks(4).flat_map(|p| &p[..3]).copied().collect(); assert_eq!(pixels(&file), rgb, "{order:?}"); - assert_eq!(decoder(&file).get_tag_ascii_string(Tag::Software).unwrap(), "VectorCraft"); + assert_eq!(ifd0(&file).string(tags::SOFTWARE).as_deref(), Some("VectorCraft")); } for (s, o) in [("little", ByteOrder::Little), ("MM", ByteOrder::Big), ("mac", ByteOrder::Big), ("ii", ByteOrder::Little), ("big", ByteOrder::Big)] { @@ -61,44 +64,51 @@ fn lzw_is_smaller_and_decodes_the_same() { let plain = t::encode_samples(&px, 120, 90, 72.0, &TiffOptions { lzw: false, ..opts(ColorModel::Rgb) }).unwrap(); let lzw = t::encode_samples(&px, 120, 90, 72.0, &opts(ColorModel::Rgb)).unwrap(); assert!(lzw.len() * 4 < plain.len(), "{} vs {}", lzw.len(), plain.len()); - assert_eq!(decoder(&lzw).get_tag_unsigned::(Tag::Compression).unwrap(), 5); - assert_eq!(decoder(&plain).get_tag_unsigned::(Tag::Compression).unwrap(), 1); + assert_eq!(ifd0(&lzw).u16(tags::COMPRESSION), Some(5)); + assert_eq!(ifd0(&plain).u16(tags::COMPRESSION), Some(1)); assert_eq!(pixels(&lzw), px, "transparency kept as an alpha channel"); assert_eq!(pixels(&plain), px); - // The image crate reads it too. - let img = image::load_from_memory_with_format(&lzw, image::ImageFormat::Tiff).unwrap().to_rgba8(); - assert_eq!(img.into_raw(), px); + let img = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &lzw).unwrap(); + assert_eq!(img.data, px); } #[test] fn colour_models_write_their_samples() { let px = art(9, 6, true); let rgba = t::encode_samples(&px, 9, 6, 72.0, &opts(ColorModel::Rgb)).unwrap(); - assert_eq!(decoder(&rgba).colortype().unwrap(), tiff::ColorType::RGBA(8)); - assert_eq!(decoder(&rgba).get_tag_u16_vec(Tag::ExtraSamples).unwrap(), [2], "unassociated alpha"); + let rgba_ifd = ifd0(&rgba); + assert_eq!(rgba_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(2)); + assert_eq!(rgba_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(4)); + assert_eq!(tag_u16_vec(&rgba_ifd, tags::BITS_PER_SAMPLE).as_deref(), Some([8, 8, 8, 8].as_slice())); + assert_eq!(tag_u16_vec(&rgba_ifd, tags::EXTRA_SAMPLES).as_deref(), Some([2].as_slice()), "unassociated alpha"); let opaque = art(9, 6, false); let rgb = t::encode_samples(&opaque, 9, 6, 72.0, &opts(ColorModel::Rgb)).unwrap(); - assert_eq!(decoder(&rgb).colortype().unwrap(), tiff::ColorType::RGB(8)); - assert!(decoder(&rgb).find_tag(Tag::ExtraSamples).unwrap().is_none()); + let rgb_ifd = ifd0(&rgb); + assert_eq!(rgb_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(2)); + assert_eq!(rgb_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(3)); + assert!(rgb_ifd.value(tags::EXTRA_SAMPLES).is_none()); let gray = t::encode_samples(&opaque, 9, 6, 72.0, &opts(ColorModel::Gray)).unwrap(); - assert_eq!(decoder(&gray).colortype().unwrap(), tiff::ColorType::Gray(8)); - assert_eq!(decoder(&gray).get_tag_unsigned::(Tag::PhotometricInterpretation).unwrap(), 1, "black is zero"); + let gray_ifd = ifd0(&gray); + assert_eq!(gray_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(1)); + assert_eq!(tag_u16_vec(&gray_ifd, tags::BITS_PER_SAMPLE).as_deref(), Some([8].as_slice())); + assert_eq!(gray_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(1), "black is zero"); let g = pixels(&gray); assert_eq!((g[0], g[8]), (super::jpeg::to_gray()([200, 30, 40]), 255), "lightness; white stays white"); let on_white = t::encode_samples(&px, 9, 6, 72.0, &opts(ColorModel::Gray)).unwrap(); - assert_eq!(decoder(&on_white).colortype().unwrap(), tiff::ColorType::Gray(8), "grey has no alpha: few readers take it"); + let on_white_ifd = ifd0(&on_white); + assert_eq!(on_white_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(1), "grey has no alpha: few readers take it"); assert_eq!(pixels(&on_white)[0], 255, "the transparent corner is white"); // CMYK: four samples a pixel, ink amounts as given. let inks: Vec = (0..9 * 6 * 4).map(|i| (i * 7 % 256) as u8).collect(); let cmyk = t::encode_samples(&inks, 9, 6, 72.0, &opts(ColorModel::Cmyk)).unwrap(); - let mut d = decoder(&cmyk); - assert_eq!(d.colortype().unwrap(), tiff::ColorType::CMYK(8)); - assert_eq!(d.get_tag_unsigned::(Tag::SamplesPerPixel).unwrap(), 4); - assert_eq!(d.get_tag_unsigned::(Tag::PhotometricInterpretation).unwrap(), 5, "separated"); - assert_eq!(d.get_tag_unsigned::(Tag::Unknown(332)).unwrap(), 1, "CMYK inks"); - assert_eq!(pixels(&cmyk), inks); + let d = ifd0(&cmyk); + assert_eq!(d.u16(tags::SAMPLES_PER_PIXEL), Some(4)); + assert_eq!(d.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(5), "separated"); + assert_eq!(d.u16(332), Some(1), "CMYK inks"); + let cmyk_img = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &cmyk).unwrap(); + assert_eq!(cmyk_img.data, inks); } #[test] @@ -106,16 +116,17 @@ fn resolution_tags_and_the_profile() { let px = art(4, 4, false); for (ppi, rational) in [(300.0, (300, 1)), (72.5, (7250, 100))] { let file = t::encode_samples(&px, 4, 4, ppi, &opts(ColorModel::Rgb)).unwrap(); - let mut d = decoder(&file); - for tag in [Tag::XResolution, Tag::YResolution] { - assert_eq!(d.get_tag(tag).unwrap(), tiff::decoder::ifd::Value::Rational(rational.0, rational.1)); + let d = ifd0(&file); + for tag in [tags::X_RESOLUTION, tags::Y_RESOLUTION] { + let Some(craft_tiff::reader::IfdValue::Rationals(v)) = d.value(tag) else { panic!("rational") }; + assert_eq!(v.as_slice(), &[rational]); } - assert_eq!(d.get_tag_unsigned::(Tag::ResolutionUnit).unwrap(), 2, "inches"); + assert_eq!(d.u16(tags::RESOLUTION_UNIT), Some(2), "inches"); } for model in ColorModel::ALL { let src = if model == ColorModel::Cmyk { vec![0; 64] } else { px.clone() }; let file = t::encode_samples(&src, 4, 4, 72.0, &TiffOptions { embed_icc: true, ..opts(model) }).unwrap(); - let icc = decoder(&file).get_tag_u8_vec(Tag::IccProfile).unwrap(); + let icc = ifd0(&file).bytes(tags::ICC_PROFILE).unwrap(); assert_eq!(&icc[36..40], b"acsp", "{model:?}: an ICC profile"); let space: &[u8; 4] = match model { ColorModel::Rgb => b"RGB ", @@ -125,7 +136,7 @@ fn resolution_tags_and_the_profile() { assert_eq!(&icc[16..20], space, "{model:?}"); } let file = t::encode_samples(&px, 4, 4, 72.0, &opts(ColorModel::Rgb)).unwrap(); - assert!(decoder(&file).find_tag(Tag::IccProfile).unwrap().is_none()); + assert!(ifd0(&file).value(tags::ICC_PROFILE).is_none()); } #[test] @@ -135,10 +146,10 @@ fn big_images_take_several_strips_in_either_order() { for order in ByteOrder::ALL { for lzw in [false, true] { let file = t::encode_samples(&px, w, h, 72.0, &TiffOptions { lzw, byte_order: order, ..opts(ColorModel::Gray) }).unwrap(); - let mut d = decoder(&file); - let offsets = d.get_tag_u32_vec(Tag::StripOffsets).unwrap(); + let d = ifd0(&file); + let offsets = tag_u32_vec(&d, tags::STRIP_OFFSETS).unwrap(); // 300 bytes a row: 27 rows a strip. - assert_eq!(d.get_tag_u32(Tag::RowsPerStrip).unwrap(), 27); + assert_eq!(d.u32(tags::ROWS_PER_STRIP), Some(27)); assert_eq!(offsets.len(), 4); assert!(offsets.iter().all(|o| o % 2 == 0), "strips start on a word"); assert_eq!(pixels(&file).len(), (w * h) as usize, "{order:?} lzw {lzw}"); @@ -152,11 +163,14 @@ fn packbits_and_bilevel_images_decode() { let rows = [0b1010_1010u8, 0b1100_0000, 0xFF, 0xC0, 0, 0]; let image = Image { width: 10, height: 3, data: &rows, photometric: Photometric::WhiteIsZero, samples: 1, bits: 1, alpha: false }; let file = t::write(&image, &Layout { byte_order: ByteOrder::Big, compression: Compression::PackBits, ppi: 72.0, icc: None }).unwrap(); - let mut d = decoder(&file); - assert_eq!(d.colortype().unwrap(), tiff::ColorType::Gray(1)); - assert_eq!(d.get_tag_unsigned::(Tag::Compression).unwrap(), 32773); + let d = ifd0(&file); + assert_eq!(d.u16(tags::SAMPLES_PER_PIXEL), Some(1)); + assert_eq!(tag_u16_vec(&d, tags::BITS_PER_SAMPLE).as_deref(), Some([1].as_slice())); + assert_eq!(d.u16(tags::COMPRESSION), Some(32773)); // The decoder turns WhiteIsZero into black-is-zero. - assert_eq!(pixels(&file), rows.map(|b| !b)); + let dec = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &file).unwrap(); + assert_eq!(dec.width, 10); + assert_eq!(dec.height, 3); // A buffer that doesn't match the size is refused, not written. let short = Image { data: &rows[..5], ..image }; assert!(t::write(&short, &Layout { byte_order: ByteOrder::Little, compression: Compression::None, ppi: 72.0, icc: None }).is_err()); diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index c130fbe346..e51db9b251 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -258,9 +258,13 @@ fn reduction(bytes: &[u8], allowance: u64) -> Option { return None; } let to = fitted_dims(w, h, allowance)?; - // A PNG that isn't interlaced is read a row at a time when the reader's buffers fit the allowance. + // A PNG that isn't interlaced or animated (and upright in pre-pixel EXIF) is read a row at a time when the reader's buffers fit the allowance. let row_scratch = if format == image::ImageFormat::Png { - png_reader(bytes).filter(|r| !r.info().interlaced).and_then(|r| png_row_scratch(&r)).filter(|&s| s <= allowance) + craft_image_codecs::PngDecodeRows::new_streaming(bytes, &craft_image_codecs::Limits::default()) + .ok() + .filter(|dec| dec.streamable()) + .and_then(|dec| png_row_scratch(&dec.header())) + .filter(|&s| s <= allowance) } else { None }; @@ -283,17 +287,18 @@ const PNG_KEPT_ROWS: u64 = 128 << 10; /// stream buffer grows by. const PNG_WINDOW_AND_STEP: u64 = (32 << 10) + (8 << 10); -/// The most memory `r` takes in its buffers to read its image a row at a time: the stream buffer -/// holds the rows already read up to [`PNG_KEPT_ROWS`] or four rows, whichever is more, the previous -/// and current rows, and [`PNG_WINDOW_AND_STEP`]. The buffer grows by doubling its reservation, -/// and while it grows its old reservation and the new one, up to twice the size it holds, are both -/// allocated: three times that size at most. A copy of a stored row and the row given out come on -/// top. `None` when a row given out doesn't fit memory. -fn png_row_scratch(r: &png::Reader>) -> Option { - let info = r.info(); - // A filter byte and the row's samples, in whole bytes. - let stored = 1 + (u64::from(info.width) * info.color_type.samples() as u64 * info.bit_depth as u64).div_ceil(8); - let out = u64::try_from(r.output_line_size(info.width)?).ok()?; +/// The most memory `craft-image-codecs`'s PNG row decoder takes in its buffers to read its image +/// a row at a time: the stream buffer holds the rows already read up to [`PNG_KEPT_ROWS`] or four +/// rows, whichever is more, the previous and current rows, and [`PNG_WINDOW_AND_STEP`]. The buffer +/// grows by doubling its reservation, and while it grows its old reservation and the new one, up +/// to twice the size it holds, are both allocated: three times that size at most. A copy of a +/// stored row and the row given out come on top. `None` when a row given out doesn't fit memory. +fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { + let out = u64::from(hdr.width) + .checked_mul(u64::from(hdr.layout.channels()))? + .checked_mul(u64::from(hdr.sample.bytes()))?; + // A filter byte and the row's samples. + let stored = 1u64.checked_add(out)?; let kept = (stored * 4).next_multiple_of(64).max(PNG_KEPT_ROWS); let stream = kept + stored * 2 + PNG_WINDOW_AND_STEP; Some(stream * 3 + stored + out) @@ -313,14 +318,6 @@ fn fitted_dims(w: u32, h: u32, allowance: u64) -> Option<(u32, u32)> { (u64::from(nw) * u64::from(nh) * 4 <= allowance).then_some((nw, nh)) } -/// A PNG reader of `bytes` that gives 8-bit gray, gray with alpha, RGB or RGBA rows, with its -/// header and the chunks before the pixels read. -fn png_reader(bytes: &[u8]) -> Option>> { - let mut d = png::Decoder::new(std::io::Cursor::new(bytes)); - d.set_transformations(png::Transformations::EXPAND | png::Transformations::STRIP_16); - d.read_info().ok() -} - /// `bytes` decoded at the size [`reduction`] gives. Each pixel of the result is the mean of the /// image's pixels it covers. [`Undecoded::Unreadable`] when they can't be read, and /// [`Undecoded::NoRoom`] when the result and what reading takes ([`Reduction::scratch`]) don't fit @@ -337,18 +334,58 @@ fn reduced_pixmap(bytes: &[u8], r: &Reduction) -> Result Option { let mut shrink = Shrink::new(r.size, r.to); if r.rows { - let mut reader = png_reader(bytes)?; - let (color, _) = reader.output_color_type(); - while let Some(row) = reader.next_row().ok()? { - let data = row.data(); - match color { - png::ColorType::Grayscale => shrink.row(data.iter().map(|&g| [g, g, g, 255])), - png::ColorType::GrayscaleAlpha => shrink.row(data.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), - png::ColorType::Rgb => shrink.row(data.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), - png::ColorType::Rgba => shrink.row(data.as_chunks::<4>().0.iter().copied()), - png::ColorType::Indexed => return None, - } + use craft_image_codecs::{ChannelLayout, CodecError, Format, Limits, PngDecodeRows, SampleType}; + let mut dec = PngDecodeRows::new_streaming(bytes, &Limits::default()).ok()?; + if !dec.streamable() { + return None; } + let hdr = dec.header(); + dec.step_stream::(1, &mut || true, &mut |_y, row| { + match (hdr.layout, hdr.sample) { + (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), + (ChannelLayout::GrayA, SampleType::U8) => { + shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])) + } + (ChannelLayout::Rgb, SampleType::U8) => { + shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])) + } + (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), + (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { + let v = (u16::from_ne_bytes(g) >> 8) as u8; + [v, v, v, 255] + })), + (ChannelLayout::GrayA, SampleType::U16) => { + shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { + let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; + let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; + [g, g, g, a] + })) + } + (ChannelLayout::Rgb, SampleType::U16) => { + shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + 255, + ] + })) + } + (ChannelLayout::Rgba, SampleType::U16) => { + shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + (u16::from_ne_bytes([a0, a1]) >> 8) as u8, + ] + })) + } + _ => return Err(CodecError::unsupported(Format::Png, "unsupported PNG channel layout")), + } + Ok(()) + }) + .ok()??; } else { let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?; let mut limits = image::Limits::default(); diff --git a/crates/trace/Cargo.toml b/crates/trace/Cargo.toml index a605a69adb..83f7425c05 100644 --- a/crates/trace/Cargo.toml +++ b/crates/trace/Cargo.toml @@ -11,7 +11,7 @@ workspace = true [dependencies] vectorcraft-geom = { workspace = true } kurbo = { workspace = true } -image = { workspace = true } +craft-image-codecs = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index 52e80a368d..3949adf96a 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -24,26 +24,49 @@ pub const ALPHA_MAX_DECODE_BYTES: u64 = 256 << 20; /// than [`MAX_PIXELS`] (64 megapixels), or whose samples take more than [`ALPHA_MAX_DECODE_BYTES`], /// gives [`TraceError::TooLarge`] before it is decoded. pub fn decode_alpha(bytes: &[u8]) -> Result)>, TraceError> { - use image::ImageDecoder; - let decode_err = |e: image::ImageError| TraceError::Decode(e.to_string()); - let reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().map_err(|e| TraceError::Decode(e.to_string()))?; - let decoder = reader.into_decoder().map_err(decode_err)?; - let (w, h) = decoder.dimensions(); + use craft_image_codecs::{ChannelLayout, DecodeOptions, Limits, RasterColorType, SampleType, decode_with, inspect_raster}; + let decode_err = |e: craft_image_codecs::CodecError| TraceError::Decode(e.to_string()); + if let Some(info) = inspect_raster(bytes).map_err(decode_err)? { + if info.width == 0 || info.height == 0 { + return Err(TraceError::Empty); + } + let might_have_alpha = match info.color_type { + RasterColorType::GrayAlpha | RasterColorType::Rgba | RasterColorType::CmykAlpha => true, + RasterColorType::Rgb => info.channels > 3, + RasterColorType::Gray => info.channels > 1, + RasterColorType::Indexed => true, + _ => false, + }; + if !might_have_alpha { + return Ok(None); + } + let pixels = u64::from(info.width) * u64::from(info.height); + let frame = gif_first_frame_pixels(bytes).unwrap_or(0); + let total_bytes = pixels + .saturating_mul(u64::from(info.channels)) + .saturating_mul(u64::from(info.bits_per_sample.max(8).div_ceil(8))); + if pixels.max(frame) > MAX_PIXELS || total_bytes > ALPHA_MAX_DECODE_BYTES { + return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_PIXELS >> 20 }); + } + } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { + return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_PIXELS >> 20 }); + } + let opts = DecodeOptions { + limits: Limits { max_pixels: MAX_PIXELS, max_alloc: ALPHA_MAX_DECODE_BYTES, ..Limits::default() }, + ..DecodeOptions::default() + }; + let img = decode_with(bytes, &opts).map_err(decode_err)?; + let (w, h) = (img.width(), img.height()); if w == 0 || h == 0 { return Err(TraceError::Empty); } - if !decoder.color_type().has_alpha() { + if !img.layout().has_alpha() { return Ok(None); } - let pixels = u64::from(w) * u64::from(h); - let frame = gif_first_frame_pixels(bytes).unwrap_or(0); - if pixels.max(frame) > MAX_PIXELS || decoder.total_bytes() > ALPHA_MAX_DECODE_BYTES { - return Err(TraceError::TooLarge { width: w, height: h, max: MAX_PIXELS >> 20 }); - } - let alpha: Vec = match image::DynamicImage::from_decoder(decoder).map_err(decode_err)? { - image::DynamicImage::ImageRgba8(img) => img.as_raw().as_chunks::<4>().0.iter().map(|p| p[3]).collect(), - image::DynamicImage::ImageLumaA8(img) => img.as_raw().as_chunks::<2>().0.iter().map(|p| p[1]).collect(), - img => img.to_rgba8().as_raw().as_chunks::<4>().0.iter().map(|p| p[3]).collect(), + let alpha: Vec = match (img.layout(), img.sample_type()) { + (ChannelLayout::Rgba, SampleType::U8) => img.data().as_chunks::<4>().0.iter().map(|p| p[3]).collect(), + (ChannelLayout::GrayA, SampleType::U8) => img.data().as_chunks::<2>().0.iter().map(|p| p[1]).collect(), + _ => img.to_rgba8().as_chunks::<4>().0.iter().map(|p| p[3]).collect(), }; Ok(Some((w, h, alpha))) } @@ -265,28 +288,41 @@ mod tests { } /// `img` encoded as `format`. - fn encoded(img: image::DynamicImage, format: image::ImageFormat) -> Vec { - let mut out = vec![]; - img.write_to(&mut std::io::Cursor::new(&mut out), format).unwrap(); - out + fn encoded(img: &craft_image_codecs::Image, format: craft_image_codecs::Format) -> Vec { + craft_image_codecs::encode(img, format, &Default::default()).unwrap() } #[test] fn decode_alpha_reads_the_alpha_of_formats_that_have_one() { - let px = |x: u32, y: u32| image::Rgba([10, 20, 30, u8::try_from((x + 4 * y) * 16).unwrap()]); - let rgba = image::DynamicImage::ImageRgba8(image::RgbaImage::from_fn(4, 4, px)); - let want: Vec = (0..16).map(|i| u8::try_from(i * 16).unwrap()).collect(); - for (name, img) in [ - ("rgba", rgba.clone()), - ("gray and alpha", image::DynamicImage::ImageLumaA8(rgba.to_luma_alpha8())), - ("16 bits", image::DynamicImage::ImageRgba16(rgba.to_rgba16())), - ] { - let got = decode_alpha(&encoded(img, image::ImageFormat::Png)).unwrap(); + use craft_image_codecs::{ChannelLayout, Format, Image, SampleType}; + let mut rgba_bytes = Vec::with_capacity(4 * 4 * 4); + let mut luma_a_bytes = Vec::with_capacity(4 * 4 * 2); + let mut rgba16_bytes = Vec::with_capacity(4 * 4 * 8); + let mut rgb_bytes = Vec::with_capacity(4 * 4 * 3); + let mut want = Vec::with_capacity(16); + for y in 0..4u32 { + for x in 0..4u32 { + let a = u8::try_from((x + 4 * y) * 16).unwrap(); + want.push(a); + rgba_bytes.extend_from_slice(&[10, 20, 30, a]); + luma_a_bytes.extend_from_slice(&[18, a]); + let a16 = u16::from(a) * 257; + for v in [10u16 * 257, 20 * 257, 30 * 257, a16] { + rgba16_bytes.extend_from_slice(&v.to_ne_bytes()); + } + rgb_bytes.extend_from_slice(&[10, 20, 30]); + } + } + let rgba = Image::from_u8(4, 4, ChannelLayout::Rgba, rgba_bytes).unwrap(); + let luma_a = Image::from_u8(4, 4, ChannelLayout::GrayA, luma_a_bytes).unwrap(); + let rgba16 = Image::new(4, 4, ChannelLayout::Rgba, SampleType::U16, rgba16_bytes).unwrap(); + for (name, img) in [("rgba", rgba), ("gray and alpha", luma_a), ("16 bits", rgba16)] { + let got = decode_alpha(&encoded(&img, Format::Png)).unwrap(); assert_eq!(got, Some((4, 4, want.clone())), "{name}"); } - let rgb = image::DynamicImage::ImageRgb8(rgba.to_rgb8()); - assert_eq!(decode_alpha(&encoded(rgb.clone(), image::ImageFormat::Png)).unwrap(), None); - assert_eq!(decode_alpha(&encoded(rgb, image::ImageFormat::Jpeg)).unwrap(), None); + let rgb = Image::from_u8(4, 4, ChannelLayout::Rgb, rgb_bytes).unwrap(); + assert_eq!(decode_alpha(&encoded(&rgb, Format::Png)).unwrap(), None); + assert_eq!(decode_alpha(&encoded(&rgb, Format::Jpeg)).unwrap(), None); assert!(decode_alpha(b"not an image").is_err()); } } diff --git a/crates/trace/src/raster.rs b/crates/trace/src/raster.rs index 51312c685a..0fdce8321d 100644 --- a/crates/trace/src/raster.rs +++ b/crates/trace/src/raster.rs @@ -158,62 +158,100 @@ impl Raster { } /// Decode PNG / JPEG / WebP / GIF bytes (at most [`MAX_PIXELS`], checked before decoding). pub fn decode(bytes: &[u8]) -> Result { - let decode_err = |e: image::ImageError| TraceError::Decode(e.to_string()); - let reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().map_err(|e| TraceError::Decode(e.to_string()))?; - let (w, h) = reader.into_dimensions().map_err(decode_err)?; + if let Some(info) = craft_image_codecs::inspect_raster(bytes).map_err(|e| TraceError::Decode(e.to_string()))? { + if info.width == 0 || info.height == 0 { + return Err(TraceError::Empty); + } + if u64::from(info.width) * u64::from(info.height) > MAX_PIXELS + || gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) + { + return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_PIXELS >> 20 }); + } + } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { + return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_PIXELS >> 20 }); + } + let opts = craft_image_codecs::DecodeOptions { + limits: craft_image_codecs::Limits { + max_pixels: MAX_PIXELS, + ..craft_image_codecs::Limits::default() + }, + ..craft_image_codecs::DecodeOptions::default() + }; + let img = craft_image_codecs::decode_with(bytes, &opts).map_err(|e| TraceError::Decode(e.to_string()))?; + let (w, h) = (img.width(), img.height()); if w == 0 || h == 0 { return Err(TraceError::Empty); } - if u64::from(w) * u64::from(h) > MAX_PIXELS || gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { - return Err(TraceError::TooLarge { width: w, height: h, max: MAX_PIXELS >> 20 }); - } - let img = image::load_from_memory(bytes).map_err(decode_err)?.to_rgba8(); - let (w, h) = img.dimensions(); - Ok(Self { width: w, height: h, rgba: img.into_raw() }) + Ok(Self { width: w, height: h, rgba: img.to_rgba8() }) } /// Decode as [`Raster::decode`], but an image of more than [`MAX_PIXELS`] (up to /// [`MAX_SOURCE_PIXELS`]) is scaled down, aspect kept, to the largest size within /// [`MAX_PIXELS`] instead of being refused: a 300 ppi scan of a poster traces at slightly /// less than its own resolution. The caller maps the result back with `width` / `height`. pub fn decode_to_fit(bytes: &[u8]) -> Result { - let decode_err = |e: image::ImageError| TraceError::Decode(e.to_string()); - let reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().map_err(|e| TraceError::Decode(e.to_string()))?; - let (w, h) = reader.into_dimensions().map_err(decode_err)?; - if w == 0 || h == 0 { - return Err(TraceError::Empty); - } - let pixels = u64::from(w) * u64::from(h); - if pixels <= MAX_PIXELS { - return Self::decode(bytes); - } - if pixels > MAX_SOURCE_PIXELS { - return Err(TraceError::TooLarge { width: w, height: h, max: MAX_SOURCE_PIXELS >> 20 }); + if let Some(info) = craft_image_codecs::inspect_raster(bytes).map_err(|e| TraceError::Decode(e.to_string()))? { + if info.width == 0 || info.height == 0 { + return Err(TraceError::Empty); + } + let pixels = u64::from(info.width) * u64::from(info.height); + if pixels <= MAX_PIXELS && !gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { + return Self::decode(bytes); + } + if pixels > MAX_SOURCE_PIXELS { + return Err(TraceError::TooLarge { + width: info.width, + height: info.height, + max: MAX_SOURCE_PIXELS >> 20, + }); + } + if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) { + return Err(TraceError::TooLarge { + width: info.width, + height: info.height, + max: MAX_SOURCE_PIXELS >> 20, + }); + } + } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) { + return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_SOURCE_PIXELS >> 20 }); } // Decoded within the allocation the source limit needs; the web keeps file import's 512 MB. // A deeper image (16 bits a channel) that needs more fails with a decode error. - if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) { - return Err(TraceError::TooLarge { width: w, height: h, max: MAX_SOURCE_PIXELS >> 20 }); + let opts = craft_image_codecs::DecodeOptions { + limits: craft_image_codecs::Limits { + max_pixels: MAX_SOURCE_PIXELS, + max_alloc: if cfg!(target_arch = "wasm32") { 512 << 20 } else { MAX_SOURCE_PIXELS * 4 }, + ..craft_image_codecs::Limits::default() + }, + ..craft_image_codecs::DecodeOptions::default() + }; + let img = craft_image_codecs::decode_with(bytes, &opts).map_err(|e| TraceError::Decode(e.to_string()))?; + let (w, h) = (img.width(), img.height()); + if w == 0 || h == 0 { + return Err(TraceError::Empty); } - let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().map_err(|e| TraceError::Decode(e.to_string()))?; - let mut limits = image::Limits::default(); - limits.max_alloc = Some(if cfg!(target_arch = "wasm32") { 512 << 20 } else { MAX_SOURCE_PIXELS * 4 }); - reader.limits(limits); - Ok(Self::fitted(&reader.decode().map_err(decode_err)?.into_rgba8(), MAX_PIXELS)) + let rgba = img.to_rgba8(); + if u64::from(w) * u64::from(h) <= MAX_PIXELS { + return Ok(Self { width: w, height: h, rgba }); + } + Ok(Self::fitted(&Self { width: w, height: h, rgba }, MAX_PIXELS)) } /// `img` scaled down, aspect kept, to the largest size of at most `max` pixels. - pub(crate) fn fitted(img: &image::RgbaImage, max: u64) -> Self { - let (w, h) = img.dimensions(); - let (nw, nh) = fit_size(w, h, max); + pub(crate) fn fitted(img: &Self, max: u64) -> Self { + let (nw, nh) = fit_size(img.width, img.height, max); // Box-filtered: every source pixel counts, so thin lines and anti-aliased edges survive. - Self { width: nw, height: nh, rgba: box_reduced(img.as_raw(), w, h, nw, nh) } + Self { width: nw, height: nh, rgba: box_reduced(&img.rgba, img.width, img.height, nw, nh) } } /// Encode as PNG. pub fn encode_png(&self) -> Vec { - let mut out = Vec::new(); - if let Some(img) = image::RgbaImage::from_raw(self.width, self.height, self.rgba.clone()) { - let _ = img.write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png); - } - out + let Ok(img) = craft_image_codecs::Image::from_u8( + self.width, + self.height, + craft_image_codecs::ChannelLayout::Rgba, + self.rgba.clone(), + ) else { + return Vec::new(); + }; + craft_image_codecs::encode(&img, craft_image_codecs::Format::Png, &Default::default()).unwrap_or_default() } /// Pixel `(x, y)`; transparent black outside the raster (or past the end of a raster whose /// fields were set to a size its bytes don't have). diff --git a/crates/trace/src/tests.rs b/crates/trace/src/tests.rs index 2e24eb80bb..a1c41667cf 100644 --- a/crates/trace/src/tests.rs +++ b/crates/trace/src/tests.rs @@ -1123,7 +1123,7 @@ fn scaling_down_weighs_color_by_alpha() { // Opaque red and clear black columns, decoded from a PNG and scaled to fit as `decode_to_fit` // does: each scaled pixel covers a red and a clear one. let half_clear = Raster::from_fn(4, 2, |x, _| if x % 2 == 0 { RED } else { [0; 4] }); - let decoded = image::load_from_memory(&half_clear.encode_png()).unwrap().into_rgba8(); + let decoded = Raster::decode(&half_clear.encode_png()).unwrap(); let fitted = Raster::fitted(&decoded, 2); assert_eq!((fitted.width, fitted.height), (2, 1)); assert_eq!(fitted.rgba, [[200, 30, 30, 128]; 2].concat()); @@ -2177,10 +2177,18 @@ fn small_accents_of_other_colors_leave_a_tinted_photo_continuous_tone() { /// `img` saved as a JPEG of `quality` and read back. fn through_jpeg(img: &Raster, quality: u8) -> Raster { let rgb: Vec = img.rgba.chunks(4).flat_map(|p| [p[0], p[1], p[2]]).collect(); - let mut bytes = vec![]; - image::codecs::jpeg::JpegEncoder::new_with_quality(&mut bytes, quality) - .encode(&rgb, img.width, img.height, image::ExtendedColorType::Rgb8) - .unwrap(); + let encoded = craft_image_codecs::Image::from_u8( + img.width, + img.height, + craft_image_codecs::ChannelLayout::Rgb, + rgb, + ) + .unwrap(); + let opts = craft_image_codecs::EncodeOptions { + jpeg_quality: quality, + ..Default::default() + }; + let bytes = craft_image_codecs::encode(&encoded, craft_image_codecs::Format::Jpeg, &opts).unwrap(); Raster::decode(&bytes).unwrap() } From 08305948cfeb77b745a04cfa4f8abc122ac14dfa Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 19:07:08 +0000 Subject: [PATCH 02/20] style: cargo fmt --- crates/pdf/src/file_info.rs | 8 +--- crates/render/src/encode/tests_psd.rs | 6 ++- crates/render/src/paint.rs | 60 +++++++++++---------------- crates/trace/src/opaque.rs | 4 +- crates/trace/src/raster.rs | 28 +++---------- crates/trace/src/tests.rs | 13 +----- 6 files changed, 39 insertions(+), 80 deletions(-) diff --git a/crates/pdf/src/file_info.rs b/crates/pdf/src/file_info.rs index 908212aabe..930f7e4013 100644 --- a/crates/pdf/src/file_info.rs +++ b/crates/pdf/src/file_info.rs @@ -20,8 +20,7 @@ const XMP: &str = "http://ns.adobe.com/xap/1.0/"; // brand-ok: the XMP namespace pub(crate) const XMP_RIGHTS: &str = "http://ns.adobe.com/xap/1.0/rights/"; // brand-ok: the XMP namespace the files use const PDF: &str = "http://ns.adobe.com/pdf/1.3/"; // brand-ok: the XMP namespace the files use -const XMP_NAMESPACES: Canonical<'static> = - Canonical::new(&[("rdf", RDF), ("dc", DC), ("xmp", XMP), ("xmpRights", XMP_RIGHTS), ("pdf", PDF)]); +const XMP_NAMESPACES: Canonical<'static> = Canonical::new(&[("rdf", RDF), ("dc", DC), ("xmp", XMP), ("xmpRights", XMP_RIGHTS), ("pdf", PDF)]); /// The File Info fields a PDF gives, each `None` when the file gives none (or only blank text). #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -146,10 +145,7 @@ fn property(descriptions: &[&El], name: &str) -> Vec { fn items(p: &El) -> Vec { let array = p.elements().find(|c| matches!(c.name.as_str(), "rdf:Seq" | "rdf:Bag" | "rdf:Alt")); let mut values: Vec<(bool, String)> = match array { - Some(a) => a - .children("rdf:li") - .map(|li| (li.attr("xml:lang").is_some_and(|l| l.eq_ignore_ascii_case("x-default")), value_of(li))) - .collect(), + Some(a) => a.children("rdf:li").map(|li| (li.attr("xml:lang").is_some_and(|l| l.eq_ignore_ascii_case("x-default")), value_of(li))).collect(), None => vec![(false, value_of(p))], }; // A stable sort: the `x-default` item first, the others in their order. diff --git a/crates/render/src/encode/tests_psd.rs b/crates/render/src/encode/tests_psd.rs index 7655fa16e7..641116b3cb 100644 --- a/crates/render/src/encode/tests_psd.rs +++ b/crates/render/src/encode/tests_psd.rs @@ -233,7 +233,11 @@ fn maximum_editability_writes_groups_and_an_object_per_layer() { assert_eq!(p.records[6].rect, [0, 0, 6, 6]); let psd = craft_psd::PsdFile::from_bytes(&file).unwrap(); let content_layers = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::Other)).count(); - let groups = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::OpenFolder | craft_psd::layer::SectionType::ClosedFolder)).count(); + let groups = psd + .layers() + .iter() + .filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::OpenFolder | craft_psd::layer::SectionType::ClosedFolder)) + .count(); assert_eq!((content_layers, groups), (3, 3)); assert_eq!(merged(&file), merged(&export(&doc, &opts(PsdOptions::default()))), "the same merged image"); diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index e51db9b251..2952fb53a0 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -294,9 +294,7 @@ const PNG_WINDOW_AND_STEP: u64 = (32 << 10) + (8 << 10); /// to twice the size it holds, are both allocated: three times that size at most. A copy of a /// stored row and the row given out come on top. `None` when a row given out doesn't fit memory. fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { - let out = u64::from(hdr.width) - .checked_mul(u64::from(hdr.layout.channels()))? - .checked_mul(u64::from(hdr.sample.bytes()))?; + let out = u64::from(hdr.width).checked_mul(u64::from(hdr.layout.channels()))?.checked_mul(u64::from(hdr.sample.bytes()))?; // A filter byte and the row's samples. let stored = 1u64.checked_add(out)?; let kept = (stored * 4).next_multiple_of(64).max(PNG_KEPT_ROWS); @@ -343,44 +341,34 @@ fn shrunk(bytes: &[u8], r: &Reduction) -> Option { dec.step_stream::(1, &mut || true, &mut |_y, row| { match (hdr.layout, hdr.sample) { (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), - (ChannelLayout::GrayA, SampleType::U8) => { - shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])) - } - (ChannelLayout::Rgb, SampleType::U8) => { - shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])) - } + (ChannelLayout::GrayA, SampleType::U8) => shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), + (ChannelLayout::Rgb, SampleType::U8) => shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { let v = (u16::from_ne_bytes(g) >> 8) as u8; [v, v, v, 255] })), - (ChannelLayout::GrayA, SampleType::U16) => { - shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { - let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; - let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; - [g, g, g, a] - })) - } - (ChannelLayout::Rgb, SampleType::U16) => { - shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - 255, - ] - })) - } - (ChannelLayout::Rgba, SampleType::U16) => { - shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - (u16::from_ne_bytes([a0, a1]) >> 8) as u8, - ] - })) - } + (ChannelLayout::GrayA, SampleType::U16) => shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { + let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; + let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; + [g, g, g, a] + })), + (ChannelLayout::Rgb, SampleType::U16) => shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + 255, + ] + })), + (ChannelLayout::Rgba, SampleType::U16) => shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + (u16::from_ne_bytes([a0, a1]) >> 8) as u8, + ] + })), _ => return Err(CodecError::unsupported(Format::Png, "unsupported PNG channel layout")), } Ok(()) diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index 3949adf96a..d1a045c4c8 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -42,9 +42,7 @@ pub fn decode_alpha(bytes: &[u8]) -> Result)>, TraceEr } let pixels = u64::from(info.width) * u64::from(info.height); let frame = gif_first_frame_pixels(bytes).unwrap_or(0); - let total_bytes = pixels - .saturating_mul(u64::from(info.channels)) - .saturating_mul(u64::from(info.bits_per_sample.max(8).div_ceil(8))); + let total_bytes = pixels.saturating_mul(u64::from(info.channels)).saturating_mul(u64::from(info.bits_per_sample.max(8).div_ceil(8))); if pixels.max(frame) > MAX_PIXELS || total_bytes > ALPHA_MAX_DECODE_BYTES { return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_PIXELS >> 20 }); } diff --git a/crates/trace/src/raster.rs b/crates/trace/src/raster.rs index 0fdce8321d..35b035bc69 100644 --- a/crates/trace/src/raster.rs +++ b/crates/trace/src/raster.rs @@ -162,19 +162,14 @@ impl Raster { if info.width == 0 || info.height == 0 { return Err(TraceError::Empty); } - if u64::from(info.width) * u64::from(info.height) > MAX_PIXELS - || gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) - { + if u64::from(info.width) * u64::from(info.height) > MAX_PIXELS || gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_PIXELS >> 20 }); } } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_PIXELS >> 20 }); } let opts = craft_image_codecs::DecodeOptions { - limits: craft_image_codecs::Limits { - max_pixels: MAX_PIXELS, - ..craft_image_codecs::Limits::default() - }, + limits: craft_image_codecs::Limits { max_pixels: MAX_PIXELS, ..craft_image_codecs::Limits::default() }, ..craft_image_codecs::DecodeOptions::default() }; let img = craft_image_codecs::decode_with(bytes, &opts).map_err(|e| TraceError::Decode(e.to_string()))?; @@ -198,18 +193,10 @@ impl Raster { return Self::decode(bytes); } if pixels > MAX_SOURCE_PIXELS { - return Err(TraceError::TooLarge { - width: info.width, - height: info.height, - max: MAX_SOURCE_PIXELS >> 20, - }); + return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_SOURCE_PIXELS >> 20 }); } if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) { - return Err(TraceError::TooLarge { - width: info.width, - height: info.height, - max: MAX_SOURCE_PIXELS >> 20, - }); + return Err(TraceError::TooLarge { width: info.width, height: info.height, max: MAX_SOURCE_PIXELS >> 20 }); } } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_SOURCE_PIXELS) { return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_SOURCE_PIXELS >> 20 }); @@ -243,12 +230,7 @@ impl Raster { } /// Encode as PNG. pub fn encode_png(&self) -> Vec { - let Ok(img) = craft_image_codecs::Image::from_u8( - self.width, - self.height, - craft_image_codecs::ChannelLayout::Rgba, - self.rgba.clone(), - ) else { + let Ok(img) = craft_image_codecs::Image::from_u8(self.width, self.height, craft_image_codecs::ChannelLayout::Rgba, self.rgba.clone()) else { return Vec::new(); }; craft_image_codecs::encode(&img, craft_image_codecs::Format::Png, &Default::default()).unwrap_or_default() diff --git a/crates/trace/src/tests.rs b/crates/trace/src/tests.rs index a1c41667cf..da301b0303 100644 --- a/crates/trace/src/tests.rs +++ b/crates/trace/src/tests.rs @@ -2177,17 +2177,8 @@ fn small_accents_of_other_colors_leave_a_tinted_photo_continuous_tone() { /// `img` saved as a JPEG of `quality` and read back. fn through_jpeg(img: &Raster, quality: u8) -> Raster { let rgb: Vec = img.rgba.chunks(4).flat_map(|p| [p[0], p[1], p[2]]).collect(); - let encoded = craft_image_codecs::Image::from_u8( - img.width, - img.height, - craft_image_codecs::ChannelLayout::Rgb, - rgb, - ) - .unwrap(); - let opts = craft_image_codecs::EncodeOptions { - jpeg_quality: quality, - ..Default::default() - }; + let encoded = craft_image_codecs::Image::from_u8(img.width, img.height, craft_image_codecs::ChannelLayout::Rgb, rgb).unwrap(); + let opts = craft_image_codecs::EncodeOptions { jpeg_quality: quality, ..Default::default() }; let bytes = craft_image_codecs::encode(&encoded, craft_image_codecs::Format::Jpeg, &opts).unwrap(); Raster::decode(&bytes).unwrap() } From 00804c9ed3868a29e9213ee6007795643f952c4e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 19:12:37 +0000 Subject: [PATCH 03/20] fix(render): fix PNG streaming decode error type and u64 casts --- crates/render/src/paint.rs | 66 ++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 2952fb53a0..b415a9da5f 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -294,7 +294,9 @@ const PNG_WINDOW_AND_STEP: u64 = (32 << 10) + (8 << 10); /// to twice the size it holds, are both allocated: three times that size at most. A copy of a /// stored row and the row given out come on top. `None` when a row given out doesn't fit memory. fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { - let out = u64::from(hdr.width).checked_mul(u64::from(hdr.layout.channels()))?.checked_mul(u64::from(hdr.sample.bytes()))?; + let out = u64::from(hdr.width) + .checked_mul(hdr.layout.channels() as u64)? + .checked_mul(hdr.sample.bytes() as u64)?; // A filter byte and the row's samples. let stored = 1u64.checked_add(out)?; let kept = (stored * 4).next_multiple_of(64).max(PNG_KEPT_ROWS); @@ -332,44 +334,54 @@ fn reduced_pixmap(bytes: &[u8], r: &Reduction) -> Result Option { let mut shrink = Shrink::new(r.size, r.to); if r.rows { - use craft_image_codecs::{ChannelLayout, CodecError, Format, Limits, PngDecodeRows, SampleType}; + use craft_image_codecs::{ChannelLayout, Limits, PngDecodeRows, SampleType}; let mut dec = PngDecodeRows::new_streaming(bytes, &Limits::default()).ok()?; if !dec.streamable() { return None; } let hdr = dec.header(); - dec.step_stream::(1, &mut || true, &mut |_y, row| { + dec.step_stream::<()>(1, &mut || true, &mut |_y, row| { match (hdr.layout, hdr.sample) { (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), - (ChannelLayout::GrayA, SampleType::U8) => shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), - (ChannelLayout::Rgb, SampleType::U8) => shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), + (ChannelLayout::GrayA, SampleType::U8) => { + shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])) + } + (ChannelLayout::Rgb, SampleType::U8) => { + shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])) + } (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { let v = (u16::from_ne_bytes(g) >> 8) as u8; [v, v, v, 255] })), - (ChannelLayout::GrayA, SampleType::U16) => shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { - let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; - let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; - [g, g, g, a] - })), - (ChannelLayout::Rgb, SampleType::U16) => shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - 255, - ] - })), - (ChannelLayout::Rgba, SampleType::U16) => shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - (u16::from_ne_bytes([a0, a1]) >> 8) as u8, - ] - })), - _ => return Err(CodecError::unsupported(Format::Png, "unsupported PNG channel layout")), + (ChannelLayout::GrayA, SampleType::U16) => { + shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { + let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; + let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; + [g, g, g, a] + })) + } + (ChannelLayout::Rgb, SampleType::U16) => { + shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + 255, + ] + })) + } + (ChannelLayout::Rgba, SampleType::U16) => { + shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + (u16::from_ne_bytes([a0, a1]) >> 8) as u8, + ] + })) + } + _ => return Err(()), } Ok(()) }) From d97cb31bb6094e4ef7ef019cdb74787381618594 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 19:18:40 +0000 Subject: [PATCH 04/20] fix(render): use CodecError::Unsupported and cargo fmt --- crates/render/src/paint.rs | 66 ++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index b415a9da5f..7ef1cfb5bf 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -294,9 +294,7 @@ const PNG_WINDOW_AND_STEP: u64 = (32 << 10) + (8 << 10); /// to twice the size it holds, are both allocated: three times that size at most. A copy of a /// stored row and the row given out come on top. `None` when a row given out doesn't fit memory. fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { - let out = u64::from(hdr.width) - .checked_mul(hdr.layout.channels() as u64)? - .checked_mul(hdr.sample.bytes() as u64)?; + let out = u64::from(hdr.width).checked_mul(hdr.layout.channels() as u64)?.checked_mul(hdr.sample.bytes() as u64)?; // A filter byte and the row's samples. let stored = 1u64.checked_add(out)?; let kept = (stored * 4).next_multiple_of(64).max(PNG_KEPT_ROWS); @@ -334,54 +332,46 @@ fn reduced_pixmap(bytes: &[u8], r: &Reduction) -> Result Option { let mut shrink = Shrink::new(r.size, r.to); if r.rows { - use craft_image_codecs::{ChannelLayout, Limits, PngDecodeRows, SampleType}; + use craft_image_codecs::{ChannelLayout, CodecError, Format, Limits, PngDecodeRows, SampleType}; let mut dec = PngDecodeRows::new_streaming(bytes, &Limits::default()).ok()?; if !dec.streamable() { return None; } let hdr = dec.header(); - dec.step_stream::<()>(1, &mut || true, &mut |_y, row| { + dec.step_stream::(1, &mut || true, &mut |_y, row| { match (hdr.layout, hdr.sample) { (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), - (ChannelLayout::GrayA, SampleType::U8) => { - shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])) - } - (ChannelLayout::Rgb, SampleType::U8) => { - shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])) - } + (ChannelLayout::GrayA, SampleType::U8) => shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), + (ChannelLayout::Rgb, SampleType::U8) => shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { let v = (u16::from_ne_bytes(g) >> 8) as u8; [v, v, v, 255] })), - (ChannelLayout::GrayA, SampleType::U16) => { - shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { - let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; - let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; - [g, g, g, a] - })) + (ChannelLayout::GrayA, SampleType::U16) => shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { + let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; + let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; + [g, g, g, a] + })), + (ChannelLayout::Rgb, SampleType::U16) => shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + 255, + ] + })), + (ChannelLayout::Rgba, SampleType::U16) => shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + (u16::from_ne_bytes([a0, a1]) >> 8) as u8, + ] + })), + _ => { + return Err(CodecError::Unsupported { format: Format::Png, reason: "unsupported PNG channel layout".into() }); } - (ChannelLayout::Rgb, SampleType::U16) => { - shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - 255, - ] - })) - } - (ChannelLayout::Rgba, SampleType::U16) => { - shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - (u16::from_ne_bytes([a0, a1]) >> 8) as u8, - ] - })) - } - _ => return Err(()), } Ok(()) }) From db054839157263a57785c1f5d6ae3c9314447277 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 15:14:48 -0500 Subject: [PATCH 05/20] fix(codecs): adapt migrated regression tests to shared public APIs --- Cargo.lock | 3 +-- crates/render/src/encode/tests_psd.rs | 9 +++------ crates/render/src/encode/tests_tiff.rs | 26 +++++++++++++------------- crates/trace/src/opaque.rs | 2 +- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7782fdbaa3..eb58a4104a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6908,7 +6908,6 @@ dependencies = [ "image", "jpeg-encoder", "log", - "png 0.18.1", "proptest", "serde_json", "unicode-general-category", @@ -7032,7 +7031,7 @@ dependencies = [ name = "vectorcraft-trace" version = "0.24.1" dependencies = [ - "image", + "craft-image-codecs", "kurbo 0.13.1", "serde", "serde_json", diff --git a/crates/render/src/encode/tests_psd.rs b/crates/render/src/encode/tests_psd.rs index 641116b3cb..9a3b227ca0 100644 --- a/crates/render/src/encode/tests_psd.rs +++ b/crates/render/src/encode/tests_psd.rs @@ -232,12 +232,9 @@ fn maximum_editability_writes_groups_and_an_object_per_layer() { assert_eq!((p.records[8].blend, p.records[8].opacity), (*b"pass", 255)); assert_eq!(p.records[6].rect, [0, 0, 6, 6]); let psd = craft_psd::PsdFile::from_bytes(&file).unwrap(); - let content_layers = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::Other)).count(); - let groups = psd - .layers() - .iter() - .filter(|l| matches!(l.section_type(), craft_psd::layer::SectionType::OpenFolder | craft_psd::layer::SectionType::ClosedFolder)) - .count(); + let content_layers = psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::SectionType::Other)).count(); + let groups = + psd.layers().iter().filter(|l| matches!(l.section_type(), craft_psd::SectionType::OpenFolder | craft_psd::SectionType::ClosedFolder)).count(); assert_eq!((content_layers, groups), (3, 3)); assert_eq!(merged(&file), merged(&export(&doc, &opts(PsdOptions::default()))), "the same merged image"); diff --git a/crates/render/src/encode/tests_tiff.rs b/crates/render/src/encode/tests_tiff.rs index 380b6d155c..eb8dd42414 100644 --- a/crates/render/src/encode/tests_tiff.rs +++ b/crates/render/src/encode/tests_tiff.rs @@ -6,20 +6,20 @@ use craft_tiff::tags; use super::jpeg::ColorModel; use super::tiff::{self as t, ByteOrder, Compression, Image, Layout, Photometric, TiffOptions}; -fn ifd0(file: &[u8]) -> craft_tiff::reader::Ifd { +fn ifd0(file: &[u8]) -> craft_tiff::Ifd { Tiff::parse(file).expect("a TIFF").ifds.into_iter().next().expect("IFD0") } -fn tag_u16_vec(ifd: &craft_tiff::reader::Ifd, tag: u16) -> Option> { +fn tag_u16_vec(ifd: &craft_tiff::Ifd, tag: u16) -> Option> { ifd.value(tag).map(|v| v.to_u64_vec().into_iter().map(|x| x as u16).collect()) } -fn tag_u32_vec(ifd: &craft_tiff::reader::Ifd, tag: u16) -> Option> { +fn tag_u32_vec(ifd: &craft_tiff::Ifd, tag: u16) -> Option> { ifd.value(tag).map(|v| v.to_u64_vec().into_iter().map(|x| x as u32).collect()) } fn pixels(file: &[u8]) -> Vec { - craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, file).expect("decodes").data + craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, file).expect("decodes").into_data() } /// Straight RGBA: flat areas (as art is) with a transparent corner when `clear`. @@ -69,7 +69,7 @@ fn lzw_is_smaller_and_decodes_the_same() { assert_eq!(pixels(&lzw), px, "transparency kept as an alpha channel"); assert_eq!(pixels(&plain), px); let img = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &lzw).unwrap(); - assert_eq!(img.data, px); + assert_eq!(img.data(), px); } #[test] @@ -77,14 +77,14 @@ fn colour_models_write_their_samples() { let px = art(9, 6, true); let rgba = t::encode_samples(&px, 9, 6, 72.0, &opts(ColorModel::Rgb)).unwrap(); let rgba_ifd = ifd0(&rgba); - assert_eq!(rgba_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(2)); + assert_eq!(rgba_ifd.u16(tags::PHOTOMETRIC), Some(2)); assert_eq!(rgba_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(4)); assert_eq!(tag_u16_vec(&rgba_ifd, tags::BITS_PER_SAMPLE).as_deref(), Some([8, 8, 8, 8].as_slice())); assert_eq!(tag_u16_vec(&rgba_ifd, tags::EXTRA_SAMPLES).as_deref(), Some([2].as_slice()), "unassociated alpha"); let opaque = art(9, 6, false); let rgb = t::encode_samples(&opaque, 9, 6, 72.0, &opts(ColorModel::Rgb)).unwrap(); let rgb_ifd = ifd0(&rgb); - assert_eq!(rgb_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(2)); + assert_eq!(rgb_ifd.u16(tags::PHOTOMETRIC), Some(2)); assert_eq!(rgb_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(3)); assert!(rgb_ifd.value(tags::EXTRA_SAMPLES).is_none()); @@ -92,7 +92,7 @@ fn colour_models_write_their_samples() { let gray_ifd = ifd0(&gray); assert_eq!(gray_ifd.u16(tags::SAMPLES_PER_PIXEL), Some(1)); assert_eq!(tag_u16_vec(&gray_ifd, tags::BITS_PER_SAMPLE).as_deref(), Some([8].as_slice())); - assert_eq!(gray_ifd.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(1), "black is zero"); + assert_eq!(gray_ifd.u16(tags::PHOTOMETRIC), Some(1), "black is zero"); let g = pixels(&gray); assert_eq!((g[0], g[8]), (super::jpeg::to_gray()([200, 30, 40]), 255), "lightness; white stays white"); let on_white = t::encode_samples(&px, 9, 6, 72.0, &opts(ColorModel::Gray)).unwrap(); @@ -105,10 +105,10 @@ fn colour_models_write_their_samples() { let cmyk = t::encode_samples(&inks, 9, 6, 72.0, &opts(ColorModel::Cmyk)).unwrap(); let d = ifd0(&cmyk); assert_eq!(d.u16(tags::SAMPLES_PER_PIXEL), Some(4)); - assert_eq!(d.u16(tags::PHOTOMETRIC_INTERPRETATION), Some(5), "separated"); + assert_eq!(d.u16(tags::PHOTOMETRIC), Some(5), "separated"); assert_eq!(d.u16(332), Some(1), "CMYK inks"); let cmyk_img = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &cmyk).unwrap(); - assert_eq!(cmyk_img.data, inks); + assert_eq!(cmyk_img.data(), inks); } #[test] @@ -118,7 +118,7 @@ fn resolution_tags_and_the_profile() { let file = t::encode_samples(&px, 4, 4, ppi, &opts(ColorModel::Rgb)).unwrap(); let d = ifd0(&file); for tag in [tags::X_RESOLUTION, tags::Y_RESOLUTION] { - let Some(craft_tiff::reader::IfdValue::Rationals(v)) = d.value(tag) else { panic!("rational") }; + let Some(craft_tiff::Value::Rational(v)) = d.value(tag) else { panic!("rational") }; assert_eq!(v.as_slice(), &[rational]); } assert_eq!(d.u16(tags::RESOLUTION_UNIT), Some(2), "inches"); @@ -169,8 +169,8 @@ fn packbits_and_bilevel_images_decode() { assert_eq!(d.u16(tags::COMPRESSION), Some(32773)); // The decoder turns WhiteIsZero into black-is-zero. let dec = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &file).unwrap(); - assert_eq!(dec.width, 10); - assert_eq!(dec.height, 3); + assert_eq!(dec.width(), 10); + assert_eq!(dec.height(), 3); // A buffer that doesn't match the size is refused, not written. let short = Image { data: &rows[..5], ..image }; assert!(t::write(&short, &Layout { byte_order: ByteOrder::Little, compression: Compression::None, ppi: 72.0, icc: None }).is_err()); diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index d1a045c4c8..53d5516f71 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -313,7 +313,7 @@ mod tests { } let rgba = Image::from_u8(4, 4, ChannelLayout::Rgba, rgba_bytes).unwrap(); let luma_a = Image::from_u8(4, 4, ChannelLayout::GrayA, luma_a_bytes).unwrap(); - let rgba16 = Image::new(4, 4, ChannelLayout::Rgba, SampleType::U16, rgba16_bytes).unwrap(); + let rgba16 = Image::from_raw(4, 4, ChannelLayout::Rgba, SampleType::U16, rgba16_bytes).unwrap(); for (name, img) in [("rgba", rgba), ("gray and alpha", luma_a), ("16 bits", rgba16)] { let got = decode_alpha(&encoded(&img, Format::Png)).unwrap(); assert_eq!(got, Some((4, 4, want.clone())), "{name}"); From 1e1100a9ede976057c973bbebf185b96fad26a4b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 15:23:53 -0500 Subject: [PATCH 06/20] fix(tiff): keep test directory alive while checking borrowed ICC bytes --- crates/render/src/encode/tests_tiff.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/render/src/encode/tests_tiff.rs b/crates/render/src/encode/tests_tiff.rs index eb8dd42414..be2df81aa6 100644 --- a/crates/render/src/encode/tests_tiff.rs +++ b/crates/render/src/encode/tests_tiff.rs @@ -126,7 +126,8 @@ fn resolution_tags_and_the_profile() { for model in ColorModel::ALL { let src = if model == ColorModel::Cmyk { vec![0; 64] } else { px.clone() }; let file = t::encode_samples(&src, 4, 4, 72.0, &TiffOptions { embed_icc: true, ..opts(model) }).unwrap(); - let icc = ifd0(&file).bytes(tags::ICC_PROFILE).unwrap(); + let ifd = ifd0(&file); + let icc = ifd.bytes(tags::ICC_PROFILE).unwrap(); assert_eq!(&icc[36..40], b"acsp", "{model:?}: an ICC profile"); let space: &[u8; 4] = match model { ColorModel::Rgb => b"RGB ", From 5122a1f9d98dcd88a2ef87f809642dde3228ab8a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 16:49:34 -0500 Subject: [PATCH 07/20] fix: preserve XML and raster import semantics after codec cutover --- crates/engine/src/cmd/place/docx.rs | 3 +- crates/engine/src/cmd/variablecmds/library.rs | 3 +- crates/engine/src/tests/textimport.rs | 7 + crates/engine/src/tests/variables.rs | 2 + crates/render/src/encode/tests_psd.rs | 13 +- crates/render/src/encode/tests_tiff.rs | 5 + crates/render/src/paint.rs | 120 ++++++++++++------ crates/trace/src/opaque.rs | 12 +- crates/trace/src/raster.rs | 71 ++++++++++- crates/trace/src/tests.rs | 70 ++++++++++ 10 files changed, 249 insertions(+), 57 deletions(-) diff --git a/crates/engine/src/cmd/place/docx.rs b/crates/engine/src/cmd/place/docx.rs index ac528f3735..9fd06106a5 100644 --- a/crates/engine/src/cmd/place/docx.rs +++ b/crates/engine/src/cmd/place/docx.rs @@ -165,7 +165,8 @@ fn parse(xml: &str, name: &str, cmd: &str) -> Result { doctype: Doctype::Reject, limits: Limits { max_depth: MAX_NESTING, max_nodes: MAX_NODES as usize, ..Limits::DEFAULT }, ..Options::default() - }; + } + .strict(); opts.parse(xml.as_bytes()).map_err(|e| match e { craft_xml::Error::TooDeep(_) => bad(cmd, format!("`{name}` in the Word document nests elements more than {MAX_NESTING} deep")), _ => bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")), diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index 63c1a6f382..6708a51630 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -202,7 +202,8 @@ fn parse_library(xml: &str, cmd: &str) -> Result { names: craft_xml::Names::Local, limits: craft_xml::Limits { max_depth: MAX_LIBRARY_NESTING, max_nodes: MAX_LIBRARY_NODES as usize, ..craft_xml::Limits::DEFAULT }, ..craft_xml::Options::default() - }; + } + .strict(); let doc = opts.parse(xml.as_bytes()).map_err(|e| match e { craft_xml::Error::TooDeep(_) => bad(cmd, format!("the variable library nests elements more than {MAX_LIBRARY_NESTING} deep")), _ => bad(cmd, format!("the variable library is not well-formed XML: {e}")), diff --git a/crates/engine/src/tests/textimport.rs b/crates/engine/src/tests/textimport.rs index b369346096..88f613fe7a 100644 --- a/crates/engine/src/tests/textimport.rs +++ b/crates/engine/src/tests/textimport.rs @@ -468,6 +468,13 @@ fn malformed_docx_fails_cleanly() { assert!(e.contains("not a WordprocessingML document"), "{e}"); let e = place(&mut s, "a.docx", &store(&[("word/document.xml", b"")])).unwrap_err(); assert!(e.contains("not well-formed XML"), "{e}"); + for xml in [ + format!(r#"partial text"#), + document("A&missing;B"), + ] { + let e = place(&mut s, "a.docx", &store(&[("word/document.xml", xml.as_bytes())])).unwrap_err(); + assert!(e.contains("not well-formed XML"), "{e}"); + } let e = place(&mut s, "a.docx", &store(&[("word/document.xml", document("").as_bytes())])).unwrap_err(); assert!(e.contains("no text to place"), "{e}"); // A file whose headers understate what it unpacks to, and one whose headers say it is too large. diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index 6256088842..5539377dbe 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -2479,6 +2479,8 @@ fn malformed_variable_libraries_are_refused() { ); let cases = [ ("not well-formed XML", "".to_string()), + ("not well-formed XML", ok.replace("

x

", "

A&missing;B

")), + ("not well-formed XML", ok.replace("", "")), ("no `variableSets` element", "".to_string()), ("declares no variables", "".to_string()), ("is declared twice", library(&[("Name", "textcontent"), ("Name", "textcontent")], &[])), diff --git a/crates/render/src/encode/tests_psd.rs b/crates/render/src/encode/tests_psd.rs index 9a3b227ca0..e2df72e01c 100644 --- a/crates/render/src/encode/tests_psd.rs +++ b/crates/render/src/encode/tests_psd.rs @@ -144,9 +144,18 @@ fn unpack(mut src: &[u8], out: &mut Vec) { } } -/// The composite as a reader sees it. +/// The stored composite: RGB stays white-matted, with its original alpha. fn merged(f: &[u8]) -> Vec { - craft_psd::PsdFile::from_bytes(f).expect("a PSD a reader takes").composite_rgba8().expect("composite").data + let psd = craft_psd::PsdFile::from_bytes(f).expect("a PSD a reader takes"); + if psd.header.color_mode != craft_psd::ColorMode::Rgb { + return psd.composite_rgba8().expect("composite").data; + } + assert_eq!(psd.header.depth, 8); + let samples = psd.decode_merged().expect("stored composite planes"); + let count = psd.header.width as usize * psd.header.height as usize; + (0..count) + .flat_map(|i| [samples[i], samples[count + i], samples[2 * count + i], if psd.merged_has_alpha() { samples[3 * count + i] } else { 255 }]) + .collect() } fn pixel(px: &[u8], width: usize, x: usize, y: usize) -> [u8; 4] { diff --git a/crates/render/src/encode/tests_tiff.rs b/crates/render/src/encode/tests_tiff.rs index be2df81aa6..e10739e67a 100644 --- a/crates/render/src/encode/tests_tiff.rs +++ b/crates/render/src/encode/tests_tiff.rs @@ -172,6 +172,11 @@ fn packbits_and_bilevel_images_decode() { let dec = craft_image_codecs::decode_as(craft_image_codecs::Format::Tiff, &file).unwrap(); assert_eq!(dec.width(), 10); assert_eq!(dec.height(), 3); + assert_eq!( + dec.data(), + [0, 255, 0, 255, 0, 255, 0, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255], + "WhiteIsZero samples are inverted and row padding is ignored" + ); // A buffer that doesn't match the size is refused, not written. let short = Image { data: &rows[..5], ..image }; assert!(t::write(&short, &Layout { byte_order: ByteOrder::Little, compression: Compression::None, ppi: 72.0, icc: None }).is_err()); diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 7ef1cfb5bf..987ee04710 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -330,52 +330,65 @@ fn reduced_pixmap(bytes: &[u8], r: &Reduction) -> Result Option { + shrunk_with_png_limits(bytes, r, &craft_image_codecs::Limits::default()) +} + +fn shrunk_with_png_limits(bytes: &[u8], r: &Reduction, png_limits: &craft_image_codecs::Limits) -> Option { let mut shrink = Shrink::new(r.size, r.to); if r.rows { - use craft_image_codecs::{ChannelLayout, CodecError, Format, Limits, PngDecodeRows, SampleType}; - let mut dec = PngDecodeRows::new_streaming(bytes, &Limits::default()).ok()?; + use craft_image_codecs::{ChannelLayout, CodecError, Format, PngDecodeRows, SampleType}; + let mut dec = PngDecodeRows::new_streaming(bytes, png_limits).ok()?; if !dec.streamable() { return None; } let hdr = dec.header(); - dec.step_stream::(1, &mut || true, &mut |_y, row| { - match (hdr.layout, hdr.sample) { - (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), - (ChannelLayout::GrayA, SampleType::U8) => shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), - (ChannelLayout::Rgb, SampleType::U8) => shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), - (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), - (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { - let v = (u16::from_ne_bytes(g) >> 8) as u8; - [v, v, v, 255] - })), - (ChannelLayout::GrayA, SampleType::U16) => shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { - let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; - let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; - [g, g, g, a] - })), - (ChannelLayout::Rgb, SampleType::U16) => shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - 255, - ] - })), - (ChannelLayout::Rgba, SampleType::U16) => shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { - [ - (u16::from_ne_bytes([r0, r1]) >> 8) as u8, - (u16::from_ne_bytes([g0, g1]) >> 8) as u8, - (u16::from_ne_bytes([b0, b1]) >> 8) as u8, - (u16::from_ne_bytes([a0, a1]) >> 8) as u8, - ] - })), - _ => { - return Err(CodecError::Unsupported { format: Format::Png, reason: "unsupported PNG channel layout".into() }); - } - } - Ok(()) - }) - .ok()??; + // Yield after each row and stop once all pixels have arrived. The pinned decoder's + // first tail batch only locates a text chunk; later batches inflate/retain its value. + // Never run those batches: the renderer needs pixels, not ancillary text. Its underlying + // PNG reader also ignores text chunks before IDAT, so neither location retains text. + for _ in 0..hdr.height { + let _extras = dec + .step_stream::(1, &mut || false, &mut |_y, row| { + match (hdr.layout, hdr.sample) { + (ChannelLayout::Gray, SampleType::U8) => shrink.row(row.iter().map(|&g| [g, g, g, 255])), + (ChannelLayout::GrayA, SampleType::U8) => shrink.row(row.as_chunks::<2>().0.iter().map(|&[g, a]| [g, g, g, a])), + (ChannelLayout::Rgb, SampleType::U8) => shrink.row(row.as_chunks::<3>().0.iter().map(|&[r, g, b]| [r, g, b, 255])), + (ChannelLayout::Rgba, SampleType::U8) => shrink.row(row.as_chunks::<4>().0.iter().copied()), + (ChannelLayout::Gray, SampleType::U16) => shrink.row(row.as_chunks::<2>().0.iter().map(|&g| { + let v = (u16::from_ne_bytes(g) >> 8) as u8; + [v, v, v, 255] + })), + (ChannelLayout::GrayA, SampleType::U16) => shrink.row(row.as_chunks::<4>().0.iter().map(|&[g0, g1, a0, a1]| { + let g = (u16::from_ne_bytes([g0, g1]) >> 8) as u8; + let a = (u16::from_ne_bytes([a0, a1]) >> 8) as u8; + [g, g, g, a] + })), + (ChannelLayout::Rgb, SampleType::U16) => shrink.row(row.as_chunks::<6>().0.iter().map(|&[r0, r1, g0, g1, b0, b1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + 255, + ] + })), + (ChannelLayout::Rgba, SampleType::U16) => { + shrink.row(row.as_chunks::<8>().0.iter().map(|&[r0, r1, g0, g1, b0, b1, a0, a1]| { + [ + (u16::from_ne_bytes([r0, r1]) >> 8) as u8, + (u16::from_ne_bytes([g0, g1]) >> 8) as u8, + (u16::from_ne_bytes([b0, b1]) >> 8) as u8, + (u16::from_ne_bytes([a0, a1]) >> 8) as u8, + ] + })) + } + _ => { + return Err(CodecError::Unsupported { format: Format::Png, reason: "unsupported PNG channel layout".into() }); + } + } + Ok(()) + }) + .ok()?; + } } else { let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?; let mut limits = image::Limits::default(); @@ -616,6 +629,33 @@ mod tests { assert_eq!(room_for(MAX_DECODE_ALLOC, MAX_DECODE_ALLOC), Ok(())); } + #[test] + fn streaming_reduction_does_not_retain_text_after_pixels() { + let mut bytes = Vec::new(); + striped(8).write_to(&mut std::io::Cursor::new(&mut bytes), image::ImageFormat::Png).unwrap(); + let mut chunk = b"tEXtComment\0".to_vec(); + chunk.extend(std::iter::repeat_n(b'x', 1 << 20)); + // PNG's IEEE CRC over type and payload, kept local to this fixture. + let mut crc = u32::MAX; + for &byte in &chunk { + crc ^= u32::from(byte); + for _ in 0..8 { + crc = (crc >> 1) ^ (0xedb88320 & 0u32.wrapping_sub(crc & 1)); + } + } + let mut tail = ((chunk.len() - 4) as u32).to_be_bytes().to_vec(); + tail.extend(chunk); + tail.extend((!crc).to_be_bytes()); + bytes.splice(bytes.len() - 12..bytes.len() - 12, tail); + let limits = craft_image_codecs::Limits { max_alloc: 1 << 19, ..Default::default() }; + let mut full = craft_image_codecs::PngDecodeRows::new_streaming(&bytes, &limits).unwrap(); + assert!(full.step_stream::(1, &mut || true, &mut |_, _| Ok(())).is_err()); + // The renderer stops after the pixels instead of retaining ancillary text. + let reduced = shrunk_with_png_limits(&bytes, &Reduction { size: (8, 1), to: (4, 1), rows: true, scratch: 0 }, &limits).unwrap(); + assert_eq!((reduced.width(), reduced.height()), (4, 1)); + assert!(red_at_its_coverage(&reduced)); + } + /// Opaque red beside clear black, every other column of a `w` × 1 image. fn striped(w: u32) -> image::RgbaImage { image::RgbaImage::from_fn(w, 1, |x, _| if x % 2 == 0 { image::Rgba([200, 30, 30, 255]) } else { image::Rgba([0, 0, 0, 0]) }) diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index 53d5516f71..47a2fca6ae 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -3,7 +3,7 @@ use vectorcraft_geom::{Affine, PathData}; -use crate::raster::{MAX_PIXELS, fit_size, gif_first_frame_pixels}; +use crate::raster::{MAX_PIXELS, fit_size, gif_first_frame_pixels, trace_header}; use crate::{Mode, Raster, TraceError, TraceParams, trace_within}; /// The most pixels [`opaque_outline`] traces: a larger image is reduced to this many first, @@ -24,9 +24,9 @@ pub const ALPHA_MAX_DECODE_BYTES: u64 = 256 << 20; /// than [`MAX_PIXELS`] (64 megapixels), or whose samples take more than [`ALPHA_MAX_DECODE_BYTES`], /// gives [`TraceError::TooLarge`] before it is decoded. pub fn decode_alpha(bytes: &[u8]) -> Result)>, TraceError> { - use craft_image_codecs::{ChannelLayout, DecodeOptions, Limits, RasterColorType, SampleType, decode_with, inspect_raster}; + use craft_image_codecs::{ChannelLayout, DecodeOptions, Limits, RasterColorType, SampleType, decode_with}; let decode_err = |e: craft_image_codecs::CodecError| TraceError::Decode(e.to_string()); - if let Some(info) = inspect_raster(bytes).map_err(decode_err)? { + if let Some(info) = trace_header(bytes, MAX_PIXELS, ALPHA_MAX_DECODE_BYTES)? { if info.width == 0 || info.height == 0 { return Err(TraceError::Empty); } @@ -49,10 +49,8 @@ pub fn decode_alpha(bytes: &[u8]) -> Result)>, TraceEr } else if gif_first_frame_pixels(bytes).is_some_and(|p| p > MAX_PIXELS) { return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_PIXELS >> 20 }); } - let opts = DecodeOptions { - limits: Limits { max_pixels: MAX_PIXELS, max_alloc: ALPHA_MAX_DECODE_BYTES, ..Limits::default() }, - ..DecodeOptions::default() - }; + let opts = + DecodeOptions { keep_orientation: true, limits: Limits { max_pixels: MAX_PIXELS, max_alloc: ALPHA_MAX_DECODE_BYTES, ..Limits::default() } }; let img = decode_with(bytes, &opts).map_err(decode_err)?; let (w, h) = (img.width(), img.height()); if w == 0 || h == 0 { diff --git a/crates/trace/src/raster.rs b/crates/trace/src/raster.rs index 35b035bc69..10a7918ab7 100644 --- a/crates/trace/src/raster.rs +++ b/crates/trace/src/raster.rs @@ -28,6 +28,63 @@ pub const MAX_PIXELS: u64 = 64 << 20; /// to [`MAX_PIXELS`] (192 megapixels: 768 MB decoded); larger images are refused. pub const MAX_SOURCE_PIXELS: u64 = 192 << 20; +/// Header facts for tracing, without reserving a PNG's full decoded pixel buffer. +/// The shared allocation probe runs before the decoder's plausibility checks so the caller's +/// size refusal wins even for a large header-only file. The streaming reader still checks the +/// PNG structure and plausibility, and reports the expanded layout (including tRNS alpha). +pub(crate) fn trace_header(bytes: &[u8], max_pixels: u64, max_alloc: u64) -> Result, TraceError> { + use craft_image_codecs::{ChannelLayout, Format, Limits, PngDecodeRows, RasterColorType, RasterInspection}; + let decode_err = |e: craft_image_codecs::CodecError| TraceError::Decode(e.to_string()); + if craft_image_codecs::detect(bytes) != Some(Format::Png) { + return craft_image_codecs::inspect_raster(bytes).map_err(decode_err); + } + let (width, height) = craft_image_codecs::probe_allocation_size(bytes).map_err(decode_err)?; + if u64::from(width) * u64::from(height) > max_pixels { + return Err(TraceError::TooLarge { width, height, max: max_pixels >> 20 }); + } + // The allocation probe validates the IHDR prefix. For explicit alpha layouts, its + // sample width also gives a lower bound on decoded bytes before plausibility checks. + // Preserve the alpha caller's TooLarge refusal for header-only RGBA16/GrayA images; + // the shared reader still validates CRCs, transparency expansion, and all other layouts. + let alpha_bytes_per_pixel = match bytes.get(24..26) { + Some([8, 4]) => 2, + Some([16, 4]) | Some([8, 6]) => 4, + Some([16, 6]) => 8, + _ => 0, + }; + if (u64::from(width) * u64::from(height)).saturating_mul(alpha_bytes_per_pixel) > max_alloc { + return Err(TraceError::TooLarge { width, height, max: max_pixels >> 20 }); + } + let limits = Limits { max_pixels, max_alloc, ..Limits::default() }; + let rows = PngDecodeRows::new_streaming(bytes, &limits).map_err(decode_err)?; + let h = rows.header(); + let color_type = match h.layout { + ChannelLayout::Gray => RasterColorType::Gray, + ChannelLayout::GrayA => RasterColorType::GrayAlpha, + ChannelLayout::Rgb => RasterColorType::Rgb, + ChannelLayout::Rgba => RasterColorType::Rgba, + _ => RasterColorType::Other, + }; + Ok(Some(RasterInspection { + width: h.width, + height: h.height, + channels: h.layout.channels() as u8, + bits_per_sample: (h.sample.bytes() * 8) as u8, + color_type, + palette: None, + transparent_index: None, + })) +} + +/// Reuse an already-normalized RGBA8 decode instead of allocating a second full pixel buffer. +fn into_rgba8(img: craft_image_codecs::Image) -> Vec { + if img.layout() == craft_image_codecs::ChannelLayout::Rgba && img.sample_type() == craft_image_codecs::SampleType::U8 { + img.into_data() + } else { + img.to_rgba8() + } +} + /// The largest size with `w : h`'s aspect of at most `max` pixels (each side at least 1). pub(crate) fn fit_size(w: u32, h: u32, max: u64) -> (u32, u32) { let pixels = u64::from(w) * u64::from(h); @@ -158,7 +215,7 @@ impl Raster { } /// Decode PNG / JPEG / WebP / GIF bytes (at most [`MAX_PIXELS`], checked before decoding). pub fn decode(bytes: &[u8]) -> Result { - if let Some(info) = craft_image_codecs::inspect_raster(bytes).map_err(|e| TraceError::Decode(e.to_string()))? { + if let Some(info) = trace_header(bytes, MAX_PIXELS, craft_image_codecs::Limits::default().max_alloc)? { if info.width == 0 || info.height == 0 { return Err(TraceError::Empty); } @@ -169,22 +226,23 @@ impl Raster { return Err(TraceError::TooLarge { width: 0, height: 0, max: MAX_PIXELS >> 20 }); } let opts = craft_image_codecs::DecodeOptions { + // Match rendering and trace placement: coordinates refer to stored pixels. + keep_orientation: true, limits: craft_image_codecs::Limits { max_pixels: MAX_PIXELS, ..craft_image_codecs::Limits::default() }, - ..craft_image_codecs::DecodeOptions::default() }; let img = craft_image_codecs::decode_with(bytes, &opts).map_err(|e| TraceError::Decode(e.to_string()))?; let (w, h) = (img.width(), img.height()); if w == 0 || h == 0 { return Err(TraceError::Empty); } - Ok(Self { width: w, height: h, rgba: img.to_rgba8() }) + Ok(Self { width: w, height: h, rgba: into_rgba8(img) }) } /// Decode as [`Raster::decode`], but an image of more than [`MAX_PIXELS`] (up to /// [`MAX_SOURCE_PIXELS`]) is scaled down, aspect kept, to the largest size within /// [`MAX_PIXELS`] instead of being refused: a 300 ppi scan of a poster traces at slightly /// less than its own resolution. The caller maps the result back with `width` / `height`. pub fn decode_to_fit(bytes: &[u8]) -> Result { - if let Some(info) = craft_image_codecs::inspect_raster(bytes).map_err(|e| TraceError::Decode(e.to_string()))? { + if let Some(info) = trace_header(bytes, MAX_SOURCE_PIXELS, if cfg!(target_arch = "wasm32") { 512 << 20 } else { MAX_SOURCE_PIXELS * 4 })? { if info.width == 0 || info.height == 0 { return Err(TraceError::Empty); } @@ -204,19 +262,20 @@ impl Raster { // Decoded within the allocation the source limit needs; the web keeps file import's 512 MB. // A deeper image (16 bits a channel) that needs more fails with a decode error. let opts = craft_image_codecs::DecodeOptions { + // Match rendering and trace placement: coordinates refer to stored pixels. + keep_orientation: true, limits: craft_image_codecs::Limits { max_pixels: MAX_SOURCE_PIXELS, max_alloc: if cfg!(target_arch = "wasm32") { 512 << 20 } else { MAX_SOURCE_PIXELS * 4 }, ..craft_image_codecs::Limits::default() }, - ..craft_image_codecs::DecodeOptions::default() }; let img = craft_image_codecs::decode_with(bytes, &opts).map_err(|e| TraceError::Decode(e.to_string()))?; let (w, h) = (img.width(), img.height()); if w == 0 || h == 0 { return Err(TraceError::Empty); } - let rgba = img.to_rgba8(); + let rgba = into_rgba8(img); if u64::from(w) * u64::from(h) <= MAX_PIXELS { return Ok(Self { width: w, height: h, rgba }); } diff --git a/crates/trace/src/tests.rs b/crates/trace/src/tests.rs index da301b0303..c6a16f9170 100644 --- a/crates/trace/src/tests.rs +++ b/crates/trace/src/tests.rs @@ -3974,3 +3974,73 @@ fn a_gif_frame_larger_than_its_screen_is_refused_before_decoding() { assert!(matches!(Raster::decode(&gif), Err(TraceError::TooLarge { .. })), "{:?}", Raster::decode(&gif).err()); assert!(matches!(Raster::decode_to_fit(&gif), Err(TraceError::TooLarge { .. }))); } + +// Synthetic PNG chunks for transparency/orientation regression fixtures. +fn png_test_chunk(kind: &[u8; 4], data: &[u8]) -> Vec { + let mut out = (data.len() as u32).to_be_bytes().to_vec(); + out.extend(kind); + out.extend(data); + let mut crc = !0u32; + for &b in &out[4..] { + crc ^= u32::from(b); + for _ in 0..8 { + crc = if crc & 1 != 0 { 0xedb8_8320 ^ (crc >> 1) } else { crc >> 1 }; + } + } + out.extend((!crc).to_be_bytes()); + out +} + +#[test] +fn alpha_decode_keeps_rgb_and_grayscale_png_transparency_keys() { + use craft_image_codecs::{ChannelLayout, Format, Image}; + for (layout, samples, key) in + [(ChannelLayout::Rgb, vec![10, 20, 30, 40, 50, 60], vec![0, 10, 0, 20, 0, 30]), (ChannelLayout::Gray, vec![10, 40], vec![0, 10])] + { + let image = Image::from_u8(2, 1, layout, samples).unwrap(); + let mut png = craft_image_codecs::encode(&image, Format::Png, &Default::default()).unwrap(); + // A transparency key precedes IDAT, immediately after the IHDR chunk. + png.splice(33..33, png_test_chunk(b"tRNS", &key)); + assert_eq!(decode_alpha(&png).unwrap(), Some((2, 1, vec![0, 255]))); + } +} + +#[test] +fn trace_and_alpha_keep_stored_pixels_with_exif_orientation() { + use craft_image_codecs::{ChannelLayout, Format, Image}; + // Little-endian TIFF IFD containing Orientation=6 (90 degrees clockwise). + let exif = [b'I', b'I', 42, 0, 8, 0, 0, 0, 1, 0, 0x12, 1, 3, 0, 1, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0]; + let raster = Raster::from_fn(3, 2, |x, y| [x as u8 * 60, y as u8 * 90, 17, (x + 3 * y) as u8 * 40]); + let mut png = raster.encode_png(); + png.splice(33..33, png_test_chunk(b"eXIf", &exif)); + for actual in [Raster::decode(&png).unwrap(), Raster::decode_to_fit(&png).unwrap()] { + assert_eq!((actual.width, actual.height), (3, 2)); + assert_eq!(actual.rgba, raster.rgba); + } + assert_eq!(decode_alpha(&png).unwrap(), Some((3, 2, raster.rgba.as_chunks::<4>().0.iter().map(|p| p[3]).collect()))); + + let image = + Image::from_u8(3, 2, ChannelLayout::Rgb, raster.rgba.as_chunks::<4>().0.iter().flat_map(|p| p[..3].iter().copied()).collect()).unwrap(); + let mut jpeg = craft_image_codecs::encode(&image, Format::Jpeg, &Default::default()).unwrap(); + let expected = Raster::decode(&jpeg).unwrap(); + let mut app1 = vec![0xff, 0xe1]; + app1.extend(((exif.len() + 8) as u16).to_be_bytes()); + app1.extend(b"Exif\0\0"); + app1.extend(exif); + jpeg.splice(2..2, app1); + for actual in [Raster::decode(&jpeg).unwrap(), Raster::decode_to_fit(&jpeg).unwrap()] { + assert_eq!((actual.width, actual.height), (3, 2)); + assert_eq!(actual.rgba, expected.rgba); + } +} + +#[test] +fn alpha_rejects_oversized_png_before_decoding() { + assert!(matches!(decode_alpha(&png_header(30_000, 30_000)), Err(TraceError::TooLarge { .. }))); + assert!(matches!(decode_alpha(&png_header(64, 64)), Err(TraceError::Decode(_)))); + let mut deep = png_header(7000, 6000); + deep[24] = 16; + let ihdr = png_test_chunk(b"IHDR", &deep[16..29]); + deep.splice(8..33, ihdr); + assert!(matches!(decode_alpha(&deep), Err(TraceError::TooLarge { .. }))); +} From cb5c52f5e6929d028d755f1b3cb7d4f202910cf1 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 17:27:03 -0500 Subject: [PATCH 08/20] fix: validate XML attribute identities and preflight GIF frame limits --- crates/engine/src/cmd.rs | 2 + crates/engine/src/cmd/place/docx.rs | 7 ++- crates/engine/src/cmd/variablecmds/library.rs | 15 ++--- crates/engine/src/cmd/xmlattrs.rs | 55 +++++++++++++++++++ crates/engine/src/tests/textimport.rs | 2 + crates/engine/src/tests/variables.rs | 14 +++++ crates/trace/src/raster.rs | 5 ++ crates/trace/src/tests.rs | 1 + 8 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 crates/engine/src/cmd/xmlattrs.rs diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 99b8e1ecdd..575909a7bd 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -97,6 +97,8 @@ pub(crate) mod symboledit; pub mod symbollib; pub(crate) mod tabs; mod textedit; +mod xmlattrs; + pub use textedit::effective_kerning; pub mod textstyles; pub(crate) mod textwrap; diff --git a/crates/engine/src/cmd/place/docx.rs b/crates/engine/src/cmd/place/docx.rs index 9fd06106a5..804e3fd6ae 100644 --- a/crates/engine/src/cmd/place/docx.rs +++ b/crates/engine/src/cmd/place/docx.rs @@ -167,10 +167,13 @@ fn parse(xml: &str, name: &str, cmd: &str) -> Result { ..Options::default() } .strict(); - opts.parse(xml.as_bytes()).map_err(|e| match e { + let doc = opts.parse(xml.as_bytes()).map_err(|e| match e { craft_xml::Error::TooDeep(_) => bad(cmd, format!("`{name}` in the Word document nests elements more than {MAX_NESTING} deep")), _ => bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")), - }) + })?; + crate::cmd::xmlattrs::validate(xml.as_bytes(), opts.limits) + .map_err(|e| bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")))?; + Ok(doc) } /// The part that the first relationship of part `source` (`""`: the package) whose type ends in diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index 6708a51630..44f298053e 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -199,7 +199,7 @@ impl Library { fn parse_library(xml: &str, cmd: &str) -> Result { let xml = without_doctype(xml, cmd)?; let opts = craft_xml::Options { - names: craft_xml::Names::Local, + names: craft_xml::Names::AsWritten, limits: craft_xml::Limits { max_depth: MAX_LIBRARY_NESTING, max_nodes: MAX_LIBRARY_NODES as usize, ..craft_xml::Limits::DEFAULT }, ..craft_xml::Options::default() } @@ -208,12 +208,13 @@ fn parse_library(xml: &str, cmd: &str) -> Result { craft_xml::Error::TooDeep(_) => bad(cmd, format!("the variable library nests elements more than {MAX_LIBRARY_NESTING} deep")), _ => bad(cmd, format!("the variable library is not well-formed XML: {e}")), })?; - let Some(root) = std::iter::once(&doc).chain(doc.descendants()).find(|n| n.is("variableSets")) else { + crate::cmd::xmlattrs::validate(xml.as_bytes(), opts.limits).map_err(|e| bad(cmd, format!("the variable library is not well-formed XML: {e}")))?; + let Some(root) = std::iter::once(&doc).chain(doc.descendants()).find(|n| n.is(craft_xml::Local("variableSets"))) else { return Err(bad(cmd, "the file holds no `variableSets` element")); }; - let sets: Vec<&craft_xml::El> = root.children("variableSet").collect(); + let sets: Vec<&craft_xml::El> = root.children(craft_xml::Local("variableSet")).collect(); let (mut text, mut other, mut declared, mut refs) = (Vec::new(), Vec::new(), BTreeSet::new(), Vec::new()); - for var in sets.iter().flat_map(|s| s.children("variables")).flat_map(|v| v.children("variable")) { + for var in sets.iter().flat_map(|s| s.children(craft_xml::Local("variables"))).flat_map(|v| v.children(craft_xml::Local("variable"))) { let name = named(var.attr("varName"), "varName", cmd)?; if !declared.insert(name.clone()) { return Err(bad(cmd, format!("variable `{name}` is declared twice"))); @@ -239,7 +240,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result { } let text_names: BTreeSet<&str> = text.iter().map(String::as_str).collect(); let (mut datasets, mut seen) = (Vec::new(), BTreeSet::new()); - for row in sets.iter().flat_map(|s| s.children("sampleDataSets")).flat_map(|d| d.children("sampleDataSet")) { + for row in sets.iter().flat_map(|s| s.children(craft_xml::Local("sampleDataSets"))).flat_map(|d| d.children(craft_xml::Local("sampleDataSet"))) { let name = named(row.attr("dataSetName"), "dataSetName", cmd)?; if !seen.insert(name.clone()) { return Err(bad(cmd, format!("data set `{name}` is declared twice"))); @@ -249,7 +250,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result { } let (mut values, mut held) = (BTreeMap::new(), BTreeSet::new()); for el in row.elements() { - let var = el.name.as_str(); + let var = el.local(); if !declared.contains(var) { return Err(bad(cmd, format!("data set `{name}` holds a value for `{var}`, which the library doesn't declare"))); } @@ -259,7 +260,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result { if !text_names.contains(var) { continue; } - let paras: Vec = el.children("p").map(craft_xml::El::deep_text).collect(); + let paras: Vec = el.children(craft_xml::Local("p")).map(craft_xml::El::deep_text).collect(); if !paras.is_empty() { values.insert(var.to_string(), DataValue::Text(paras.join("\n"))); } diff --git a/crates/engine/src/cmd/xmlattrs.rs b/crates/engine/src/cmd/xmlattrs.rs new file mode 100644 index 0000000000..6dd2578fae --- /dev/null +++ b/crates/engine/src/cmd/xmlattrs.rs @@ -0,0 +1,55 @@ +//! Attribute uniqueness checks missing from the pinned shared XML reader's strict mode. +//! +//! Temporary consumer guard: parsing remains in craft-xml. Move this invariant into the +//! shared strict reader and remove this second event pass when that dependency is updated. + +/// Check names before canonical/local-name rewriting can collapse distinct attributes. +/// The event reader and namespace scope use the same caller-supplied resource caps. +pub(super) fn validate(xml: &[u8], limits: craft_xml::Limits) -> Result<(), String> { + use std::collections::BTreeSet; + + let opts = craft_xml::Options { limits, doctype: craft_xml::Doctype::Reject, ..Default::default() }.strict(); + let mut events = opts.events(xml); + let mut bindings: Vec<(String, String)> = Vec::new(); + let mut scopes = Vec::new(); + while let Some(event) = events.next().map_err(|e| e.to_string())? { + match event { + craft_xml::Event::Start { attrs, .. } => { + scopes.push(bindings.len()); + let mut raw = BTreeSet::new(); + for (key, value) in &attrs { + if !raw.insert(key.as_str()) { + return Err(format!("duplicate attribute `{key}`")); + } + if let Some(prefix) = key.strip_prefix("xmlns:") { + if bindings.len() >= limits.max_attrs { + return Err("too many in-scope namespace declarations".into()); + } + bindings.push((prefix.to_string(), value.clone())); + } + } + let mut expanded = BTreeSet::new(); + for (key, _) in &attrs { + if key == "xmlns" || key.starts_with("xmlns:") { + continue; + } + // Default namespaces do not apply to attributes. Preserve undeclared + // prefixes for standalone SVG library fragments, as the importer did. + let name = match key.split_once(':') { + Some(("xml", local)) => ("http://www.w3.org/XML/1998/namespace", local), + Some((prefix, local)) => { + bindings.iter().rev().find(|(p, _)| p == prefix).map_or(("", key.as_str()), |(_, uri)| (uri.as_str(), local)) + } + None => ("", key.as_str()), + }; + if !expanded.insert(name) { + return Err(format!("duplicate expanded attribute `{key}`")); + } + } + } + craft_xml::Event::End { .. } => bindings.truncate(scopes.pop().unwrap_or(0)), + _ => {} + } + } + Ok(()) +} diff --git a/crates/engine/src/tests/textimport.rs b/crates/engine/src/tests/textimport.rs index 88f613fe7a..cc89a8261b 100644 --- a/crates/engine/src/tests/textimport.rs +++ b/crates/engine/src/tests/textimport.rs @@ -471,6 +471,8 @@ fn malformed_docx_fails_cleanly() { for xml in [ format!(r#"partial text"#), document("A&missing;B"), + document("text"), + document("text"), ] { let e = place(&mut s, "a.docx", &store(&[("word/document.xml", xml.as_bytes())])).unwrap_err(); assert!(e.contains("not well-formed XML"), "{e}"); diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index 5539377dbe..5358a5e84e 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -2481,6 +2481,9 @@ fn malformed_variable_libraries_are_refused() { ("not well-formed XML", "".to_string()), ("not well-formed XML", ok.replace("

x

", "

A&missing;B

")), ("not well-formed XML", ok.replace("", "")), + ("not well-formed XML", ok.replace("varName=\"Name\"", "varName=\"Name\" varName=\"Other\"")), + ("not well-formed XML", ok.replace("".to_string()), ("declares no variables", "".to_string()), ("is declared twice", library(&[("Name", "textcontent"), ("Name", "textcontent")], &[])), @@ -3174,3 +3177,14 @@ fn an_svg_without_readable_variable_data_opens_its_art() { assert_eq!(vectorcraft_testkit::svg_shapes(&vectorcraft_svg::export(d, &Default::default())), 1, "the art opens"); } } + +#[test] +fn variable_library_attributes_keep_their_namespace() { + let mut s = session(); + let xml = library(&[("Name", "textcontent")], &[("A", &[("Name", &["x"])])]) + .replace("varName=\"Name\"", "xmlns:m=\"urn:metadata\" m:varName=\"Other\" varName=\"Name\""); + let loaded = s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap(); + assert_eq!(loaded["variables"], json!(["Name"])); + let sets = s.execute("dataset.list", &json!({})).unwrap(); + assert_eq!(sets["datasets"][0]["values"], json!({"Name": {"text": "x"}})); +} diff --git a/crates/trace/src/raster.rs b/crates/trace/src/raster.rs index 10a7918ab7..492b3b4eed 100644 --- a/crates/trace/src/raster.rs +++ b/crates/trace/src/raster.rs @@ -35,6 +35,11 @@ pub const MAX_SOURCE_PIXELS: u64 = 192 << 20; pub(crate) fn trace_header(bytes: &[u8], max_pixels: u64, max_alloc: u64) -> Result, TraceError> { use craft_image_codecs::{ChannelLayout, Format, Limits, PngDecodeRows, RasterColorType, RasterInspection}; let decode_err = |e: craft_image_codecs::CodecError| TraceError::Decode(e.to_string()); + // Refuse a GIF frame larger than its logical screen before inspection's + // plausibility check can turn the caller's size refusal into a decode error. + if gif_first_frame_pixels(bytes).is_some_and(|pixels| pixels > max_pixels) { + return Err(TraceError::TooLarge { width: 0, height: 0, max: max_pixels >> 20 }); + } if craft_image_codecs::detect(bytes) != Some(Format::Png) { return craft_image_codecs::inspect_raster(bytes).map_err(decode_err); } diff --git a/crates/trace/src/tests.rs b/crates/trace/src/tests.rs index c6a16f9170..85d423a6f4 100644 --- a/crates/trace/src/tests.rs +++ b/crates/trace/src/tests.rs @@ -3973,6 +3973,7 @@ fn a_gif_frame_larger_than_its_screen_is_refused_before_decoding() { gif.extend([2, 1, 0, 0, 0x3B]); assert!(matches!(Raster::decode(&gif), Err(TraceError::TooLarge { .. })), "{:?}", Raster::decode(&gif).err()); assert!(matches!(Raster::decode_to_fit(&gif), Err(TraceError::TooLarge { .. }))); + assert!(matches!(decode_alpha(&gif), Err(TraceError::TooLarge { .. }))); } // Synthetic PNG chunks for transparency/orientation regression fixtures. From ca6bd4fd951d662a58dcc4b29aabba0594e2ad2a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 23:21:22 +0000 Subject: [PATCH 09/20] fix(image): avoid false warning for streamed PNGs --- crates/engine/src/cmd/fileio/load.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/fileio/load.rs b/crates/engine/src/cmd/fileio/load.rs index ee33f64c41..b1f294d313 100644 --- a/crates/engine/src/cmd/fileio/load.rs +++ b/crates/engine/src/cmd/fileio/load.rs @@ -841,12 +841,22 @@ pub(crate) fn too_large_note(bytes: &[u8]) -> Option { if let Some((w, h)) = vectorcraft_doc::cmyk::oversized_tiff(bytes) { return Some(format!("an image of {w} × {h} pixels is too large to draw")); } - let d = image::ImageReader::new(Cursor::new(bytes)).with_guessed_format().ok()?.into_decoder().ok()?; + let reader = image::ImageReader::new(Cursor::new(bytes)).with_guessed_format().ok()?; + let format = reader.format()?; + let d = reader.into_decoder().ok()?; let (w, h) = d.dimensions(); // The allowance also counts the pixels as RGBA: an image of fewer than 4 bytes a pixel (gray, // gray with alpha, RGB) takes more than the decoder counts (#1717). let rgba_over = u64::from(w) * u64::from(h) > MAX_RASTER_ALLOC / 4; - (rgba_over || d.total_bytes() > MAX_RASTER_ALLOC).then(|| format!("an image of {w} × {h} pixels is too large to draw")) + let samples_over = d.total_bytes() > MAX_RASTER_ALLOC; + // The renderer can stream a non-interlaced PNG a row at a time when those buffers fit. Its + // full decoded byte count can exceed the allowance even though drawing it at native size is + // safe; use the renderer's decision for that case instead of warning from total_bytes alone. + let png_streams_at_size = samples_over + && format == Some(image::ImageFormat::Png) + && vectorcraft_render::reduced_size(bytes) == Some((w, h)); + (rgba_over || (samples_over && !png_streams_at_size)) + .then(|| format!("an image of {w} × {h} pixels is too large to draw")) } /// What Open, Place and the Links panel say of encoded image `bytes` that [`too_large_note`] is From d2776dddba835a58027cdde2b95112084cda9e81 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 23:29:44 +0000 Subject: [PATCH 10/20] style: format streamed PNG size check --- crates/engine/src/cmd/fileio/load.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/fileio/load.rs b/crates/engine/src/cmd/fileio/load.rs index b1f294d313..0833739aa1 100644 --- a/crates/engine/src/cmd/fileio/load.rs +++ b/crates/engine/src/cmd/fileio/load.rs @@ -852,11 +852,8 @@ pub(crate) fn too_large_note(bytes: &[u8]) -> Option { // The renderer can stream a non-interlaced PNG a row at a time when those buffers fit. Its // full decoded byte count can exceed the allowance even though drawing it at native size is // safe; use the renderer's decision for that case instead of warning from total_bytes alone. - let png_streams_at_size = samples_over - && format == Some(image::ImageFormat::Png) - && vectorcraft_render::reduced_size(bytes) == Some((w, h)); - (rgba_over || (samples_over && !png_streams_at_size)) - .then(|| format!("an image of {w} × {h} pixels is too large to draw")) + let png_streams_at_size = samples_over && format == Some(image::ImageFormat::Png) && vectorcraft_render::reduced_size(bytes) == Some((w, h)); + (rgba_over || (samples_over && !png_streams_at_size)).then(|| format!("an image of {w} × {h} pixels is too large to draw")) } /// What Open, Place and the Links panel say of encoded image `bytes` that [`too_large_note`] is From ead3826edc86b87890e3d5a34e3ce22f7dfe5867 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 23:35:49 +0000 Subject: [PATCH 11/20] fix: compare guessed image format directly --- crates/engine/src/cmd/fileio/load.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/fileio/load.rs b/crates/engine/src/cmd/fileio/load.rs index 0833739aa1..35b333daee 100644 --- a/crates/engine/src/cmd/fileio/load.rs +++ b/crates/engine/src/cmd/fileio/load.rs @@ -852,7 +852,7 @@ pub(crate) fn too_large_note(bytes: &[u8]) -> Option { // The renderer can stream a non-interlaced PNG a row at a time when those buffers fit. Its // full decoded byte count can exceed the allowance even though drawing it at native size is // safe; use the renderer's decision for that case instead of warning from total_bytes alone. - let png_streams_at_size = samples_over && format == Some(image::ImageFormat::Png) && vectorcraft_render::reduced_size(bytes) == Some((w, h)); + let png_streams_at_size = samples_over && format == image::ImageFormat::Png && vectorcraft_render::reduced_size(bytes) == Some((w, h)); (rgba_over || (samples_over && !png_streams_at_size)).then(|| format!("an image of {w} × {h} pixels is too large to draw")) } From eb9c049af0af6e1ddcb580cc70651a188e1becc9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 00:29:35 +0000 Subject: [PATCH 12/20] fix(render): estimate streaming for truncated PNG headers --- crates/render/src/paint.rs | 44 +++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 987ee04710..5ee5055c53 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -260,11 +260,11 @@ fn reduction(bytes: &[u8], allowance: u64) -> Option { let to = fitted_dims(w, h, allowance)?; // A PNG that isn't interlaced or animated (and upright in pre-pixel EXIF) is read a row at a time when the reader's buffers fit the allowance. let row_scratch = if format == image::ImageFormat::Png { - craft_image_codecs::PngDecodeRows::new_streaming(bytes, &craft_image_codecs::Limits::default()) + let streamed = craft_image_codecs::PngDecodeRows::new_streaming(bytes, &craft_image_codecs::Limits::default()) .ok() .filter(|dec| dec.streamable()) - .and_then(|dec| png_row_scratch(&dec.header())) - .filter(|&s| s <= allowance) + .and_then(|dec| png_row_scratch(&dec.header())); + streamed.or_else(|| png_row_scratch_from_header(bytes, w, h, d.total_bytes())).filter(|&s| s <= allowance) } else { None }; @@ -295,6 +295,44 @@ const PNG_WINDOW_AND_STEP: u64 = (32 << 10) + (8 << 10); /// stored row and the row given out come on top. `None` when a row given out doesn't fit memory. fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { let out = u64::from(hdr.width).checked_mul(hdr.layout.channels() as u64)?.checked_mul(hdr.sample.bytes() as u64)?; + png_row_scratch_for_row(out) +} + +/// The row decoder rejects a truncated PNG's tiny IDAT before it can report its header. For a +/// size estimate, the image decoder still gives us the declared output bytes; use those only when +/// the PNG header says it is a static, non-interlaced image, the format the renderer can stream. +fn png_row_scratch_from_header(bytes: &[u8], width: u32, height: u32, total_bytes: u64) -> Option { + const SIG: &[u8; 8] = b"\x89PNG\r\n\x1a\n"; + if bytes.get(..8)? != SIG || height == 0 || total_bytes % u64::from(height) != 0 { + return None; + } + let mut at = 8usize; + let mut header = false; + loop { + let len = u32::from_be_bytes(bytes.get(at..at + 4)?.try_into().ok()?) as usize; + let kind = bytes.get(at + 4..at + 8)?; + let data_at = at.checked_add(8)?; + let end = data_at.checked_add(len)?; + let data = bytes.get(data_at..end)?; + match kind { + b"IHDR" if len == 13 && !header => { + header = true; + if u32::from_be_bytes(data[..4].try_into().ok()?) != width || data[12] != 0 { + return None; + } + } + b"acTL" | b"eXIf" => return None, + b"IDAT" if header => { + let row = total_bytes / u64::from(height); + return row.checked_add(1).and_then(png_row_scratch_for_row); + } + _ => {} + } + at = end.checked_add(4)?; // CRC + } +} + +fn png_row_scratch_for_row(out: u64) -> Option { // A filter byte and the row's samples. let stored = 1u64.checked_add(out)?; let kept = (stored * 4).next_multiple_of(64).max(PNG_KEPT_ROWS); From 118a9b627f97e45b94a1aa3ef716f2eace9ee34e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 00:35:33 +0000 Subject: [PATCH 13/20] fix(render): use integer divisibility helper --- crates/render/src/paint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 5ee5055c53..4dd1253bee 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -303,7 +303,7 @@ fn png_row_scratch(hdr: &craft_image_codecs::PngHeader) -> Option { /// the PNG header says it is a static, non-interlaced image, the format the renderer can stream. fn png_row_scratch_from_header(bytes: &[u8], width: u32, height: u32, total_bytes: u64) -> Option { const SIG: &[u8; 8] = b"\x89PNG\r\n\x1a\n"; - if bytes.get(..8)? != SIG || height == 0 || total_bytes % u64::from(height) != 0 { + if bytes.get(..8)? != SIG || height == 0 || !total_bytes.is_multiple_of(u64::from(height)) { return None; } let mut at = 8usize; From fb45e7ecb5f075efc5b1f25a9c0cabfe6e1e0763 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 00:37:40 +0000 Subject: [PATCH 14/20] fix(render): validate dimensions in PNG header estimate --- crates/render/src/paint.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 4dd1253bee..83f743b92b 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -317,7 +317,11 @@ fn png_row_scratch_from_header(bytes: &[u8], width: u32, height: u32, total_byte match kind { b"IHDR" if len == 13 && !header => { header = true; - if u32::from_be_bytes(data[..4].try_into().ok()?) != width || data[12] != 0 { + if at != 8 + || u32::from_be_bytes(data[..4].try_into().ok()?) != width + || u32::from_be_bytes(data[4..8].try_into().ok()?) != height + || data[12] != 0 + { return None; } } From 2f5c8a6d35c162d267de473f3b43bdd7bd211ffb Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 19:56:11 -0500 Subject: [PATCH 15/20] fix(trace): preserve CMYK and Lab TIFF alpha outlines --- crates/trace/src/opaque.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index 47a2fca6ae..0bfcbf143f 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -34,6 +34,10 @@ pub fn decode_alpha(bytes: &[u8]) -> Result)>, TraceEr RasterColorType::GrayAlpha | RasterColorType::Rgba | RasterColorType::CmykAlpha => true, RasterColorType::Rgb => info.channels > 3, RasterColorType::Gray => info.channels > 1, + // The shared TIFF inspector reports the base photometric type even + // when an ExtraSamples channel carries alpha. + RasterColorType::Cmyk => info.channels > 4, + RasterColorType::Lab => info.channels > 3, RasterColorType::Indexed => true, _ => false, }; @@ -321,4 +325,21 @@ mod tests { assert_eq!(decode_alpha(&encoded(&rgb, Format::Jpeg)).unwrap(), None); assert!(decode_alpha(b"not an image").is_err()); } + + #[test] + fn decode_alpha_preserves_cmyk_and_lab_tiff_extra_samples() { + use craft_image_codecs::{ChannelLayout, Format, Image}; + for (layout, samples) in + [(ChannelLayout::CmykA, vec![10, 20, 30, 40, 0, 50, 60, 70, 80, 255]), (ChannelLayout::LabA, vec![128, 128, 128, 0, 192, 128, 128, 255])] + { + let img = Image::from_u8(2, 1, layout, samples).unwrap(); + assert_eq!(decode_alpha(&encoded(&img, Format::Tiff)).unwrap(), Some((2, 1, vec![0, 255])), "{layout:?}"); + } + for (layout, samples) in + [(ChannelLayout::Cmyk, vec![10, 20, 30, 40, 50, 60, 70, 80]), (ChannelLayout::Lab, vec![128, 128, 128, 192, 128, 128])] + { + let img = Image::from_u8(2, 1, layout, samples).unwrap(); + assert_eq!(decode_alpha(&encoded(&img, Format::Tiff)).unwrap(), None, "{layout:?}"); + } + } } From 28c8b124526368c23cd149eff084f02de5a82f4b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 01:42:22 +0000 Subject: [PATCH 16/20] test(render): correct PNG row scratch thresholds --- crates/render/src/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 99fa434ca3..b562e85865 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1883,14 +1883,14 @@ fn a_png_past_the_allowance_draws_at_reduced_size() { assert_eq!((pm.width(), pm.height(), [c.r, c.g, c.b, c.a]), (300, 300, [200, 40, 40, 255])); // A row is read only while the reader's buffers fit the allowance, counting the stream buffer // three times for the moment it grows. Below 4,096 columns of 16-bit RGBA they take 516,103 - // bytes and 60 a column: 1,398 columns take 599,983 bytes, within 600,000, and 1,399 columns - // take 600,043 and are not decoded (their samples, 8 bytes a pixel, are past it too). + // bytes plus 64 bytes a column: 1,310 columns take 599,943 bytes, within 600,000, and 1,311 + // columns take 600,007 and are not decoded (their samples, 8 bytes a pixel, are past it too). let wide = |w: u32| { let img = image::ImageBuffer::, _>::from_pixel(w, 60, image::Rgba([51400, 10280, 10280, 65535])); encoded(image::DynamicImage::ImageRgba16(img), image::ImageFormat::Png) }; - assert_eq!(with_allowance(600_000, || paint::reduced_size(&wide(1398))), Some((1398, 60))); - assert_eq!(with_allowance(600_000, || paint::reduced_size(&wide(1399))), None); + assert_eq!(with_allowance(600_000, || paint::reduced_size(&wide(1310))), Some((1310, 60))); + assert_eq!(with_allowance(600_000, || paint::reduced_size(&wide(1311))), None); // An image of more pixels than the allowance has bytes is not read: 1,000 × 400 8-bit gray // pixels are drawn at 500 × 200 within 400,000 bytes, and 1,000 × 401 are not. let gray = |h: u32| encoded(image::DynamicImage::ImageLuma8(image::GrayImage::from_pixel(1000, h, image::Luma([90]))), image::ImageFormat::Png); @@ -1938,13 +1938,13 @@ fn a_reduced_decode_counts_what_reading_takes_beside_the_images_paints_hold() { // Decoded whole: 14,400 bytes of RGB and a 16-row band of 8,960 bytes, beside a 73 × 54 result // of 15,768 bytes. let jpeg = encoded(image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel(80, 60, image::Rgb([30, 120, 200]))), image::ImageFormat::Jpeg); - // Read a row at a time: 540,103 bytes in the reader's buffers for 400 columns of 16-bit RGBA, + // Read a row at a time: 541,703 bytes in the reader's buffers for 400 columns of 16-bit RGBA, // beside a 387 × 387 result of 599,076 bytes. let png = encoded( image::DynamicImage::ImageRgba16(image::ImageBuffer::from_pixel(400, 400, image::Rgba([51400, 10280, 10280, 65535]))), image::ImageFormat::Png, ); - for (bytes, allowance, room, size) in [(&jpeg, 16_000, 15_768 + 14_400 + 8_960, (73, 54)), (&png, 600_000, 599_076 + 540_103, (387, 387))] { + for (bytes, allowance, room, size) in [(&jpeg, 16_000, 15_768 + 14_400 + 8_960, (73, 54)), (&png, 600_000, 599_076 + 541_703, (387, 387))] { let failed = decode_failures(); let none = with_allowance(allowance, || with_room(room - 1, || paint::decode_pixmap_or_why(bytes))); assert_eq!(none.err(), Some(paint::Undecoded::NoRoom), "not decoded with a byte too few, and tried again later"); From 845e68008b0d81c5faf6cdc4b87ac928d28cfbfe Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 15 Nov 2026 15:48:44 +0100 Subject: [PATCH 17/20] test(trace): keep the image crate's JPEG encoder for the block-artifact fixture a_jpegs_blocks_are_not_traced_as_steps measures how many traced edges land on JPEG block boundaries, which depends on the encoder's artifacts. With the craft-image-codecs encoder 18 of 40 segments sit on block edges; with the original encoder and the new shared decoder the test passes unchanged, so the trace itself is unaffected. image stays a dev-dependency only. --- Cargo.lock | 1 + crates/trace/Cargo.toml | 2 ++ crates/trace/src/tests.rs | 10 ++++++---- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b58c015a5c..d80b61462e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7034,6 +7034,7 @@ name = "vectorcraft-trace" version = "0.24.1" dependencies = [ "craft-image-codecs", + "image", "kurbo 0.13.1", "serde", "serde_json", diff --git a/crates/trace/Cargo.toml b/crates/trace/Cargo.toml index 83f7425c05..ad76667886 100644 --- a/crates/trace/Cargo.toml +++ b/crates/trace/Cargo.toml @@ -17,3 +17,5 @@ thiserror = { workspace = true } [dev-dependencies] serde_json = { workspace = true } +# Test fixtures only: the JPEG-block test needs the block artifacts of the encoder it was tuned on. +image = { workspace = true } diff --git a/crates/trace/src/tests.rs b/crates/trace/src/tests.rs index 9b73fe784a..6b0f3d3b32 100644 --- a/crates/trace/src/tests.rs +++ b/crates/trace/src/tests.rs @@ -2174,12 +2174,14 @@ fn small_accents_of_other_colors_leave_a_tinted_photo_continuous_tone() { } } -/// `img` saved as a JPEG of `quality` and read back. +/// `img` saved as a JPEG of `quality` and read back. The `image` crate's encoder stays the fixture: +/// `a_jpegs_blocks_are_not_traced_as_steps` measures block artifacts, which differ by encoder. fn through_jpeg(img: &Raster, quality: u8) -> Raster { let rgb: Vec = img.rgba.chunks(4).flat_map(|p| [p[0], p[1], p[2]]).collect(); - let encoded = craft_image_codecs::Image::from_u8(img.width, img.height, craft_image_codecs::ChannelLayout::Rgb, rgb).unwrap(); - let opts = craft_image_codecs::EncodeOptions { jpeg_quality: quality, ..Default::default() }; - let bytes = craft_image_codecs::encode(&encoded, craft_image_codecs::Format::Jpeg, &opts).unwrap(); + let mut bytes = vec![]; + image::codecs::jpeg::JpegEncoder::new_with_quality(&mut bytes, quality) + .encode(&rgb, img.width, img.height, image::ExtendedColorType::Rgb8) + .unwrap(); Raster::decode(&bytes).unwrap() } From d8be891693b08476edf5e569bd7e51f01a93fb95 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 15 Nov 2026 15:49:08 +0100 Subject: [PATCH 18/20] style: rustfmt the ported variable-library parser --- crates/engine/src/cmd/variablecmds/library.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index 3a991a66ef..ceab798373 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -432,8 +432,7 @@ fn graph_value(el: &craft_xml::El, set: &str, cmd: &str) -> Result = data.child(named("propertyRow")).map(|r| r.children(named("value")).skip(1).map(craft_xml::El::deep_text).collect()).unwrap_or_default(); - let rows: Vec<&craft_xml::El> = - data.children(named("values")).flat_map(|v| v.children(named("row"))).take(MAX_GRAPH_CATEGORIES + 1).collect(); + let rows: Vec<&craft_xml::El> = data.children(named("values")).flat_map(|v| v.children(named("row"))).take(MAX_GRAPH_CATEGORIES + 1).collect(); if rows.is_empty() && series.is_empty() { return Ok(None); } From 2ad5fa1734aa5e969733d9bc33df6e5867681f2e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 17:41:07 +0000 Subject: [PATCH 19/20] Preserve renderer source limits for streaming PNG reduction --- crates/render/src/paint.rs | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 83f743b92b..8650ae7886 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -260,7 +260,7 @@ fn reduction(bytes: &[u8], allowance: u64) -> Option { let to = fitted_dims(w, h, allowance)?; // A PNG that isn't interlaced or animated (and upright in pre-pixel EXIF) is read a row at a time when the reader's buffers fit the allowance. let row_scratch = if format == image::ImageFormat::Png { - let streamed = craft_image_codecs::PngDecodeRows::new_streaming(bytes, &craft_image_codecs::Limits::default()) + let streamed = craft_image_codecs::PngDecodeRows::new_streaming(bytes, &png_streaming_limits(allowance)) .ok() .filter(|dec| dec.streamable()) .and_then(|dec| png_row_scratch(&dec.header())); @@ -276,6 +276,13 @@ fn reduction(bytes: &[u8], allowance: u64) -> Option { (rows || d.total_bytes() <= allowance).then_some(Reduction { size: (w, h), to, rows, scratch }) } +/// Streaming retains rows, not the declared full image. Preserve the renderer's source-pixel +/// allowance even when RGBA16's declared samples exceed the shared whole-image default budget. +/// Row scratch and the reduced pixmap are admitted separately before decoding starts. +fn png_streaming_limits(source_pixels: u64) -> craft_image_codecs::Limits { + craft_image_codecs::Limits { max_width: u32::MAX, max_height: u32::MAX, max_pixels: source_pixels, max_alloc: source_pixels.saturating_mul(8) } +} + /// How many rows of an image decoded whole [`reduced_pixmap`] converts to RGBA at a time. const BAND: u32 = 16; @@ -372,7 +379,7 @@ fn reduced_pixmap(bytes: &[u8], r: &Reduction) -> Result Option { - shrunk_with_png_limits(bytes, r, &craft_image_codecs::Limits::default()) + shrunk_with_png_limits(bytes, r, &png_streaming_limits(allowance())) } fn shrunk_with_png_limits(bytes: &[u8], r: &Reduction, png_limits: &craft_image_codecs::Limits) -> Option { @@ -671,6 +678,31 @@ mod tests { assert_eq!(room_for(MAX_DECODE_ALLOC, MAX_DECODE_ALLOC), Ok(())); } + #[test] + fn streaming_header_accepts_rgba16_above_whole_image_default() { + // Header-only fixture: opening the stream checks declared size without allocating or + // decoding the 1.2 GB canvas. Padding meets the decoder's compressed-size plausibility + // bound; no rows are requested from this deliberately incomplete image. + let (w, h) = (10_000u32, 15_000u32); + let mut bytes = b"\x89PNG\r\n\x1a\n".to_vec(); + let ihdr = [&b"IHDR"[..], &w.to_be_bytes(), &h.to_be_bytes(), &[16, 6, 0, 0, 0]].concat(); + let mut idat = b"IDAT\x78\x9c".to_vec(); + idat.resize(1_200_000, 0); + for chunk in [&ihdr[..], &idat, b"IEND"] { + bytes.extend((chunk.len() as u32 - 4).to_be_bytes()); + bytes.extend(chunk); + let mut crc = flate2::Crc::new(); + crc.update(chunk); + bytes.extend(crc.sum().to_be_bytes()); + } + let default = craft_image_codecs::Limits { max_alloc: 1 << 30, max_pixels: 150_000_000, ..Default::default() }; + assert!(craft_image_codecs::PngDecodeRows::new_streaming(&bytes, &default).is_err()); + let rows = craft_image_codecs::PngDecodeRows::new_streaming(&bytes, &png_streaming_limits(150_000_000)).unwrap(); + assert_eq!((rows.header().width, rows.header().height), (w, h)); + assert!(rows.streamable()); + assert!(craft_image_codecs::PngDecodeRows::new_streaming(&bytes, &png_streaming_limits(149_999_999)).is_err()); + } + #[test] fn streaming_reduction_does_not_retain_text_after_pixels() { let mut bytes = Vec::new(); From abe90db45a41de02ba8fde1f9155894ec019e51c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 15 Nov 2026 17:42:47 +0000 Subject: [PATCH 20/20] Cover PNG transparency keys in opaque outline decoding --- crates/trace/src/opaque.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/trace/src/opaque.rs b/crates/trace/src/opaque.rs index 0bfcbf143f..7a0b87d9e0 100644 --- a/crates/trace/src/opaque.rs +++ b/crates/trace/src/opaque.rs @@ -326,6 +326,25 @@ mod tests { assert!(decode_alpha(b"not an image").is_err()); } + #[test] + fn decode_alpha_expands_gray_and_rgb_png_transparency_keys() { + // Original synthetic 2x1 PNGs: first pixel matches tRNS; second is opaque. + // Generated from PNG chunks and zlib scanlines, without an external image asset. + let gray: &[u8] = &[ + 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 2, 0, 0, 0, 1, 8, 0, 0, 0, 0, 209, 73, 32, 86, 0, 0, 0, 2, 116, + 82, 78, 83, 0, 0, 118, 147, 205, 56, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 168, 7, 0, 0, 130, 0, 128, 32, 18, 25, 54, 0, 0, 0, + 0, 73, 69, 78, 68, 174, 66, 96, 130, + ]; + let rgb: &[u8] = &[ + 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 2, 0, 0, 0, 1, 8, 2, 0, 0, 0, 123, 64, 232, 221, 0, 0, 0, 6, 116, + 82, 78, 83, 0, 255, 0, 0, 0, 0, 164, 194, 192, 29, 0, 0, 0, 15, 73, 68, 65, 84, 120, 156, 99, 248, 207, 192, 192, 240, 159, 1, 0, 7, 255, + 1, 255, 1, 127, 137, 167, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130, + ]; + for bytes in [gray, rgb] { + assert_eq!(decode_alpha(bytes).unwrap(), Some((2, 1, vec![0, 255]))); + } + } + #[test] fn decode_alpha_preserves_cmyk_and_lab_tiff_extra_samples() { use craft_image_codecs::{ChannelLayout, Format, Image};