diff --git a/crates/cms/README.md b/crates/cms/README.md index 55bbc6b8..71f3146d 100644 --- a/crates/cms/README.md +++ b/crates/cms/README.md @@ -30,6 +30,8 @@ Buffers are checked once per call, before any pixel is written. * `Transform::proof(src, proof, display, intent, bpc, simulate_paper)`: soft-proof chain. * `GamutCheck::new(src, proof, ΔE)`: out-of-gamut test (gamut warning). + `check.mask_rgba(&rgba, &mut mask)?` fills a whole mask (255 where alpha > 0 and out of gamut) on + the thread pool and returns the count; it needs an RGB source profile. * `Lut3d::from_transform(&t, 33)?.to_rgba16f_bytes()`: display transform as a 3D texture; `Lut3d::from_fn(size, f)` samples any RGB function. * `Transform::gpu_matrix_shaper()`: accurate RGB input curves + matrix + output curves for diff --git a/crates/cms/src/gamut.rs b/crates/cms/src/gamut.rs index 247b183f..11dd1773 100644 --- a/crates/cms/src/gamut.rs +++ b/crates/cms/src/gamut.rs @@ -2,13 +2,18 @@ use crate::clut::Clut; use crate::pipeline::{Pipeline, Stage}; -use crate::profile::Profile; +use crate::profile::{ColorSpace, Profile}; use crate::transform::link_stages; -use crate::{CmsError, Intent, check_len}; +use crate::{CmsError, Intent, check_len, invalid_arg}; /// Default ΔE76 threshold above which a colour counts as out of gamut. pub const DEFAULT_THRESHOLD: f32 = 4.0; +#[cfg(not(target_arch = "wasm32"))] +const PAR_MIN_PIXELS: usize = 16 * 1024; +#[cfg(not(target_arch = "wasm32"))] +const PAR_CHUNK_PIXELS: usize = 16 * 1024; + /// Precomputed out-of-gamut test for colours of `src` against `proof`. /// /// A colour is out of gamut when sending it to the proof device (relative colorimetric, @@ -17,6 +22,7 @@ pub const DEFAULT_THRESHOLD: f32 = 4.0; #[derive(Debug)] pub struct GamutCheck { inputs: usize, + source_space: ColorSpace, grid: Option, exact: (Pipeline, Pipeline), pub threshold: f32, @@ -44,7 +50,7 @@ impl GamutCheck { 4 => Some(Clut::sample(vec![17; 4], 1, de)?), _ => None, }; - Ok(GamutCheck { inputs: n, grid, exact: (direct, via), threshold }) + Ok(GamutCheck { inputs: n, source_space: src.color_space, grid, exact: (direct, via), threshold }) } pub fn inputs(&self) -> usize { @@ -76,4 +82,84 @@ impl GamutCheck { pub fn out_of_gamut(&self, px: &[f32]) -> Result { Ok(self.delta_e(px)? > self.threshold) } + + /// Out-of-gamut mask for RGBA pixels: `255` where the pixel has alpha > 0 and its RGB + /// channels are out of gamut, `0` elsewhere. Returns the number of flagged pixels. Only + /// `min(px.len(), mask.len())` pixels are processed. Runs on the rayon pool (not on wasm); + /// the result is identical to calling [`Self::out_of_gamut`] per pixel. + /// + /// # Errors + /// [`CmsError::InvalidArgument`] when the check was not built from an RGB source profile: an + /// RGBA pixel carries only RGB colour channels, so it cannot be tested against a CMYK, gray + /// or Lab source. Nothing is written to `mask` in that case. + pub fn mask_rgba(&self, px: &[[f32; 4]], mask: &mut [u8]) -> Result { + if self.source_space != ColorSpace::Rgb || self.inputs != 3 { + return Err(invalid_arg(format!("gamut mask needs an RGB source profile, got {:?} with {} channels", self.source_space, self.inputs))); + } + let n = px.len().min(mask.len()); + let (Some(px), Some(mask)) = (px.get(..n), mask.get_mut(..n)) else { return Ok(0) }; + let run = |p: &[[f32; 4]], m: &mut [u8]| { + let mut count = 0; + for (p, m) in p.iter().zip(m.iter_mut()) { + // The source has three channels (checked above), so the check cannot fail. + let hit = p[3] > 0.0 && self.out_of_gamut(&[p[0], p[1], p[2]]).unwrap_or(false); + *m = if hit { 255 } else { 0 }; + count += usize::from(hit); + } + count + }; + #[cfg(not(target_arch = "wasm32"))] + if n >= PAR_MIN_PIXELS { + use rayon::prelude::*; + return Ok(px.par_chunks(PAR_CHUNK_PIXELS).zip(mask.par_chunks_mut(PAR_CHUNK_PIXELS)).map(|(p, m)| run(p, m)).sum()); + } + Ok(run(px, mask)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Builtin; + + fn image(n: usize) -> Vec<[f32; 4]> { + let mut s = 0x9E37_79B9u32; + let mut rnd = move || { + s ^= s << 13; + s ^= s >> 17; + s ^= s << 5; + (s >> 8) as f32 / (1u32 << 24) as f32 + }; + // Includes out-of-range values and zero alpha. + (0..n).map(|i| [rnd() * 1.2 - 0.1, rnd(), rnd(), if i % 7 == 0 { 0.0 } else { rnd() }]).collect() + } + + #[test] + fn mask_rgba_matches_per_pixel_check() { + let src = Builtin::Srgb.profile(); + // Sizes straddle the parallel threshold and the chunk size, odd ones included. + for (proof, threshold) in [(Builtin::CoatedCmyk, 4.0), (Builtin::CoatedCmyk, 1.5), (Builtin::GrayGamma22, 4.0), (Builtin::DisplayP3, 0.5)] { + let check = GamutCheck::new(src, proof.profile(), threshold).unwrap(); + for n in [0, 1, 1023, 16 * 1024 - 1, 16 * 1024, 16 * 1024 + 1, 50_001] { + let px = image(n); + let want: Vec = px.iter().map(|p| if p[3] > 0.0 && check.out_of_gamut(&p[..3]).unwrap() { 255 } else { 0 }).collect(); + let mut got = vec![7u8; n]; + let count = check.mask_rgba(&px, &mut got).unwrap(); + assert_eq!(got, want, "{proof:?} threshold {threshold} n {n}"); + assert_eq!(count, want.iter().filter(|&&m| m == 255).count()); + } + } + } + + #[test] + fn mask_rgba_rejects_non_rgb_source() { + // A CMYK (4-channel), gray (1-channel) and Lab (3-channel, not RGB) source cannot be tested from RGBA pixels. + for src in [Builtin::CoatedCmyk, Builtin::GrayGamma22, Builtin::LabD50] { + let check = GamutCheck::new(src.profile(), Builtin::Srgb.profile(), 4.0).unwrap(); + let px = image(64); + let mut mask = vec![9u8; 64]; + assert!(matches!(check.mask_rgba(&px, &mut mask), Err(CmsError::InvalidArgument(_))), "{src:?}"); + assert!(mask.iter().all(|&m| m == 9), "mask must stay untouched"); + } + } } diff --git a/crates/cms/src/lutfile.rs b/crates/cms/src/lutfile.rs index c2261325..404b5a31 100644 --- a/crates/cms/src/lutfile.rs +++ b/crates/cms/src/lutfile.rs @@ -135,7 +135,8 @@ pub fn parse_as(format: Format, bytes: &[u8]) -> Result { } } -/// UTF-16 with a byte-order mark (Photoshop loads such `.look` files, #9118), else UTF-8. +/// UTF-16 with a byte-order mark (Photoshop loads such `.look` files, #9118), else UTF-8 +/// (a UTF-8 BOM is stripped). fn decode_text(bytes: &[u8]) -> std::borrow::Cow<'_, str> { let utf16 = |rest: &[u8], from: fn([u8; 2]) -> u16| { let units: Vec = rest.as_chunks::<2>().0.iter().map(|&c| from(c)).collect(); @@ -144,6 +145,7 @@ fn decode_text(bytes: &[u8]) -> std::borrow::Cow<'_, str> { match bytes { [0xFF, 0xFE, rest @ ..] => utf16(rest, u16::from_le_bytes), [0xFE, 0xFF, rest @ ..] => utf16(rest, u16::from_be_bytes), + [0xEF, 0xBB, 0xBF, rest @ ..] => String::from_utf8_lossy(rest), _ => String::from_utf8_lossy(bytes), } } @@ -155,14 +157,19 @@ pub fn parse_cube(text: &str) -> Result { let (mut dmin, mut dmax) = ([0.0f32; 3], [1.0f32; 3]); let mut rows: Vec = Vec::new(); for line in text.lines() { - // A '#' inside a quoted TITLE is part of the title, not a comment. - let raw = line.trim_start(); - let skip = if raw.starts_with("TITLE") { raw.find('"').and_then(|q| raw[q + 1..].find('"').map(|e| q + e + 2)) } else { None }.unwrap_or(0); - let line = match raw[skip..].find('#') { - Some(i) => &raw[..skip + i], - None => raw, + // Only a quote at the beginning of a TITLE value protects '#'. Quotes inside an + // unquoted title's comment must not turn that comment into part of the title. + if let Some(quoted) = line + .trim_start() + .strip_prefix("TITLE") + .filter(|rest| rest.starts_with(char::is_whitespace)) + .and_then(|rest| rest.trim_start().strip_prefix('"')) + { + let Some((value, _)) = quoted.split_once('"') else { return err("unterminated quoted TITLE") }; + title = value.to_string(); + continue; } - .trim(); + let line = line.split('#').next().unwrap_or("").trim(); if line.is_empty() { continue; } @@ -187,7 +194,9 @@ pub fn parse_cube(text: &str) -> Result { } k if k.starts_with(|c: char| c.is_ascii_digit() || c == '-' || c == '+' || c == '.') => { let mut v = vec![k.parse::().map_err(|_| LutError(format!("bad number in `{line}`")))?]; - v.extend(it.filter_map(|s| s.parse::().ok())); + for value in it { + v.push(value.parse::().map_err(|_| LutError(format!("bad number in `{line}`")))?); + } if v.len() != 3 { return err(format!("bad data line `{line}`")); } @@ -209,6 +218,15 @@ pub fn parse_cube(text: &str) -> Result { if rows.len() != expected_rows { return err("no LUT_3D_SIZE in .cube file"); } + // #8913: the resample below bakes the curve onto a fixed 33-point grid, so a steep + // curve over a narrow domain cannot survive the resample — the baked table silently + // shifts colours (measured 42 RGB8 levels for a two-node ramp over 0.24..0.26, where + // Photoshop renders the ramp exactly). Rather than proxy the risk by domain width (a + // 0.1..0.17 ramp spans two grid cells yet still loses ~28 levels), measure the + // resample's actual error against the exact curve and refuse only tables whose baked + // output shifts by more than a couple of RGB8 levels. Default-domain tables keep the + // existing bake untouched; gentle curves over any domain bake within tolerance and + // stay accepted. // The 1D table is resampled onto a 0..1 cube anyway, so its input domain // (`DOMAIN_MIN`/`MAX`, `LUT_1D_INPUT_RANGE`) is baked into the resample: an input `v` // reads the table at `(v - min) / (max - min)`, saturating outside. @@ -221,6 +239,36 @@ pub fn parse_cube(text: &str) -> Result { }; let mut l = LutFile::from_fn(&title, 33, |c| [curve(0, c[0]), curve(1, c[1]), curve(2, c[2])])?; l.title = title; + let non_default = dmin.iter().any(|v| *v != 0.0) || dmax.iter().any(|v| *v != 1.0); + if non_default { + // Worst baked-vs-exact error per channel, in RGB8 levels: the baked cube is read + // by linear interpolation between grid points (red fastest, so channel `ch` sits + // at stride 33^ch), while `curve` is what the file actually says. + const MAX_BAKE_ERROR: f32 = 2.0 / 255.0; + for ch in 0..3 { + let stride = [1usize, 33, 33 * 33][ch]; + let mut worst = 0.0f32; + for k in 0..=256 { + let v = k as f32 / 256.0; + let x = v * 32.0; + let i = (x as usize).min(31); + let f = x - i as f32; + let baked = l.data[i * stride * 3 + ch] * (1.0 - f) + l.data[(i + 1).min(32) * stride * 3 + ch] * f; + worst = worst.max((baked - curve(ch, v)).abs()); + } + if worst > MAX_BAKE_ERROR { + return err(format!( + "1D LUT domain on {} is {min}..{max}: the curve changes faster than the 33-point grid can \ + hold (baking shifts colours by up to {levels:.0} RGB8 levels); export the look as a 3D LUT \ + or widen DOMAIN_MIN/DOMAIN_MAX", + ["red", "green", "blue"][ch], + min = dmin[ch], + max = dmax[ch], + levels = worst * 255.0, + )); + } + } + } return l.check(); } err("no LUT_3D_SIZE in .cube file") @@ -554,6 +602,48 @@ pub fn builtin(id: &str) -> Option { mod tests { use super::*; + #[test] + fn cube_title_hashes_and_comment_quotes_are_distinct() { + let rows = "LUT_1D_SIZE 2\n0 0 0 # low\n1 1 1 # high\n"; + for (line, title) in [ + (r#"TITLE "Look #3" # comment "ignored""#, "Look #3"), + (r#"TITLE Look # "draft""#, "Look"), + (" TITLE\t\"Été #2\"", "Été #2"), + (r#"TITLE "" # empty"#, ""), + ] { + assert_eq!(parse_cube(&format!("{line}\n{rows}")).unwrap().title, title); + } + let lut = LutFile::from_fn("Look #3", 2, |c| c).unwrap(); + assert_eq!(parse_cube(&write_cube(&lut)).unwrap().title, "Look #3"); + assert!(parse_cube(&format!("TITLE \"unterminated # title\n{rows}")).is_err()); + } + + #[test] + fn cube_rejects_invalid_numeric_fields() { + let text = "LUT_3D_SIZE 2\n0.25 nope 0.5 0.75\n0 0 1\n0 1 0\n0 1 1\n1 0 0\n1 0 1\n1 1 0\n1 1 1\n"; + assert!(parse_cube(text).is_err(), "invalid numeric field must not be ignored"); + } + + #[test] + fn utf8_bom_is_stripped() { + let l = LutFile::identity(3).unwrap(); + let mut bytes = vec![0xEF, 0xBB, 0xBF]; + bytes.extend_from_slice(write_cube(&l).as_bytes()); + let p = parse("a.cube", &bytes).unwrap(); + assert_eq!(p.size, l.size); + assert_eq!(p.title, l.title); + let titled = LutFile::from_fn("Look", 3, |c| c).unwrap(); + let mut bytes = vec![0xEF, 0xBB, 0xBF]; + bytes.extend_from_slice(write_cube(&titled).as_bytes()); + assert_eq!(parse("a.cube", &bytes).unwrap().title, "Look"); + let mut bytes = vec![0xEF, 0xBB, 0xBF]; + bytes.extend_from_slice(b"LUT_3D_SIZE 2\n"); + for i in 0..8 { + bytes.extend_from_slice(format!("{} {} {}\n", i & 1, (i >> 1) & 1, i >> 2).as_bytes()); + } + assert_eq!(parse("a.cube", &bytes).unwrap().size, 2); + } + /// An identity `.3dl` of edge `n` (12-bit nodes, blue fastest) under `shaper`. fn identity_3dl(shaper: &[u32], n: usize) -> String { let mut t = shaper.iter().map(u32::to_string).collect::>().join(" ") + "\n"; @@ -811,6 +901,49 @@ DOMAIN_MAX 0.5 1 1 assert!(e.0.contains("below DOMAIN_MAX"), "{e}"); } + #[test] + fn cube_1d_narrow_domain_is_refused() { + // #8913: the issue's fixture — a two-node ramp over 0.24..0.26. Baking it onto the + // 33-point grid rendered grey-65 as 148 where Photoshop renders 190 (42-level error). + // It must now be an actionable parse error rather than a silently wrong table. + let e = parse_cube("LUT_1D_SIZE 2\nDOMAIN_MIN 0.24 0.24 0.24\nDOMAIN_MAX 0.26 0.26 0.26\n0 0 0\n1 1 1\n").unwrap_err(); + assert!(e.0.contains("changes faster than the 33-point grid"), "{e}"); + // The same domain via LUT_1D_INPUT_RANGE is refused identically. + let e = parse_cube("LUT_1D_SIZE 2\nLUT_1D_INPUT_RANGE 0.24 0.26\n0 0 0\n1 1 1\n").unwrap_err(); + assert!(e.0.contains("changes faster"), "{e}"); + // A per-channel narrow domain names the offending channel. + let e = parse_cube("LUT_1D_SIZE 2\nDOMAIN_MIN 0 0 0.24\nDOMAIN_MAX 1 1 0.26\n0 0 0\n1 1 1\n").unwrap_err(); + assert!(e.0.contains("blue"), "{e}"); + } + + #[test] + fn cube_1d_wide_domain_still_bakes() { + // A steep ramp over 0.1..0.17 spans two grid cells but still loses ~28 RGB8 levels + // to the resample, so the measured-error check refuses it even though the old width + // check let it through. + let e = parse_cube("LUT_1D_SIZE 2\nLUT_1D_INPUT_RANGE 0.1 0.17\n0 0 0\n1 1 1\n").unwrap_err(); + assert!(e.0.contains("changes faster"), "steep ramp over 0.1..0.17 refused: {e}"); + // A gentle (here linear) curve over a grid-aligned domain bakes exactly and stays + // accepted — the check measures the bake's error, not the domain's width. Value + // assertions, not just a successful parse: the baked table must reproduce the curve. + // Table row k sits at t = k/32 of the domain 0.25..0.75; the curve y = 0.4 + 0.4t + // is gentle (a half-level per grid step), so the bake must reproduce it exactly. + let mut table = String::from("LUT_1D_SIZE 33\nLUT_1D_INPUT_RANGE 0.25 0.75\n"); + for k in 0..33 { + let y = 0.4 + 0.4 * (k as f32 / 32.0); + table.push_str(&format!("{y} {y} {y}\n")); + } + let l = parse_cube(&table).expect("gentle curve over a non-default domain still bakes"); + let at = |r: usize| l.data[r * 3]; + // Grid entry r is input r/32: 0.5 is the middle of the domain (t=0.5) -> 0.6. + assert!((at(16) - 0.6).abs() < 1.0 / 255.0, "baked value at 0.5 tracks the curve: {}", at(16)); + // Domain top (input 0.75 = grid 24, t=1) -> 0.8. + assert!((at(24) - 0.8).abs() < 1.0 / 255.0, "baked value at the domain top tracks the curve: {}", at(24)); + // Below the domain saturates to the first row. + assert!((at(8) - 0.4).abs() < 1.0 / 255.0, "below the domain saturates: {}", at(8)); + parse_cube("LUT_1D_SIZE 2\n0 0 0\n1 1 1\n").expect("default domain"); + } + #[test] fn cube_1d_domain_is_baked_into_the_resample() { // An identity ramp over the domain 0.25..0.75: 0.25 maps to 0, 0.5 to 0.5, 0.75 to 1. @@ -1042,11 +1175,14 @@ DOMAIN_MAX 0.5 1 1 } #[test] - fn look_hex_floats() { - let id = LutFile::identity(2).unwrap(); - let hex: String = id.data.iter().flat_map(|v| v.to_le_bytes()).map(|b| format!("{b:02X}")).collect(); + fn look_data_is_little_endian_float32() { + // Bytes written out by hand rather than by the encoder the parser mirrors, so a wrong byte + // order fails: 0.25, 0.5, 1.0 (3E800000, 3F000000, 3F800000 stored low byte first), then zeros. + let hex = format!("0000803E 0000003F 0000803F {}", "00000000 ".repeat(21)); let xml = format!("\"2\"\"{hex}\""); - assert_eq!(parse("a.look", xml.as_bytes()).unwrap().data, id.data); + let data = parse("a.look", xml.as_bytes()).unwrap().data; + assert_eq!(data[..3], [0.25, 0.5, 1.0]); + assert!(data[3..].iter().all(|&v| v == 0.0)); } #[test]