From b73c3dd87a030b6760235f08bf90e9dac36a7147 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sat, 14 Nov 2026 07:42:07 -0500 Subject: [PATCH] Build: restore designcraft_format::write; render decode without the image crate (#2780) * Build: restore designcraft_format::write for recovery; render decodes without naming the image crate * Build: collapse the nested if in images tone (clippy) * IDML export: placements in cells and footnotes embed their PDF layer choices * Tests: Word fixtures carry the document relationships part * PDF/X-4: untagged WebP images carry sRGB * Images: CMYK TIFFs with other inks fall back to RGB; associated alpha keeps its inks craft-image-codecs' TiffPage lists InkSet (storytold/craft-libs#1307), so decode_cmyk_tiff again refuses InkSet 2 instead of reading other inks as C, M, Y and K. Associated alpha is made straight by the decoder; the PDF tests check those inks and that alpha. * PDF tests: CMYK fallback expects the screen's RGB; alpha images read the masked image The RGB fallback for other inks converts through the working CMYK space, as decode_rgba does, so the test takes its expected samples from decode_rgba. image_color_samples took the first image stream, which for an image with alpha is its soft mask. * Images: repin craft-image-codecs to the InkSet enum (storytold/craft-libs#1307) * Pin craft-image-codecs to storytold/craft-libs ea1c842 (#1307) --- Cargo.lock | 81 +++++++++++++++---- Cargo.toml | 2 +- crates/engine/src/cmd/links.rs | 9 +-- crates/engine/src/cmd/place_text.rs | 40 +++++---- crates/engine/src/cmd/place_text/tests.rs | 13 +-- crates/format/src/lib.rs | 10 +++ crates/idml/src/export/item.rs | 4 +- crates/idml/src/xml.rs | 4 +- crates/images/src/lib.rs | 23 ++++-- crates/images/src/tone.rs | 8 +- crates/pdf/src/export/images.rs | 52 +++++------- .../support/image_content_cache_contract.rs | 23 +++--- crates/pdf/tests/support/image_fixtures.rs | 4 +- crates/render/src/decode.rs | 10 +-- crates/textimport/src/docx.rs | 4 +- crates/textimport/src/docx/tests.rs | 3 +- 16 files changed, 171 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d0f6a9d9..93af4350a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -161,13 +161,28 @@ version = "2.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" +[[package]] +name = "alloc-no-stdlib" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2fb6cfd47bf496ff64095c20eaba0c201404ee38714d4142fcfa1dc334fcc7a" + [[package]] name = "alloc-stdlib" version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e76a019e91224d279006ff972f1e984179a6e9feb050adba6ce8274aef23195" dependencies = [ - "alloc-no-stdlib", + "alloc-no-stdlib 2.0.4", +] + +[[package]] +name = "alloc-stdlib" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b5c1865780388bfa186411ab5f247819487fc4864c6e9c3106611fa347586e1" +dependencies = [ + "alloc-no-stdlib 3.0.0", ] [[package]] @@ -644,9 +659,9 @@ version = "8.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5cc91aac060a7a1e25823bdccbfb6af1875b88f17c6daac97894eed8207166b3" dependencies = [ - "alloc-no-stdlib", - "alloc-stdlib", - "brotli-decompressor", + "alloc-no-stdlib 2.0.4", + "alloc-stdlib 0.2.4", + "brotli-decompressor 5.0.3", ] [[package]] @@ -655,8 +670,18 @@ version = "5.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a32acac15fe1967bc3986b2a6347dffc965602354ea6f450ad07e8bfd253583" dependencies = [ - "alloc-no-stdlib", - "alloc-stdlib", + "alloc-no-stdlib 2.0.4", + "alloc-stdlib 0.2.4", +] + +[[package]] +name = "brotli-decompressor" +version = "6.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75c74c9c51b9140663e196b5fb88bc94c334d7a497c8311cb759d8557874747f" +dependencies = [ + "alloc-no-stdlib 3.0.0", + "alloc-stdlib 0.3.0", ] [[package]] @@ -1273,9 +1298,9 @@ dependencies = [ [[package]] name = "craft-image-codecs" version = "0.1.0" -source = "git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895#2e670cf9377b523112f3fe4b409399772c745895" +source = "git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f#cc23ffd4d26f3074808e2187d332564817624f8f" dependencies = [ - "craft-jpeg 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-jpeg 0.1.0 (git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f)", "exr", "flate2", "half", @@ -1295,9 +1320,10 @@ dependencies = [ [[package]] name = "craft-image-codecs" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f#cc23ffd4d26f3074808e2187d332564817624f8f" +source = "git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6#ea1c84290dadec636900b4d1bce107e59ca3f0d6" dependencies = [ - "craft-jpeg 0.1.0 (git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f)", + "craft-jpeg 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", + "craft-metadata", "exr", "flate2", "half", @@ -1347,7 +1373,7 @@ dependencies = [ [[package]] name = "craft-jpeg" version = "0.1.0" -source = "git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895#2e670cf9377b523112f3fe4b409399772c745895" +source = "git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f#cc23ffd4d26f3074808e2187d332564817624f8f" dependencies = [ "thiserror 2.0.21", ] @@ -1355,7 +1381,7 @@ dependencies = [ [[package]] name = "craft-jpeg" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f#cc23ffd4d26f3074808e2187d332564817624f8f" +source = "git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6#ea1c84290dadec636900b4d1bce107e59ca3f0d6" dependencies = [ "thiserror 2.0.21", ] @@ -1389,6 +1415,18 @@ dependencies = [ "serde", ] +[[package]] +name = "craft-metadata" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6#ea1c84290dadec636900b4d1bce107e59ca3f0d6" +dependencies = [ + "brotli-decompressor 6.3.1", + "chrono", + "craft-tiff", + "quick-xml 0.41.0", + "serde", +] + [[package]] name = "craft-offcrypto" version = "0.1.0" @@ -1573,6 +1611,15 @@ dependencies = [ "thiserror 2.0.21", ] +[[package]] +name = "craft-tiff" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6#ea1c84290dadec636900b4d1bce107e59ca3f0d6" +dependencies = [ + "serde", + "thiserror 2.0.21", +] + [[package]] name = "craft-ui" version = "0.1.0" @@ -1990,7 +2037,7 @@ name = "designcraft-cli" version = "0.20.1" dependencies = [ "craft-file-formats", - "craft-image-codecs 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-image-codecs 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", "designcraft-color", "designcraft-compose", "designcraft-doc", @@ -2164,7 +2211,7 @@ dependencies = [ "craft-cms", "craft-eps 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ba24fe2435adb3c81ad6057faf36365fbba736cf)", "craft-file-formats", - "craft-image-codecs 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-image-codecs 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", "craft-psd", "craft-svg 0.1.0 (git+https://github.com/storytold/craft-libs?rev=cc23ffd4d26f3074808e2187d332564817624f8f)", "designcraft-color", @@ -2198,7 +2245,7 @@ version = "0.20.1" dependencies = [ "chrono", "craft-cms", - "craft-image-codecs 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-image-codecs 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", "craft-pdf", "craft-prepress", "designcraft-color", @@ -2223,7 +2270,7 @@ name = "designcraft-pptx" version = "0.20.1" dependencies = [ "chrono", - "craft-image-codecs 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-image-codecs 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", "craft-opc 0.1.0 (git+https://github.com/storytold/craft-libs?rev=8abfa9d0f14bafbfc3449fc552ffeafb9ca6a551)", "craft-pptx", "designcraft-color", @@ -2244,7 +2291,7 @@ name = "designcraft-render" version = "0.20.1" dependencies = [ "bytemuck", - "craft-image-codecs 0.1.0 (git+https://github.com/dylanmtaylor/craft-libs?rev=2e670cf9377b523112f3fe4b409399772c745895)", + "craft-image-codecs 0.1.0 (git+https://github.com/storytold/craft-libs?rev=ea1c84290dadec636900b4d1bce107e59ca3f0d6)", "craft-pdf", "designcraft-color", "designcraft-compose", diff --git a/Cargo.toml b/Cargo.toml index 34cd5cbc9..aa498ab1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ craft-xlsx = { git = "https://github.com/storytold/craft-libs", rev = "2b9a325f5 craft-docx = { git = "https://github.com/storytold/craft-libs", rev = "ba24fe2435adb3c81ad6057faf36365fbba736cf", version = "=0.1.0" } craft-wordmodel = { git = "https://github.com/storytold/craft-libs", rev = "ba24fe2435adb3c81ad6057faf36365fbba736cf", version = "=0.1.0" } craft-csv = { git = "https://github.com/storytold/craft-libs", rev = "387c64892b30fdad491b263cc3d9eac660d88dcc", version = "=0.1.0" } -craft-image-codecs = { git = "https://github.com/dylanmtaylor/craft-libs", rev = "2e670cf9377b523112f3fe4b409399772c745895", version = "=0.1.0" } +craft-image-codecs = { git = "https://github.com/storytold/craft-libs", rev = "ea1c84290dadec636900b4d1bce107e59ca3f0d6", version = "=0.1.0" } craft-psd = { git = "https://github.com/dylanmtaylor/craft-libs", rev = "2e670cf9377b523112f3fe4b409399772c745895", version = "=0.1.0" } craft-file-formats = { git = "https://github.com/storytold/craft-libs", rev = "eed3c28cf016723d004d6f2c79c61424c15d4558", version = "=0.1.0" } craft-opc = { git = "https://github.com/storytold/craft-libs", rev = "8abfa9d0f14bafbfc3449fc552ffeafb9ca6a551", version = "=0.1.0" } diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 22e31cf64..8ed455adf 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -1095,7 +1095,6 @@ mod text_link_tests { /// A small Word file: a Title paragraph, then Body paragraphs (`body` is WordprocessingML). fn docx(body: &str) -> Vec { - use std::io::Write; const W: &str = r#"xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main""#; let styles = format!( r#" @@ -1105,13 +1104,7 @@ mod text_link_tests { r#"Head {body}"# ); - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - let o = zip::write::SimpleFileOptions::default(); - for (n, c) in [("word/document.xml", doc), ("word/styles.xml", styles)] { - w.start_file(n, o).unwrap(); - w.write_all(c.as_bytes()).unwrap(); - } - w.finish().unwrap().into_inner() + crate::cmd::place_text::word_package(&doc, &styles) } fn text_rows(s: &mut Session) -> Vec { diff --git a/crates/engine/src/cmd/place_text.rs b/crates/engine/src/cmd/place_text.rs index f80d3fba8..f994b0ddb 100644 --- a/crates/engine/src/cmd/place_text.rs +++ b/crates/engine/src/cmd/place_text.rs @@ -506,6 +506,29 @@ pub(super) fn place_text( }) } +/// A minimal Word package for tests: the document and its styles, with the content types and +/// relationships that tell a reader where those parts are. +#[cfg(test)] +pub(crate) fn word_package(document: &str, styles: &str) -> Vec { + use std::io::Write; + const CONTENT_TYPES: &str = r#""#; + const ROOT_RELS: &str = r#""#; + const DOCUMENT_RELS: &str = r#""#; + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let o = zip::write::SimpleFileOptions::default(); + for (n, c) in [ + ("[Content_Types].xml", CONTENT_TYPES), + ("_rels/.rels", ROOT_RELS), + ("word/_rels/document.xml.rels", DOCUMENT_RELS), + ("word/document.xml", document), + ("word/styles.xml", styles), + ] { + w.start_file(n, o).unwrap(); + w.write_all(c.as_bytes()).unwrap(); + } + w.finish().unwrap().into_inner() +} + #[cfg(test)] mod tests; @@ -710,7 +733,6 @@ mod pdf_page_tests { } fn word_file() -> Vec { - use std::io::Write; const W: &str = r#"xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main""#; let styles = format!( r#" @@ -721,13 +743,7 @@ mod pdf_page_tests { r#"Head Text"# ); - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - let o = zip::write::SimpleFileOptions::default(); - for (n, c) in [("word/document.xml", doc), ("word/styles.xml", styles)] { - w.start_file(n, o).unwrap(); - w.write_all(c.as_bytes()).unwrap(); - } - w.finish().unwrap().into_inner() + super::word_package(&doc, &styles) } #[test] @@ -770,19 +786,13 @@ mod pdf_page_tests { #[test] fn word_normal_comes_in_as_its_own_style() { // Regression (#722): Normal's formatting (over Word's document defaults) was dropped. - use std::io::Write; const W: &str = r#"xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main""#; let styles = format!( r#" "# ); let doc = format!(r#"Plain"#); - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - for (n, c) in [("word/document.xml", doc), ("word/styles.xml", styles)] { - w.start_file(n, zip::write::SimpleFileOptions::default()).unwrap(); - w.write_all(c.as_bytes()).unwrap(); - } - let b64 = crate::cmd::base64_encode(&w.finish().unwrap().into_inner()); + let b64 = crate::cmd::base64_encode(&super::word_package(&doc, &styles)); let basic = designcraft_doc::story::BASIC_PARAGRAPH; let placed = |s: &Session, story: &serde_json::Value| { let d = &s.doc().unwrap().doc; diff --git a/crates/engine/src/cmd/place_text/tests.rs b/crates/engine/src/cmd/place_text/tests.rs index 8275e7ab2..cc8fda19a 100644 --- a/crates/engine/src/cmd/place_text/tests.rs +++ b/crates/engine/src/cmd/place_text/tests.rs @@ -311,7 +311,6 @@ fn autoflow_adds_the_pages_odd_and_even_page_breaks_ask_for() { #[test] fn autoflow_sizes_no_frame_by_one_a_start_option_ends_early() { // Regression (#1209): the title frame a start option ends early isn't a frame's worth of text. - use std::io::Write; const W: &str = r#"xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main""#; let para = |style: &str, text: &str| format!(r#"{text}"#); let body = [para("Title", "Front"), para("Chapter", "Chapter one"), para("Body", "lorem ipsum dolor sit amet ".repeat(60).trim())].concat(); @@ -319,16 +318,8 @@ fn autoflow_sizes_no_frame_by_one_a_start_option_ends_early() { .iter() .map(|s| format!(r#""#)) .collect(); - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - let o = zip::write::SimpleFileOptions::default(); - for (n, c) in [ - ("word/document.xml", format!(r#"{body}"#)), - ("word/styles.xml", format!(r#"{styles}"#)), - ] { - w.start_file(n, o).unwrap(); - w.write_all(c.as_bytes()).unwrap(); - } - let docx = w.finish().unwrap().into_inner(); + let docx = + super::word_package(&format!(r#"{body}"#), &format!(r#"{styles}"#)); for (start, pages) in [("nextOddPage", 5), ("nextEvenPage", 4)] { let mut s = Session::new(); s.execute("file.new", &json!({"pages": 3, "facingPages": true})).unwrap(); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index bcfacd976..a05a8baa7 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -36,6 +36,16 @@ pub fn save_with_preview(doc: &Document, preview: Option<&[u8]>) -> Result(out: &mut W, doc: &Document, preview: Option<&[u8]>) -> Result<(), FormatError> { + let bytes = save_with_preview(doc, preview)?; + for chunk in bytes.chunks(1 << 20) { + out.write_all(chunk).map_err(|e| FormatError::Io(e.to_string()))?; + } + Ok(()) +} + /// Required native format version for a document. Kept for app-side compatibility; inference is /// performed by the shared codec over the serialized neutral document schema. #[cfg(test)] diff --git a/crates/idml/src/export/item.rs b/crates/idml/src/export/item.rs index bfcc0619c..fd5731133 100644 --- a/crates/idml/src/export/item.rs +++ b/crates/idml/src/export/item.rs @@ -1,7 +1,5 @@ use super::common::{affine_s, bool_s, pct, uid, with_props}; -use super::{ - Affine, ColumnsKind, Content, El, Ex, Item, Node, OBJECT_BUILTINS, PathData, Shape, TextFrameOptions, base64_encode, names, num, pt, -}; +use super::{Affine, ColumnsKind, Content, El, Ex, Item, Node, OBJECT_BUILTINS, PathData, Shape, TextFrameOptions, base64_encode, names, num, pt}; impl<'a> Ex<'a> { pub(super) fn item_el(&mut self, it: &Item, xf: Affine) -> El { diff --git a/crates/idml/src/xml.rs b/crates/idml/src/xml.rs index 8926062ec..d09df0400 100644 --- a/crates/idml/src/xml.rs +++ b/crates/idml/src/xml.rs @@ -236,7 +236,9 @@ pub(crate) fn from_craft(el: craft_xml::El) -> El { .filter_map(|n| match n { craft_xml::Node::El(e) => Some(Node::El(from_craft(e))), craft_xml::Node::Text(t) => Some(Node::Text(t)), - craft_xml::Node::CData(t) => Some(Node::Text(t)), + // The parser joins CDATA to the text around it, so only trees built for export + // (`` image data passed through `with_props`) carry sections here. + craft_xml::Node::CData(t) => Some(Node::CData(t)), craft_xml::Node::Pi { target, data } => Some(Node::Pi(target, data.trim().to_string())), craft_xml::Node::Comment(_) => None, }) diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index 7efabb115..efb0a83cf 100644 --- a/crates/images/src/lib.rs +++ b/crates/images/src/lib.rs @@ -512,22 +512,31 @@ pub struct CmykRaster { pub alpha: Option>, } +/// The TIFF page that decoding opens; `None` for other formats. +fn default_tiff_page(bytes: &[u8]) -> Option { + let mut info = craft_image_codecs::tiff_info(bytes).ok()?; + let page = info.default_page()?; + (page < info.pages.len()).then(|| info.pages.swap_remove(page)) +} + /// A TIFF whose pixels are CMYK (PhotometricInterpretation Separated, four inks). pub fn is_cmyk_tiff(bytes: &[u8]) -> bool { - craft_image_codecs::tiff_info(bytes) - .ok() - .and_then(|info| info.default_page().and_then(|i| info.pages.get(i).map(|page| (page.photometric, page.samples_per_pixel)))) - .is_some_and(|(photometric, samples)| photometric == Some(5) && matches!(samples, 4 | 5)) + default_tiff_page(bytes).is_some_and(|page| page.photometric == Some(5) && matches!(page.samples_per_pixel, 4 | 5)) } /// The ink values of a CMYK TIFF, which [`decode_rgba`] converts to RGB. 8- or 16-bit CMYK, -/// pixels interleaved, opaque or with unassociated alpha. `None` for anything else — another -/// format, another ink set (InkSet 2), planar storage or premultiplied alpha — and for files -/// over the decoder's 256 MiB limit; [`is_cmyk_tiff`] tells those apart from RGB files. +/// opaque or with alpha (associated alpha is made straight). `None` for anything else — +/// another format or another ink set (InkSet 2) — and for files over the decoder's 256 MiB +/// limit; [`is_cmyk_tiff`] tells those apart from RGB files. pub fn decode_cmyk_tiff(bytes: &[u8]) -> Option { if too_large_to_decode(bytes) { return None; } + // InkSet (tag 332): absent or 1 = CMYK; 2 = the inks named in InkNames; other values are + // undefined. Only CMYK samples are C, M, Y and K; reading others as such prints wrong colours. + if default_tiff_page(bytes).is_some_and(|page| page.ink_set.is_some_and(|set| set != craft_image_codecs::InkSet::Cmyk)) { + return None; + } let image = craft_image_codecs::decode_with(bytes, &decode_options()).ok()?; image.layout().is_cmyk().then(|| cmyk_raster(&image, bytes)).flatten() } diff --git a/crates/images/src/tone.rs b/crates/images/src/tone.rs index b5db792bb..0ec3f4da8 100644 --- a/crates/images/src/tone.rs +++ b/crates/images/src/tone.rs @@ -13,10 +13,10 @@ pub enum Tone { /// The [`Tone`] of an image file, `None` when a swatch doesn't colour it. pub fn tone(bytes: &[u8]) -> Option { - if let Some(info) = craft_image_codecs::inspect_raster(bytes).ok().flatten() { - if info.channels > 1 { - return None; - } + if let Some(info) = craft_image_codecs::inspect_raster(bytes).ok().flatten() + && info.channels > 1 + { + return None; } match craft_image_codecs::classify_tone(bytes)? { craft_image_codecs::ToneClass::Bitmap => Some(Tone::Bitmap), diff --git a/crates/pdf/src/export/images.rs b/crates/pdf/src/export/images.rs index 4ddafa82d..c3816f758 100644 --- a/crates/pdf/src/export/images.rs +++ b/crates/pdf/src/export/images.rs @@ -17,8 +17,7 @@ pub(super) struct ConvertedImage { pub(super) warning: Option<&'static str>, } -pub(super) const CMYK_FALLBACK_WARNING: &str = - "converted to RGB (a CMYK TIFF that is planar, has premultiplied alpha, uses other inks or is over 256 MiB)"; +pub(super) const CMYK_FALLBACK_WARNING: &str = "converted to RGB (a CMYK TIFF that uses other inks or is over 256 MiB)"; #[cfg(not(test))] pub(super) type ImageContentCache = HashMap>, ConvertedImage>; @@ -84,13 +83,10 @@ pub(super) fn jpeg(data: krilla::Data, icc: impl FnOnce() -> Option>, mo } /// Whether krilla would write this PNG, GIF or WebP as untagged RGB: not grey, and no profile of -/// its own. +/// its own. The header inspection doesn't read WebP, which has no grey form, so a file it can't +/// read counts as colour. fn untagged_rgb(data: &[u8]) -> bool { - let Some(info) = craft_image_codecs::inspect_raster(data).ok().flatten() else { return false }; - !matches!( - info.color_type, - craft_image_codecs::RasterColorType::Bitmap | craft_image_codecs::RasterColorType::Gray | craft_image_codecs::RasterColorType::GrayAlpha - ) && embedded_icc(data).is_none() + !is_grey(data) && embedded_icc(data).is_none() } pub(super) fn lossless(data: &Arc>, fmt: Option, mode: RasterMode) -> Option { @@ -392,28 +388,18 @@ mod image_content_cache_tests { let opts = PdfOptions { standard, ..Default::default() }; let mut ex = exporter(&d, &cache, &opts); ex.rgb_only = standard == Standard::PdfA2b; - // Unassociated alpha (2) decodes as inks, and as RGB for RGB-only exports. - // Associated alpha is rejected by both paths: retain both failure names. - let succeeds = alpha.is_none() || alpha == Some(2); - let first = ex.load_image(a); - assert_eq!(first.is_some(), succeeds, "{alpha:?} {standard:?}"); - if alpha.is_none() { - let (space, samples) = image_color_samples(first.unwrap()); - assert_eq!(space, if ex.rgb_only { "DeviceRGB" } else { "DeviceCMYK" }); - assert_eq!(samples, if ex.rgb_only { screen_rgb.clone() } else { crate::image_fixtures::INKS.to_vec() }); - } - assert_eq!(ex.load_image(b).is_some(), succeeds, "{alpha:?} {standard:?}"); - assert_eq!(ex.load_image(a).is_some(), succeeds); - assert_eq!(ex.image_conversions, if succeeds { 1 } else { 2 }); - assert_eq!(ex.images_by_content.len(), usize::from(succeeds)); - if succeeds { - assert!(ex.warnings.is_empty()); - } else { - for name in ["a.tif", "b.tif"] { - assert!(ex.warnings.contains(&format!("image `{name}` could not be decoded and was skipped"))); - assert_eq!(ex.warnings.contains(&format!("{name}: {CMYK_FALLBACK_WARNING}")), !ex.rgb_only); - } - } + // Every alpha kind decodes as inks, and as RGB for RGB-only exports; associated + // alpha (1) is made straight, so it gives the same inks as unassociated (2). + let first = ex.load_image(a).unwrap(); + let (space, samples) = image_color_samples(first); + assert_eq!(space, if ex.rgb_only { "DeviceRGB" } else { "DeviceCMYK" }, "{alpha:?} {standard:?}"); + let want = if ex.rgb_only { screen_rgb.clone() } else { crate::image_fixtures::INKS.to_vec() }; + assert_eq!(samples, want, "{alpha:?} {standard:?}"); + assert!(ex.load_image(b).is_some(), "{alpha:?} {standard:?}"); + assert!(ex.load_image(a).is_some()); + assert_eq!(ex.image_conversions, 1); + assert_eq!(ex.images_by_content.len(), 1); + assert!(ex.warnings.is_empty()); } } } @@ -421,7 +407,7 @@ mod image_content_cache_tests { // Serialize the converted raster without an archival validator so the RGB-only pixel // contract stays independent of main's existing PDF/A interpolation validation failure. fn image_color_samples(image: Image) -> (String, Vec) { - use craft_pdf::syntax::object::Name; + use craft_pdf::syntax::object::{Name, Stream}; let mut pdf = krilla::Document::new(); let size = Size::from_wh(20.0, 20.0).unwrap(); let mut page = pdf.start_page_with(PageSettings::new(size)); @@ -435,7 +421,9 @@ mod image_content_cache_tests { .objects() .into_iter() .filter_map(|o| o.into_stream()) - .find(|s| s.dict().get::("Subtype").is_some_and(|n| n.as_str() == "Image")) + .filter(|s| s.dict().get::("Subtype").is_some_and(|n| n.as_str() == "Image")) + // An image with alpha comes with its soft mask, another image; take the masked one. + .max_by_key(|s| s.dict().get::>("SMask").is_some()) .unwrap(); (stream.dict().get::("ColorSpace").unwrap().as_str().to_owned(), stream.decoded().unwrap().into_owned()) } diff --git a/crates/pdf/tests/support/image_content_cache_contract.rs b/crates/pdf/tests/support/image_content_cache_contract.rs index fc2b53c9d..d86d35310 100644 --- a/crates/pdf/tests/support/image_content_cache_contract.rs +++ b/crates/pdf/tests/support/image_content_cache_contract.rs @@ -230,20 +230,22 @@ fn successful_fallback_warns_for_both_names_and_preserves_rgb_samples() { for (name, x) in [("other-inks-a.tif", 36.0), ("other-inks-b.tif", 66.0)] { place_named(&mut d, bytes.clone(), x, name, "image/tiff", 0); } - let reason = "converted to RGB (a CMYK TIFF that is planar, has premultiplied alpha, uses other inks or is over 256 MiB)"; + let reason = "converted to RGB (a CMYK TIFF that uses other inks or is over 256 MiB)"; let expected_warnings = [format!("other-inks-a.tif: {reason}"), format!("other-inks-b.tif: {reason}")]; + // The fallback is the RGB the screen shows: converted through the working CMYK space. + let rgb: Vec = designcraft_images::decode_rgba(&bytes).unwrap().pixels().flat_map(|p| [p[0], p[1], p[2]]).collect(); for compress_images in [false, true] { let report = export_pdf_with_report(&d, &Cache::new(), &PdfOptions { compress_images, ..Default::default() }).unwrap(); assert_eq!(report.warnings, expected_warnings); let images = image_objects(&report.bytes); assert_eq!(images.len(), 1); assert_eq!(images[0].space, "DeviceRGB"); - assert_eq!(images[0].samples, [0, 0, 0, 255, 0, 0]); + assert_eq!(images[0].samples, rgb); assert_eq!(images[0].size, (2, 1)); } let booklet = export_booklet(&d, &Cache::new(), &BookletOptions::default()).unwrap(); assert_eq!(booklet.warnings, expected_warnings); - assert_eq!(image_objects(&booklet.bytes)[0].samples, [0, 0, 0, 255, 0, 0]); + assert_eq!(image_objects(&booklet.bytes)[0].samples, rgb); } #[test] @@ -272,19 +274,20 @@ fn pdfa_rgb_conversion_does_not_poison_later_cmyk_exports() { } #[test] -fn unsupported_associated_alpha_keeps_each_asset_failure_warning() { +fn associated_alpha_exports_straight_inks_once() { + // The fixture stores the inks multiplied by alpha; the export carries them made straight. let bytes = fixtures::cmyk_tiff(Some(1), false); let mut d = Document::new(&Default::default()); for (name, x) in [("alpha-a.tif", 36.0), ("alpha-b.tif", 66.0)] { place_named(&mut d, bytes.clone(), x, name, "image/tiff", 0); } let report = export_pdf_with_report(&d, &Cache::new(), &PdfOptions::default()).unwrap(); - assert!(image_objects(&report.bytes).is_empty()); - assert_eq!(report.warnings.len(), 4); - for name in ["alpha-a.tif", "alpha-b.tif"] { - assert!(report.warnings.iter().any(|w| w.starts_with(&format!("{name}: converted to RGB")))); - assert!(report.warnings.contains(&format!("image `{name}` could not be decoded and was skipped"))); - } + assert!(report.warnings.is_empty(), "{:?}", report.warnings); + let images = image_objects(&report.bytes); + let cmyk: Vec<_> = images.iter().filter(|i| i.space == "DeviceCMYK").collect(); + assert_eq!(cmyk.len(), 1, "{images:?}"); + assert_eq!(cmyk[0].samples, fixtures::INKS); + assert_eq!(cmyk[0].alpha.as_deref(), Some(&fixtures::ALPHA[..])); } fn two_page_pdf() -> Vec { diff --git a/crates/pdf/tests/support/image_fixtures.rs b/crates/pdf/tests/support/image_fixtures.rs index 02dafae20..c59a02e0c 100644 --- a/crates/pdf/tests/support/image_fixtures.rs +++ b/crates/pdf/tests/support/image_fixtures.rs @@ -11,7 +11,9 @@ pub fn cmyk_tiff(alpha: Option, other_inks: bool) -> Vec { if let Some(kind) = alpha { let mut image = encoder.new_image::(2, 1).unwrap(); image.encoder().write_tag(Tag::ExtraSamples, &[kind][..]).unwrap(); - image.write_data(&[0, 0, 0, 255, ALPHA[0], 0, 255, 255, 0, ALPHA[1]]).unwrap(); + // Associated alpha (1) stores each ink multiplied by alpha: 255 at alpha 128 is 128. + let ink = if kind == 1 { 128 } else { 255 }; + image.write_data(&[0, 0, 0, 255, ALPHA[0], 0, ink, ink, 0, ALPHA[1]]).unwrap(); } else { let mut image = encoder.new_image::(2, 1).unwrap(); if other_inks { diff --git a/crates/render/src/decode.rs b/crates/render/src/decode.rs index f6bc7339b..cc4d9f859 100644 --- a/crates/render/src/decode.rs +++ b/crates/render/src/decode.rs @@ -18,22 +18,22 @@ pub fn decode_pixmap_page(bytes: &[u8], page: u32) -> Option { let (px, w, h) = designcraft_images::render_svg(bytes, 3000)?; return Some(Pixmap::from_parts(premul_pixels(px), w.min(u16::MAX as u32) as u16, h.min(u16::MAX as u32) as u16)); } - premultiplied(designcraft_images::decode_rgba(bytes)?) + let img = designcraft_images::decode_rgba(bytes)?; + premultiplied(img.width(), img.height(), img.into_raw()) } /// [`decode_pixmap`] of a JPEG at 1/2^`shrink` of its size, `shrink` 1 to /// [`designcraft_images::MAX_REDUCED_SHRINK`], without a full-size decode. `None` where only a /// full decode applies (other formats, CMYK JPEGs). pub(crate) fn decode_pixmap_reduced(bytes: &[u8], shrink: u8) -> Option { - premultiplied(designcraft_images::decode_rgba_reduced(bytes, shrink)?) + let img = designcraft_images::decode_rgba_reduced(bytes, shrink)?; + premultiplied(img.width(), img.height(), img.into_raw()) } -fn premultiplied(img: image::RgbaImage) -> Option { - let (w, h) = img.dimensions(); +fn premultiplied(w: u32, h: u32, mut px: Vec) -> Option { if w == 0 || h == 0 || w > u16::MAX as u32 || h > u16::MAX as u32 { return None; } - let mut px = img.into_raw(); px.truncate(w as usize * h as usize * 4); Some(Pixmap::from_parts(premultiply(px), w as u16, h as u16)) } diff --git a/crates/textimport/src/docx.rs b/crates/textimport/src/docx.rs index b59426f25..b7bd274df 100644 --- a/crates/textimport/src/docx.rs +++ b/crates/textimport/src/docx.rs @@ -6,9 +6,7 @@ use std::collections::{HashMap, HashSet}; use craft_wordmodel::Block; use craft_wordmodel::numbering::LevelSuffix; use craft_wordmodel::para::{InlineObject, NoteKind, Paragraph}; -use craft_wordmodel::props::{ - Align as WordAlign, CharProps as WordCharProps, NumRef, ParaProps as WordParaProps, Underline, VertAlign, -}; +use craft_wordmodel::props::{Align as WordAlign, CharProps as WordCharProps, NumRef, ParaProps as WordParaProps, Underline, VertAlign}; use craft_wordmodel::section::NumFormat; use craft_wordmodel::styles::{StyleKind, StyleSheet}; use designcraft_doc::story::{BASIC_PARAGRAPH, NO_CHAR_STYLE}; diff --git a/crates/textimport/src/docx/tests.rs b/crates/textimport/src/docx/tests.rs index c7fb3ff28..d49b2a2cc 100644 --- a/crates/textimport/src/docx/tests.rs +++ b/crates/textimport/src/docx/tests.rs @@ -60,7 +60,8 @@ fn docx_parts(parts: &[(&str, &str)]) -> Vec { } if !rels.is_empty() { w.start_file("word/_rels/document.xml.rels", o).unwrap(); - let rels_doc = format!(r#"{}"#, rels.join("")); + let rels_doc = + format!(r#"{}"#, rels.join("")); w.write_all(rels_doc.as_bytes()).unwrap(); } w.finish().unwrap().into_inner()