Merge branch 'print-color-prepress-fixes' into print-color

.github/workflows/pdf.yml

@@ -3,9 +3,9 @@ name: Shared PDF
 on:
   push:
     branches: [main]
-    paths: ["crates/pdf/**", "crates/plate-test/**", "crates/prepress/**", "crates/cms/**", "vendor/hayro*/**", "vendor/krilla/**", "scripts/test-pdf-backends.sh", "scripts/*fetch-pdf-test-fonts.py", "Cargo.toml", "Cargo.lock", ".github/workflows/pdf.yml"]
+    paths: ["crates/pdf/**", "crates/plate-test/**", "crates/prepress/**", "vendor/hayro*/**", "vendor/krilla/**", "scripts/test-pdf-backends.sh", "scripts/*fetch-pdf-test-fonts.py", "Cargo.toml", "Cargo.lock", ".github/workflows/pdf.yml"]
   pull_request:
-    paths: ["crates/pdf/**", "crates/plate-test/**", "crates/prepress/**", "crates/cms/**", "vendor/hayro*/**", "vendor/krilla/**", "scripts/test-pdf-backends.sh", "scripts/*fetch-pdf-test-fonts.py", "Cargo.toml", "Cargo.lock", ".github/workflows/pdf.yml"]
+    paths: ["crates/pdf/**", "crates/plate-test/**", "crates/prepress/**", "vendor/hayro*/**", "vendor/krilla/**", "scripts/test-pdf-backends.sh", "scripts/*fetch-pdf-test-fonts.py", "Cargo.toml", "Cargo.lock", ".github/workflows/pdf.yml"]
   workflow_dispatch:
 
 permissions:
@@ -91,11 +91,20 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@stable
+      - uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable
         with:
           components: clippy
-      - uses: Swatinem/rust-cache@v2
+      - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3
         with:
           key: craft-plate-test
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+      - name: Provision licensed PDF test fonts
+        shell: bash
+        run: |
+          python scripts/test-fetch-pdf-test-fonts.py
+          python scripts/fetch-pdf-test-fonts.py "$RUNNER_TEMP/craft-pdf-test-fonts"
+          echo "CRAFT_PDF_TEST_FONT_DIR=$RUNNER_TEMP/craft-pdf-test-fonts" >> "$GITHUB_ENV"
       - run: cargo clippy -p craft-plate-test -p craft-prepress --all-targets --locked -- -D warnings
       - run: cargo test -p craft-plate-test -p craft-prepress --locked

crates/plate-test/tests/scenes.rs

@@ -8,6 +8,16 @@ fn scenes() -> Vec<Scene> {
     scene::load_dir(&scene::scenes_dir()).unwrap()
 }
 
