diff --git a/crates/engine/src/cmd/fileio/ppi.rs b/crates/engine/src/cmd/fileio/ppi.rs index 54363e3ad..1c04db9c8 100644 --- a/crates/engine/src/cmd/fileio/ppi.rs +++ b/crates/engine/src/cmd/fileio/ppi.rs @@ -220,6 +220,21 @@ mod tests { assert!(close(resolution(&jpg).unwrap(), (254.0, 240.0 * 2.54))); } + /// A CMYK JPEG carries no JFIF (it describes one- and three-component images), so its + /// resolution is written as EXIF instead and this reader still finds it (#3084). + #[test] + fn a_cmyk_jpeg_keeps_its_resolution_without_jfif() { + use vectorcraft_render::encode::jpeg::{self, ColorModel, JpegOptions}; + let o = JpegOptions { color_model: ColorModel::Cmyk, ..Default::default() }; + let jpg = jpeg::encode(&[0; 4 * 4], 2, 2, 90, Some(240.0), &o).unwrap(); + assert!(!jpg.windows(5).any(|w| w == b"JFIF\0"), "no JFIF on a CMYK file"); + assert_eq!(resolution(&jpg), Some((240.0, 240.0))); + // An RGB file still says it in JFIF, as it always did. + let rgb = jpeg::encode(&[0; 3 * 4], 2, 2, 90, Some(240.0), &JpegOptions::default()).unwrap(); + assert!(rgb.windows(5).any(|w| w == b"JFIF\0")); + assert_eq!(resolution(&rgb), Some((240.0, 240.0))); + } + /// A little-endian TIFF/EXIF block declaring `x`/`y` resolution in `unit`. fn exif_block(x: u32, y: u32, unit: u16) -> Vec { let mut b = b"II*\0".to_vec(); diff --git a/crates/render/src/encode/jpeg.rs b/crates/render/src/encode/jpeg.rs index c6f74cb5c..596399c3e 100644 --- a/crates/render/src/encode/jpeg.rs +++ b/crates/render/src/encode/jpeg.rs @@ -155,9 +155,68 @@ pub fn encode(px: &[u8], width: u32, height: u32, quality: u8, ppi: Option, ColorModel::Gray => ColorType::Luma, }; enc.encode(px, w, h, ty).map_err(fail)?; + // JFIF describes one- and three-component images (greyscale and YCbCr), so its APP0 segment + // must not head a four-component CMYK frame: the file then says two different things about + // its colours, and a strict reader rejects it rather than guessing. Adobe's own CMYK JPEGs + // carry the APP14 marker the encoder already writes, and no JFIF (#3084). + if o.color_model == ColorModel::Cmyk { + unlabel_jfif(&mut buf, ppi); + } Ok(buf) } +/// Replace the leading APP0/JFIF segment, which the encoder always writes, with an EXIF segment +/// carrying the same resolution (TIFF `XResolution`, `YResolution` and `ResolutionUnit`, which is +/// where a reader looks when there is no JFIF). Without a resolution the segment is simply +/// dropped. +fn unlabel_jfif(buf: &mut Vec, ppi: Option) { + // SOI, then APP0 with its two length bytes and the tag it is identified by. + let Some(head) = buf.get(..13) else { return }; + if head[..4] != [0xFF, 0xD8, 0xFF, 0xE0] || &head[6..11] != b"JFIF\0" { + return; + } + let len = usize::from(u16::from_be_bytes([head[4], head[5]])); + if 4 + len > buf.len() { + return; + } + let exif = ppi.filter(|p| p.is_finite() && *p >= 1.0).map(|p| exif_resolution(p.round() as u32)).unwrap_or_default(); + buf.splice(2..4 + len, exif); +} + +/// An APP1 EXIF segment holding `dpi` as IFD0's resolution: big-endian TIFF, three entries, each +/// rational pointing past the directory. +fn exif_resolution(dpi: u32) -> Vec { + let mut tiff = Vec::new(); + tiff.extend_from_slice(b"MM\0*"); // big-endian, TIFF magic + tiff.extend_from_slice(&8u32.to_be_bytes()); // IFD0 right after the header + tiff.extend_from_slice(&3u16.to_be_bytes()); // three entries + // Entry: tag, type (5 = rational, 3 = short), count, then the value or its offset. + let entry = |tag: u16, ty: u16, value: [u8; 4]| { + let mut e = Vec::with_capacity(12); + e.extend_from_slice(&tag.to_be_bytes()); + e.extend_from_slice(&ty.to_be_bytes()); + e.extend_from_slice(&1u32.to_be_bytes()); + e.extend_from_slice(&value); + e + }; + // The two rationals sit after the directory: header 8 + count 2 + 3 entries + next-IFD 4. + let after = 8 + 2 + 3 * 12 + 4; + tiff.extend(entry(282, 5, (after as u32).to_be_bytes())); // XResolution + tiff.extend(entry(283, 5, (after as u32 + 8).to_be_bytes())); // YResolution + tiff.extend(entry(296, 3, [0, 2, 0, 0])); // ResolutionUnit: inches + tiff.extend_from_slice(&0u32.to_be_bytes()); // no next IFD + for _ in 0..2 { + tiff.extend_from_slice(&dpi.to_be_bytes()); + tiff.extend_from_slice(&1u32.to_be_bytes()); + } + let mut seg = vec![0xFF, 0xE1]; + let body_len = 6 + tiff.len() + 2; + seg.extend_from_slice(&u16::try_from(body_len).unwrap_or(u16::MAX).to_be_bytes()); + seg.extend_from_slice(b"Exif\0\0"); + seg.extend_from_slice(&tiff); + seg +} + /// `img` flattened on white as RGB, or as grey with each colour's lightness. pub(crate) fn screen_pixels(img: &Rendered, gray: bool) -> Vec { let px = img.to_straight(); diff --git a/crates/render/src/encode/tests_jpeg.rs b/crates/render/src/encode/tests_jpeg.rs index 32366ed5a..2bba70330 100644 --- a/crates/render/src/encode/tests_jpeg.rs +++ b/crates/render/src/encode/tests_jpeg.rs @@ -48,6 +48,44 @@ fn noise(w: u32, h: u32, channels: usize) -> Vec { .collect() } +/// JFIF describes one- and three-component images, so its APP0 segment must not head a CMYK +/// frame: the file would say two different things about its colours and a strict reader rejects +/// it. A CMYK JPEG carries the Adobe APP14 marker instead, as Adobe's own do (#3084). +#[test] +fn a_cmyk_jpeg_is_not_labelled_jfif() { + let px = noise(16, 16, 4); + let o = JpegOptions { color_model: ColorModel::Cmyk, ..Default::default() }; + let file = jpeg::encode(&px, 16, 16, 90, Some(300.0), &o).unwrap(); + let markers: Vec = segments(&file).into_iter().map(|(m, _)| m).collect(); + assert_eq!(frame(&file).1, 4, "four components"); + assert!(!markers.contains(&0xE0), "no APP0/JFIF: {markers:02x?}"); + assert!(markers.contains(&0xEE), "the Adobe APP14 marker names the colours: {markers:02x?}"); + assert!(icc(&file).is_some(), "the CMYK profile is still embedded"); + assert_eq!(&file[..2], &[0xFF, 0xD8], "the file still starts at SOI"); + // The resolution JFIF carried moves to EXIF, where a reader looks when there is no JFIF. + let exif = segments(&file).into_iter().find(|(m, _)| *m == 0xE1).expect("APP1/EXIF").1; + assert!(exif.starts_with(b"Exif\0\0"), "{:02x?}", &exif[..8.min(exif.len())]); + // Big-endian TIFF with XResolution (282), YResolution (283) and ResolutionUnit (296). + assert_eq!(&exif[6..10], b"MM\0*"); + // "Exif\0\0" then the TIFF header (8 bytes) and the entry count (2), so entries start at 16. + let tags: Vec = (0..3).map(|i| u16::from_be_bytes([exif[16 + i * 12], exif[17 + i * 12]])).collect(); + assert_eq!(tags, [282, 283, 296], "{tags:?}"); + // No resolution asked for: nothing stands in for the dropped segment. + let plain = jpeg::encode(&px, 16, 16, 90, None, &o).unwrap(); + let markers: Vec = segments(&plain).into_iter().map(|(m, _)| m).collect(); + assert!(!markers.contains(&0xE0) && !markers.contains(&0xE1), "{markers:02x?}"); + + // Three- and one-component files keep JFIF, which is what it is for, and their resolution. + for (model, channels) in [(ColorModel::Rgb, 3), (ColorModel::Gray, 1)] { + let px = noise(16, 16, channels); + let o = JpegOptions { color_model: model, ..Default::default() }; + let file = jpeg::encode(&px, 16, 16, 90, Some(300.0), &o).unwrap(); + let markers: Vec = segments(&file).into_iter().map(|(m, _)| m).collect(); + assert!(markers.contains(&0xE0), "{model:?} keeps JFIF: {markers:02x?}"); + assert_eq!(frame(&file).1, channels as u8); + } +} + #[test] fn size_falls_with_quality() { let px = noise(64, 48, 3); @@ -85,9 +123,18 @@ fn colour_models_write_their_components_and_profiles() { assert_eq!(frame(&file).1, comps, "{model:?}"); let p = IccProfile::from_bytes(None, &icc(&file).expect("APP2 ICC")).unwrap(); assert_eq!(p.kind, kind, "{model:?}"); - let app0 = segments(&file).into_iter().find(|(m, _)| *m == 0xE0).unwrap().1; - assert_eq!(&app0[..5], b"JFIF\0"); - assert_eq!(app0[7..12], [1, 0, 150, 0, 150], "dots per inch"); + // JFIF describes one- and three-component images, so a CMYK file says its resolution in + // EXIF instead (see `a_cmyk_jpeg_is_not_labelled_jfif`). + if model == ColorModel::Cmyk { + let exif = segments(&file).into_iter().find(|(m, _)| *m == 0xE1).expect("APP1/EXIF").1; + assert!(exif.starts_with(b"Exif\0\0")); + // "Exif\0\0" (6) + TIFF header (8) + count (2) + three entries (36) + next IFD (4). + assert_eq!(&exif[56..64], &[0, 0, 0, 150, 0, 0, 0, 1], "150 dots per inch, as a rational"); + } else { + let app0 = segments(&file).into_iter().find(|(m, _)| *m == 0xE0).unwrap().1; + assert_eq!(&app0[..5], b"JFIF\0"); + assert_eq!(app0[7..12], [1, 0, 150, 0, 150], "dots per inch"); + } let none = jpeg::encode(&noise(16, 8, model.channels()), 16, 8, 90, None, &JpegOptions { embed_icc: false, ..o }).unwrap(); assert!(icc(&none).is_none()); }