diff --git a/crates/color/src/cms/mod.rs b/crates/color/src/cms/mod.rs --- a/crates/color/src/cms/mod.rs +++ b/crates/color/src/cms/mod.rs @@ -160,11 +160,41 @@ impl ColorSettings { pub struct OutputIntent { pub icc: Cow<'static, [u8]>, /// `OutputConditionIdentifier`: a registered characterisation, or `Custom`. - pub identifier: &'static str, + pub identifier: Cow<'static, str>, /// `OutputCondition` / `Info`: the profile's name. pub condition: String, } +/// The registered characterisation identifier a profile names in its own description, spelled as +/// the ICC registry spells it. +/// +/// A print provider's preflight matches on the identifier, so `Custom` is only right when the +/// profile names no characterisation: `PSOuncoated_v3_FOGRA52` is FOGRA52 and +/// `eciCMYK v2 (FOGRA59)` is FOGRA59, and saying `Custom` for those loses the one field the +/// provider reads (#1475). Nothing is guessed from a profile's other properties — the identifier +/// has to be in the description, which is where these profiles put it. +pub fn registered_identifier(description: &str) -> Option { + // `to_ascii_uppercase` keeps every byte's position, so an offset into `upper` is an offset + // into `description` as well. The digits are read from `upper` at that same offset: reading + // them from a `Vec` indexed by a byte offset went wrong as soon as anything before the + // name was multi-byte -- "Coated (R) FOGRA39" gave FOGRA9 (review of #1475). + let upper: String = description.to_ascii_uppercase(); + let bytes = upper.as_bytes(); + // `FOGRA` or `CGATS21_CRPC` followed by digits, and not inside a longer word. + for prefix in ["CGATS21_CRPC", "FOGRA"] { + let mut from = 0; + while let Some(at) = upper.get(from..).and_then(|s| s.find(prefix)).map(|i| i + from) { + from = at + prefix.len(); + let before_is_word = at > 0 && bytes.get(at - 1).is_some_and(|b| b.is_ascii_alphanumeric()); + let digits: String = upper.get(from..).unwrap_or("").chars().take_while(char::is_ascii_digit).collect(); + if !before_is_word && !digits.is_empty() { + return Some(format!("{prefix}{digits}")); + } + } + } + None +} + /// Target colour model for [`Cms::convert`]. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Model { @@ -332,12 +362,18 @@ impl Cms { /// described by the Generic CMYK one. pub fn output_intent(&self) -> Result { Ok(match &self.cmyk { - CmykSpace::Icc(p) if p.name == COATED_FOGRA39 => { - OutputIntent { icc: Cow::Borrowed(craft_cms::builtin::COATED_FOGRA39_ICC), identifier: "FOGRA39", condition: COATED_FOGRA39.into() } - } - CmykSpace::Icc(p) => OutputIntent { icc: Cow::Owned(p.profile().to_bytes().to_vec()), identifier: "Custom", condition: p.name.clone() }, + CmykSpace::Icc(p) if p.name == COATED_FOGRA39 => OutputIntent { + icc: Cow::Borrowed(craft_cms::builtin::COATED_FOGRA39_ICC), + identifier: "FOGRA39".into(), + condition: COATED_FOGRA39.into(), + }, + CmykSpace::Icc(p) => OutputIntent { + icc: Cow::Owned(p.profile().to_bytes().to_vec()), + identifier: registered_identifier(&p.name).map_or(Cow::Borrowed("Custom"), Cow::Owned), + condition: p.name.clone(), + }, CmykSpace::Generic(_) | CmykSpace::Device => { - OutputIntent { icc: Cow::Borrowed(icc_out::generic_cmyk_icc()?), identifier: "Custom", condition: GENERIC_CMYK.into() } + OutputIntent { icc: Cow::Borrowed(icc_out::generic_cmyk_icc()?), identifier: "Custom".into(), condition: GENERIC_CMYK.into() } } }) } diff --git a/crates/color/src/cms/tests.rs b/crates/color/src/cms/tests.rs --- a/crates/color/src/cms/tests.rs +++ b/crates/color/src/cms/tests.rs @@ -81,13 +81,56 @@ fn saved_cmyk_names_still_load() { #[test] fn output_intent_follows_the_working_cmyk_space() { let fogra = cms().output_intent().unwrap(); - assert_eq!((fogra.identifier, fogra.condition.as_str()), ("FOGRA39", COATED_FOGRA39)); + assert_eq!((&*fogra.identifier, fogra.condition.as_str()), ("FOGRA39", COATED_FOGRA39)); assert_eq!(&*fogra.icc, craft_cms::builtin::COATED_FOGRA39_ICC); for name in [GENERIC_CMYK, DEVICE_CMYK] { let g = Cms::new(&ColorSettings { cmyk: name.into(), ..Default::default() }).unwrap().output_intent().unwrap(); - assert_eq!((g.identifier, g.condition.as_str()), ("Custom", GENERIC_CMYK), "{name}"); + assert_eq!((&*g.identifier, g.condition.as_str()), ("Custom", GENERIC_CMYK), "{name}"); assert_eq!(&*g.icc, icc_out::generic_cmyk_icc().unwrap()); } + // A loaded profile whose description names its characterisation: the registered identifier, + // not `Custom`, which is the one field a print provider's preflight matches on (#1475). + let bytes = craft_cms::Builtin::CoatedCmyk.profile().to_bytes().to_vec(); + for (name, want) in [ + ("PSOuncoated_v3_FOGRA52", "FOGRA52"), + ("eciCMYK v2 (FOGRA59)", "FOGRA59"), + ("Coated FOGRA51 (ISO 12647-2:2013)", "FOGRA51"), + ("CGATS21_CRPC6", "CGATS21_CRPC6"), + ("Our House Press", "Custom"), + ] { + super::register_icc(&bytes, Some(name.to_string())).unwrap_or_else(|e| panic!("{name}: {e}")); + let c = Cms::new(&ColorSettings { cmyk: name.into(), ..Default::default() }).unwrap_or_else(|e| panic!("{name}: {e}")); + let intent = c.output_intent().unwrap(); + assert_eq!((&*intent.identifier, intent.condition.as_str()), (want, name), "{name}"); + } +} + +/// The identifier is read off the profile's own description, never guessed from anything else. +#[test] +fn a_registered_characterisation_is_the_one_the_profile_names() { + use super::registered_identifier; + for (description, want) in [ + ("PSOuncoated_v3_FOGRA52", Some("FOGRA52")), + ("eciCMYK v2 (FOGRA59)", Some("FOGRA59")), + ("Coated FOGRA39 (ISO 12647-2:2004)", Some("FOGRA39")), + ("fogra51 lowercase", Some("FOGRA51")), + ("CGATS21_CRPC6", Some("CGATS21_CRPC6")), + ("GRACoL2013_CRPC6", None), + ("Our House Press", None), + ("FOGRA", None), + ("XFOGRA39", None), + ("", None), + // A multi-byte character before the name used to shift the digits by one, so this gave + // FOGRA9 and a print provider's preflight read the wrong characterisation (review of + // #1475). The offset and the digits now come from the same string. + ("Coated \u{AE} FOGRA39 (ISO 12647-2:2004)", Some("FOGRA39")), + ("\u{201C}eciCMYK v2\u{201D} (FOGRA59)", Some("FOGRA59")), + ("\u{30B3}\u{30FC}\u{30C8} FOGRA51", Some("FOGRA51")), + ("Papier m\u{E4}t\u{E9} CGATS21_CRPC6", Some("CGATS21_CRPC6")), + ("", None), + ] { + assert_eq!(registered_identifier(description).as_deref(), want, "{description:?}"); + } } #[test] diff --git a/crates/pdf/src/pdfx.rs b/crates/pdf/src/pdfx.rs --- a/crates/pdf/src/pdfx.rs +++ b/crates/pdf/src/pdfx.rs @@ -60,7 +60,7 @@ pub fn make_pdfx4(pdf: &[u8], title: &str, date: Option) -> Option> oi_id, format!( "<>", - lit(intent.identifier), + lit(&intent.identifier), lit(cond), lit(cond) )