+/// On CI (`CI` set) the workflow provisions the font, so a scene that left text out fails.
+/// Local runs without the font skip text and only note it.
+fn text_must_be_written() -> bool {
+    std::env::var_os("CI").is_some_and(|v| !v.is_empty() && v != "false")
+}
+
+fn missing_font(scene: &Scene) -> String {
+    format!("{}: text skipped; {} must name a directory with {} on CI", scene.id, reference::FONT_DIR_ENV, reference::FONT_FILE)
+}
+
 #[test]
 fn every_scene_parses_and_is_consistent() {
     let scenes = scenes();
@@ -21,7 +31,7 @@ fn every_scene_parses_and_is_consistent() {
 }
 
 /// Non-pending scenes are written and match their `structure` block (with text left out for
-/// lack of fonts, a mismatch is only a note). Pending scenes must
+/// lack of fonts, a mismatch is only a note, except on CI). Pending scenes must
 /// either write or stop at a feature the writer lacks, so their other fields stay valid.
 #[test]
 fn written_scenes_match_their_structure() {
@@ -55,7 +65,9 @@ fn written_scenes_match_their_structure() {
         let problems = scene.structure.check(&summary);
         // A scene whose overprint or other states sit only on text can't match without fonts.
         let text_skipped = output.notes.iter().any(|note| note.contains(reference::FONT_DIR_ENV));
-        if !problems.is_empty() && text_skipped {
+        if text_skipped && text_must_be_written() {
+            failures.push(missing_font(scene));
+        } else if !problems.is_empty() && text_skipped {
             println!("note: {}: structure not checked with text skipped: {}", scene.id, problems.join("; "));
         } else if !problems.is_empty() {
             let json = serde_json::to_string_pretty(&summary).unwrap();
@@ -84,7 +96,9 @@ fn print_preset_adds_one_gts_pdfx_intent() {
         want.output_intents = Some(vec!["GTS_PDFX".to_owned()]);
         let problems = want.check(&summary);
         let text_skipped = output.notes.iter().any(|note| note.contains(reference::FONT_DIR_ENV));
-        if !problems.is_empty() && text_skipped {
+        if text_skipped && text_must_be_written() {
+            failures.push(missing_font(scene));
+        } else if !problems.is_empty() && text_skipped {
             assert!(summary.output_intents.iter().filter(|i| *i == "GTS_PDFX").count() == 1, "{}: {summary:?}", scene.id);
         } else if !problems.is_empty() {
             failures.push(format!("{}: {}", scene.id, problems.join("; ")));

crates/prepress/src/cms.rs

@@ -19,6 +19,7 @@ pub struct CmsConversion {
 
 /// Why a profile can't be a printing condition.
 #[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
+#[non_exhaustive]
 pub enum ProfileError {
     #[error("the profile could not be read: {0}")]
     Unreadable(String),

crates/prepress/src/composite.rs

@@ -7,6 +7,7 @@
 use craft_pdf::interpret::color::{CmykConversion, cmyk_to_srgb8};
 
 use crate::cms::lab_to_srgb8;
+use crate::coverage::filled;
 use crate::{Alternate, InkId, Plates, Process};
 
 /// Pixels converted per call into the CMYK profile, to bound the scratch buffers.
@@ -60,10 +61,11 @@ impl Plates {
     /// colorimetric with black point compensation, or through the bundled CMYK profile when
     /// `cms` can't. Each spot plate is then multiplied over them by the spot's appearance at
     /// its tint, from its alternate. Spots whose Lab alternate craft-cms can't convert are
-    /// left out.
+    /// left out. Empty when `width × height × 3` overflows or can't be allocated.
     pub fn composite(&self, enabled: &[bool], cms: &dyn CmykConversion) -> Vec<u8> {
-        let n = (self.width as usize).saturating_mul(self.height as usize);
-        let mut rgb = vec![255_u8; n.saturating_mul(3)];
+        let Some(mut rgb) = self.pixels().and_then(|n| n.checked_mul(3)).and_then(|len| filled(len, 255_u8)) else {
+            return Vec::new();
+        };
         let process: Vec<Option<&[u8]>> = Process::ALL
             .iter()
             .map(|p| {
@@ -140,4 +142,14 @@ mod tests {
         let no_gold = p.composite(&[true, true, true, true, false], &cms);
         assert_eq!(&no_gold[6..9], black);
     }
+
+    #[test]
+    fn plates_too_large_to_hold_give_empty_results() {
+        let p = crate::Plates { width: u32::MAX, height: u32::MAX, ..synthetic(0, 0, [50.0, 0.0, 0.0], Default::default()) };
+        assert!(p.composite(&[], &CmsConversion::fogra39()).is_empty());
+        assert!(p.tac(&[]).is_empty());
+        assert_eq!(p.stats(&[]).max_total_at, None);
+        assert!(p.above(10.0, &[]).regions().is_empty());
+        assert_eq!(p.tint(crate::InkId::process(crate::Process::Black), u32::MAX - 1, u32::MAX - 1), None);
+    }
 }

crates/prepress/src/coverage.rs

@@ -66,6 +66,14 @@ pub struct Region {
     pub pixels: u32,
 }
 
+/// `len` copies of `value`; `None` when the memory can't be had.
+pub(crate) fn filled<T: Clone>(len: usize, value: T) -> Option<Vec<T>> {
+    let mut v = Vec::new();
+    v.try_reserve_exact(len).ok()?;
+    v.resize(len, value);
+    Some(v)
+}
+
 fn on(enabled: &[bool], index: usize) -> bool {
     enabled.get(index).copied().unwrap_or(true)
 }
@@ -75,12 +83,16 @@ fn percent(tint: u64) -> f32 {
 }
 
 impl Plates {
-    fn pixels(&self) -> usize {
-        (self.width as usize).saturating_mul(self.height as usize)
+    /// `width × height`; `None` when it overflows `usize`, as it can for hand-made plates.
+    pub(crate) fn pixels(&self) -> Option<usize> {
+        usize::try_from(self.width).ok()?.checked_mul(usize::try_from(self.height).ok()?)
     }
 
-    fn index_of(&self, x: u32, y: u32) -> Option<usize> {
-        (x < self.width && y < self.height).then(|| y as usize * self.width as usize + x as usize)
+    pub(crate) fn index_of(&self, x: u32, y: u32) -> Option<usize> {
+        if x >= self.width || y >= self.height {
+            return None;
+        }
+        usize::try_from(y).ok()?.checked_mul(usize::try_from(self.width).ok()?)?.checked_add(usize::try_from(x).ok()?)
     }
 
     fn enabled_planes<'a>(&'a self, enabled: &'a [bool]) -> impl Iterator<Item = &'a Vec<u8>> + 'a {
@@ -112,9 +124,9 @@ impl Plates {
     }
 
     /// The TAC of the enabled inks at every pixel, row-major, as a sum of tint bytes (255 per
-    /// ink at 100%).
+    /// ink at 100%). Empty when `width × height` overflows or can't be allocated.
     fn totals(&self, enabled: &[bool]) -> Vec<u32> {
-        let mut totals = vec![0_u32; self.pixels()];
+        let Some(mut totals) = self.pixels().and_then(|n| filled(n, 0_u32)) else { return Vec::new() };
         for plane in self.enabled_planes(enabled) {
             for (sum, tint) in totals.iter_mut().zip(plane) {
                 *sum = sum.saturating_add(u32::from(*tint));
@@ -130,7 +142,7 @@ impl Plates {
 
     /// Each ink's figures over the page, and the TAC of the enabled inks.
     pub fn stats(&self, enabled: &[bool]) -> Stats {
-        let n = self.pixels();
+        let n = self.pixels().unwrap_or(0);
         let mean = |sum: u64| if n == 0 { 0.0 } else { percent(sum) / n as f32 };
         let inks = self
             .planes

crates/prepress/src/ink.rs

@@ -147,22 +147,31 @@ impl InkSet {
             Ink::Spot(spot) => spot,
         };
         let name = normalize_name(&spot.name).into_owned();
-        if name == "All" || name == "None" {
+        self.add_decoded(Spot { name, ..spot })
+    }
+
+    /// [`add`](Self::add) for a spot whose name is already decoded, as a PDF reader hands it
+    /// over: `#` in the name is a literal character.
+    pub(crate) fn add_decoded(&mut self, spot: Spot) -> Option<InkId> {
+        if spot.name == "All" || spot.name == "None" {
             return None;
         }
-        if let Some(id) = self.lookup(&name) {
+        if let Some(id) = self.lookup_decoded(&spot.name) {
             return Some(id);
         }
         let id = InkId(u16::try_from(self.inks.len()).ok()?);
-        self.inks.push(Ink::Spot(Spot { name, ..spot }));
+        self.inks.push(Ink::Spot(spot));
         Some(id)
     }
 
     /// The ink named `name`, compared after [`normalize_name`]: `"/Gold#20Foil"` finds
     /// `Gold Foil`.
     pub fn lookup(&self, name: &str) -> Option<InkId> {
-        let name = normalize_name(name);
-        if let Some(p) = Process::from_name(&name) {
+        self.lookup_decoded(&normalize_name(name))
+    }
+
+    fn lookup_decoded(&self, name: &str) -> Option<InkId> {
+        if let Some(p) = Process::from_name(name) {
             return Some(InkId::process(p));
         }
         let at = self.inks.iter().skip(4).position(|ink| ink.name() == name)?;
@@ -350,6 +359,7 @@ impl InkSet {
 
 /// A color as ink tints.
 #[derive(Clone, Debug, PartialEq)]
+#[non_exhaustive]
 pub enum InkColor {
     /// Tints `0.0..=1.0` of the listed inks; other inks are unspecified (they keep what is
     /// under the paint when it overprints, and are knocked out otherwise). One entry is a
@@ -388,12 +398,15 @@ impl InkColor {
 ///
 /// Spot alternates are [`InkSet::spot_cmyk`]: a CMYK alternate as given, a Lab one through
 /// `cms` into the printing condition. A tint listed twice combines as overlapping halftones,
-/// as in `resolve`. `None` when a tint's ink is not in `inks`, a spot's Lab alternate can't be
-/// converted, a spot name is empty, or a `DeviceN` space would have more than 32 colorants.
+/// as in `resolve`. `None` when `Tints` lists no ink (there is nothing to paint; use
+/// [`InkColor::None`] to mark no plate), a tint's ink is not in `inks`, a spot's Lab alternate
+/// can't be converted, a spot name is empty, or a `DeviceN` space would have more than 32
+/// colorants.
 pub fn to_pdf_color(color: &InkColor, inks: &InkSet, cms: &dyn CmykConversion) -> Option<PdfColor> {
     let tints = match color {
         InkColor::All(tint) => return Some(separation(SeparationColorant::AllColorants, *tint, cmyk::Color::new(255, 255, 255, 255))),
         InkColor::None => return Some(separation(SeparationColorant::NoColorant, 0.0, cmyk::Color::new(0, 0, 0, 0))),
+        InkColor::Tints(tints) if tints.is_empty() => return None,
         InkColor::Tints(tints) => tints,
     };
     // Sorted by id: process inks first, in plate order, then spots in set order.

crates/prepress/src/plates.rs

@@ -24,7 +24,7 @@ use craft_pdf::syntax::object::{Array, Dict, Name, Stream, String as PdfText};
 use craft_pdf::{OpenError, open_slice};
 
 use crate::cms::{CmsConversion, ProfileError, srgb8_to_lab};
-use crate::ink::{Alternate, Ink, InkId, InkSet, Spot};
+use crate::ink::{Alternate, InkId, InkSet, Spot};
 
 /// Spot inks separated per page, after the four process inks. More than any press runs: a
 /// `DeviceN` space has at most 32 colorants, and every plate costs a render and a byte per
@@ -53,6 +53,7 @@ const MAX_CONDITION: usize = 160;
 /// The printing condition process color converts to when the page has no usable output
 /// intent.
 #[derive(Clone, Debug, Default)]
+#[non_exhaustive]
 pub enum Fallback {
     /// craft-cms's Coated FOGRA39 profile.
     #[default]
@@ -82,6 +83,7 @@ impl Default for Options {
 
 /// How process color that isn't CMYK already became CMYK.
 #[derive(Clone, Debug, PartialEq)]
+#[non_exhaustive]
 pub enum Conversion {
     /// Through the CMYK profile of the page's or document's output intent (ISO 32000-2
     /// §14.11.5).
@@ -123,14 +125,12 @@ impl Plates {
 
     /// Ink `id`'s tint at pixel (`x`, `y`), 0–255.
     pub fn tint(&self, id: InkId, x: u32, y: u32) -> Option<u8> {
-        if x >= self.width || y >= self.height {
-            return None;
-        }
-        self.plane(id)?.get(y as usize * self.width as usize + x as usize).copied()
+        self.plane(id)?.get(self.index_of(x, y)?).copied()
     }
 }
 
 #[derive(Debug, thiserror::Error)]
+#[non_exhaustive]
 pub enum Error {
     #[error(transparent)]
     Open(#[from] OpenError),
@@ -178,10 +178,10 @@ pub fn plates(pdf: &[u8], page: usize, dpi: f32, opts: &Options) -> Result<Plate
             continue;
         }
         let alternate = Alternate::Lab(srgb8_to_lab(swatch).unwrap_or([0.0; 3]));
-        let spot = Ink::Spot(Spot { name: ink.name().to_string(), alternate, solidity: None });
-        // A name that normalizes to one already listed (`A#2341` and `AA`) has no plate of its
-        // own.
-        match inks.add(spot) {
+        let spot = Spot { name: ink.name().to_string(), alternate, solidity: None };
+        // hayro hands over names with their `#xx` escapes decoded, so `#` here is a literal
+        // character. Two names that read the same after lossy UTF-8 decoding share one plate.
+        match inks.add_decoded(spot) {
             Some(id) if id.index() == separated.len() => separated.push(ink),
             _ => unseparated.push(ink.name().to_string()),
         }

crates/prepress/tests/plates.rs

@@ -144,6 +144,30 @@ fn spot_inks_past_the_cap_are_listed_not_separated_and_bad_input_is_refused() {
     assert!(matches!(plates(b"not a pdf", 0, 72.0, &Options::default()), Err(Error::Open(_))));
 }
 
+#[test]
+fn escaped_spot_names_are_decoded_once_and_each_gets_its_own_plate() {
+    // `/A#2341` is the spot `A#41`, not `AA`; `/Spot#20#2312` is `Spot #12`.
+    let names = ["/A#2341", "/AA", "/Spot#20#2312"];
+    let spaces: String = names
+        .iter()
+        .enumerate()
+        .map(|(i, name)| format!("/S{i} [/Separation {name} /DeviceGray << /FunctionType 2 /Domain [0 1] /C0 [1] /C1 [0] /N 1 >>] "))
+        .collect();
+    let content: String = (0..names.len()).map(|i| format!("/S{i} cs 1 scn {} 0 10 10 re f ", i * 10)).collect();
+    let pdf = page_pdf(&content, &format!("/ColorSpace << {spaces} >>"), "", &[]);
+    let p = plates(&pdf, 0, 72.0, &Options::default()).unwrap();
+    let spots: Vec<&str> = p.inks.spots().map(|(_, s)| s.name.as_str()).collect();
+    assert_eq!(spots, ["A#41", "AA", "Spot #12"]);
+    assert!(p.unseparated.is_empty());
+    assert_eq!(p.planes.len(), 7);
+    for (i, (id, _)) in p.inks.spots().enumerate() {
+        for x in [5u32, 15, 25] {
+            let expected = if (x / 10) as usize == i { 255 } else { 0 };
+            assert_eq!(p.tint(id, x, 35), Some(expected), "spot {i} at x = {x}");
+        }
+    }
+}
+
 #[test]
 fn lab_colors_convert_through_the_profile_and_lab_white_is_paper() {
     let resources = "/ColorSpace << /D50 [/Lab << /WhitePoint [0.9642 1 0.8249] >>] /D65 [/Lab << /WhitePoint [0.9505 1 1.089] >>] >>";
@@ -243,6 +267,7 @@ fn ink_colors_write_as_cmyk_separation_or_device_n_and_separate_onto_their_plate
 
     let unknown = InkSet::new();
     assert_eq!(to_pdf_color(&colors[1], &unknown, &cms), None, "an ink not in the set");
+    assert_eq!(to_pdf_color(&InkColor::Tints(vec![]), &inks, &cms), None, "no ink: nothing to paint");
     let mut many = InkSet::new();
     let tints: Vec<(InkId, f32)> = (0..33)
         .map(|i| (many.add(Ink::Spot(Spot { name: format!("S{i}"), alternate: Alternate::Cmyk([0.0; 4]), solidity: None })).unwrap(), 1.0))