From c8136c538ba91a6b977bc83ee15de87fd2bd1623 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:20:51 -0800 Subject: [PATCH] Place: an unusable EXIF resolution falls back to the JFIF density A JPEG whose EXIF resolution is 0 ppi or outside 1 to 100,000 ppi now takes its size from its JFIF density instead of one point per pixel. Tests place an 11 x 17 in image (330 x 510 px at 30 ppi) with a click, check that it comes in at 792 x 1224 pt and keeps that size through a save and reopen, and that an image with no stored resolution comes in at 72 ppi. --- crates/engine/src/cmd/file/place.rs | 84 +++++++++++++++++++++++------ crates/images/src/lib.rs | 4 +- crates/images/src/resolution.rs | 10 ++-- crates/images/src/tests.rs | 13 +++++ crates/render/src/decode.rs | 2 +- 5 files changed, 88 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 29358d834..e0d285702 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -589,10 +589,8 @@ pub(super) mod image_place_tests { } } - #[test] - fn placed_photo_honors_its_embedded_resolution() { - // #1991: a placed graphic's embedded density sets its size (600 dpi: 40 px place - // 4.8 pt wide), instead of a pixel always becoming a point. + /// A `width` x `height` PNG with a pHYs chunk of `ppm` pixels per meter on both axes, or none. + fn png_with_density(width: u32, height: u32, ppm: Option) -> std::io::Result { fn crc32(data: &[u8]) -> u32 { let mut crc = 0xFFFF_FFFFu32; for &b in data { @@ -603,22 +601,33 @@ pub(super) mod image_place_tests { } !crc } - let png = designcraft_render::Rendered { width: 40, height: 20, pixels: [30, 90, 180, 255].repeat(800) }.to_png(); + let pixels = [30, 90, 180, 255].repeat((width * height) as usize); + let png = designcraft_render::Rendered { width, height, pixels }.to_png(); assert_eq!(&png[12..16], b"IHDR"); - let mut phys = Vec::new(); - phys.extend_from_slice(&23622u32.to_be_bytes()); - phys.extend_from_slice(&23622u32.to_be_bytes()); - phys.push(1); let mut bytes = png[..33].to_vec(); - bytes.extend_from_slice(&(phys.len() as u32).to_be_bytes()); - bytes.extend_from_slice(b"pHYs"); - bytes.extend_from_slice(&phys); - let mut c = b"pHYs".to_vec(); - c.extend_from_slice(&phys); - bytes.extend_from_slice(&crc32(&c).to_be_bytes()); + if let Some(ppm) = ppm { + let mut phys = Vec::new(); + phys.extend_from_slice(&ppm.to_be_bytes()); + phys.extend_from_slice(&ppm.to_be_bytes()); + phys.push(1); + bytes.extend_from_slice(&(phys.len() as u32).to_be_bytes()); + bytes.extend_from_slice(b"pHYs"); + bytes.extend_from_slice(&phys); + let mut c = b"pHYs".to_vec(); + c.extend_from_slice(&phys); + bytes.extend_from_slice(&crc32(&c).to_be_bytes()); + } bytes.extend_from_slice(&png[33..]); - let file = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); - std::fs::write(file.path(), bytes).unwrap(); + let file = tempfile::Builder::new().suffix(".png").tempfile()?; + std::fs::write(file.path(), bytes)?; + Ok(file) + } + + #[test] + fn placed_photo_honors_its_embedded_resolution() { + // #1991: a placed graphic's embedded density sets its size (600 dpi: 40 px place + // 4.8 pt wide), instead of a pixel always becoming a point. + let file = png_with_density(40, 20, Some(23622)).unwrap(); let mut session = Session::new(); session.execute("file.new", &json!({})).unwrap(); let placed = session.execute("file.place", &json!({"path": file.path(), "x": 10.0, "y": 20.0})).unwrap(); @@ -629,6 +638,47 @@ pub(super) mod image_place_tests { assert!((b.width() - 4.8).abs() < 0.01 && (b.height() - 2.4).abs() < 0.01, "{b:?}"); } + /// File › Place, then a click: an 11 x 17 in image (330 x 510 px at 30 ppi) comes in at + /// 792 x 1224 pt, keeps that size through a save and reopen, and an image with no stored + /// resolution comes in at 72 ppi (330 x 510 pt). + #[test] + fn a_clicked_image_keeps_its_print_size_through_save_and_reopen() { + // 1181 pixels per meter is 29.9974 ppi. + let (w, h) = (330.0 * 72.0 / (1181.0 * 0.0254), 510.0 * 72.0 / (1181.0 * 0.0254)); + for (ppm, (ew, eh)) in [(Some(1181), (w, h)), (None, (330.0, 510.0))] { + let image = png_with_density(330, 510, ppm).unwrap(); + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + session.execute("file.place", &json!({"path": image.path(), "loadCursor": true})).unwrap(); + session.execute("place.drop", &json!({"x": 0, "y": 0})).unwrap(); + // The bounds and natural size of the document's one placed graphic. + let shown = |session: &Session| { + let doc = &session.doc().unwrap().doc; + let graphics: Vec<_> = doc + .all_items() + .into_iter() + .filter_map(|id| doc.item(id)) + .filter_map(|item| match &item.content { + Content::Graphic(g) => Some((item.bounds(), g.size)), + _ => None, + }) + .collect(); + assert_eq!(graphics.len(), 1, "{graphics:?}"); + graphics[0] + }; + let (b, size) = shown(&session); + assert!((b.width() - ew).abs() < 1e-6 && (b.height() - eh).abs() < 1e-6, "{ppm:?}: {b:?}"); + assert!((size.0 - ew).abs() < 1e-6 && (size.1 - eh).abs() < 1e-6, "{ppm:?}: {size:?}"); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("placed.designcraft"); + session.execute("file.save", &json!({"path": path.to_string_lossy()})).unwrap(); + let mut reopened = Session::new(); + reopened.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap(); + assert_eq!(shown(&reopened), (b, size), "{ppm:?}"); + } + } + #[test] fn explicit_frame_wins_over_placement_and_selection() { let image = image_file().unwrap(); diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index 7540eee90..b32be1063 100644 --- a/crates/images/src/lib.rs +++ b/crates/images/src/lib.rs @@ -374,8 +374,8 @@ pub fn natural_size(bytes: &[u8]) -> Option<(f64, f64)> { return Some((bb[2] - bb[0], bb[3] - bb[1])).filter(|(w, h)| w.is_finite() && h.is_finite() && *w > 0.0 && *h > 0.0); } let (w, h) = pixel_size(bytes)?; - // Honor the file's density (#1991): a 600 px photo saved at 600 dpi places one inch wide, - // as in InDesign. Absurd metadata falls back to a pixel per point. + // Honor the file's density (#1991): a 600 px photo saved at 600 dpi places one inch wide. + // Absurd metadata falls back to a pixel per point. match raster_resolution(bytes) { Some(dpi) if resolution::sane_resolution(dpi) => Some((w as f64 * 72.0 / dpi.0, h as f64 * 72.0 / dpi.1)), _ => Some((w as f64, h as f64)), diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index c4a95e8e5..dabaf1c61 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -1,6 +1,6 @@ //! Embedded raster resolution: the density a placed graphic was saved with, so placing honors -//! it instead of assuming 72 ppi (#1991: a 600 px photo saved at 600 dpi places one inch wide, -//! as in InDesign). Reads JFIF/EXIF from JPEG (and EXIF from WebP), pHYs from PNG, the resolution +//! it instead of assuming 72 ppi (#1991: a 600 px photo saved at 600 dpi places one inch wide). +//! Reads JFIF/EXIF from JPEG (and EXIF from WebP), pHYs from PNG, the resolution //! tags from TIFF, and the header density from BMP. GIF, PSD and files without (usable) metadata //! report nothing and keep the pixel-is-a-point size. @@ -78,10 +78,10 @@ fn jpeg_resolution(bytes: &[u8]) -> Option<(f64, f64)> { if m == 0xE0 && jfif.is_none() { jfif = jfif_density(payload); } else if m == 0xE1 - && let Some(dpi) = exif_resolution(payload) + && let Some(dpi) = exif_resolution(payload).filter(|dpi| sane_resolution(*dpi)) { - // A non-EXIF APP1 (XMP, ICC, ...) parses to nothing: keep scanning, since a - // later APP1 can still carry the EXIF resolution. + // A non-EXIF APP1 (XMP, ICC, ...) or an unusable EXIF density parses to nothing: + // keep scanning, so a later APP1 or the JFIF density still sets the size. return Some(dpi); } pos += len; diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index 917693f87..f2fcf2bf0 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -537,6 +537,19 @@ fn jpeg_exif_resolution_beats_jfif() { assert_eq!(raster_resolution(&zero), None); } +/// An EXIF density of 0 ppi (or any value outside the usable range) does not hide the JFIF +/// density: the 600x900 px photo at a JFIF 300 dpi still places 144 x 216 pt. +#[test] +fn unusable_exif_density_keeps_the_jfif_density() { + for exif in [(0, 1), (1, 4), (1_000_000, 1)] { + let mut app = jfif_app(1, 300, 300); + app.extend_from_slice(&exif_app(&tiff_bytes(exif, exif, Some(2)))); + let jpg = jpeg_with(&app); + assert_eq!(raster_resolution(&jpg), Some((300.0, 300.0)), "EXIF {exif:?}"); + assert_eq!(natural_size(&jpg), Some((144.0, 216.0)), "EXIF {exif:?}"); + } +} + /// Big-endian EXIF parses too. #[test] fn jpeg_exif_big_endian_parses() { diff --git a/crates/render/src/decode.rs b/crates/render/src/decode.rs index 135dbf108..df8e1f047 100644 --- a/crates/render/src/decode.rs +++ b/crates/render/src/decode.rs @@ -87,7 +87,7 @@ pub fn image_size(bytes: &[u8]) -> Option<(u32, u32)> { } /// Natural size of a placed graphic, unrounded: a PDF page (crop box) and an SVG in points, a raster -/// in pixels, an EPS its bounding box independently of preview pixels. Placed graphics are drawn at +/// at its stored resolution (72 ppi without one), an EPS its bounding box independently of preview pixels. Placed graphics are drawn at /// this size; `image_size` rounds it to whole pixels for display. pub fn natural_size(bytes: &[u8], page: usize) -> Option<(f64, f64)> { if is_pdf(bytes) {