From 481b7d2f3b4caa756b11a4b893c656398b9a5d74 Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:17:58 +1000 Subject: [PATCH] PARITY-EFFECTIVE-PPI: check transformed image resolution Co-Authored-By: Codex --- crates/engine/src/cmd/preflight.rs | 87 ++++---- crates/engine/tests/preflight_resolution.rs | 217 ++++++++++++++++++++ 2 files changed, 264 insertions(+), 40 deletions(-) create mode 100644 crates/engine/tests/preflight_resolution.rs diff --git a/crates/engine/src/cmd/preflight.rs b/crates/engine/src/cmd/preflight.rs index 3fc061d..f5ee8df 100644 --- a/crates/engine/src/cmd/preflight.rs +++ b/crates/engine/src/cmd/preflight.rs @@ -5,6 +5,7 @@ use designcraft_compose::ComposedStory; use designcraft_doc::{Content, Document, Intent, Item, SpreadRef}; use designcraft_fonts::FaceRef; +use designcraft_geom::Affine; use serde::Serialize; use serde_json::{Value, json}; @@ -29,10 +30,12 @@ pub fn specs() -> Vec { "{minPpi?: 150} → issues [{severity, kind, message, item?, page?}]", has_doc, run)] } -fn page_of(d: &Document, sr: SpreadRef, it: &Item) -> Option { +fn page_of(d: &Document, sr: SpreadRef, it: &Item, parent: Affine) -> Option { let SpreadRef::Doc(si) = sr else { return None }; let sp = d.spreads.get(si)?; - Some(d.first_page_of_spread(si) + sp.page_at_x(it.bounds().center().x).unwrap_or(0)) + // Item bounds include its own transform; ancestors bring the frame into spread space. + let bounds = parent.transform_rect_bbox(it.bounds()); + Some(d.first_page_of_spread(si) + sp.page_at_x(bounds.center().x).unwrap_or(0)) } /// Characters a missing-glyph report names before "and N more". @@ -85,7 +88,7 @@ pub fn check(s: &Session, min_ppi: f64) -> Vec { missing_glyphs(&cs, &story.text, 0, &mut missing); if cs.is_overset() { let last = story.frames.last().copied(); - let page = last.and_then(|f| d.find(f).zip(d.item(f))).and_then(|(loc, it)| page_of(d, loc.spread, it)); + let page = last.and_then(|f| d.find(f).zip(d.item(f))).and_then(|(loc, it)| page_of(d, loc.spread, it, Affine::IDENTITY)); let n = story.text[cs.overset_at.unwrap_or(0).min(story.len())..].chars().count(); out.push(Issue { severity: "error", kind: "overset", message: format!("Overset text: {n} characters"), item: last.map(|i| i.0), page }); } @@ -161,48 +164,52 @@ pub fn check(s: &Session, min_ppi: f64) -> Vec { page: None, }); } - // Items. + // Carry the ancestor transform in the same order as rendering, retaining pre-order. for (si, sp) in d.spreads.iter().enumerate() { - for top in &sp.items { - top.walk(&mut |it| { - let page = page_of(d, SpreadRef::Doc(si), it); - match &it.content { - Content::Graphic(g) => match d.assets.get(&g.asset) { - None => { - out.push(Issue { severity: "error", kind: "missingLink", message: "Missing graphic".into(), item: Some(it.id.0), page }) - } - Some(a) if a.data.is_empty() => out.push(Issue { - severity: "error", - kind: "missingLink", - message: format!("Missing link: {}", a.link.as_deref().unwrap_or(&a.name)), - item: Some(it.id.0), - page, - }), - Some(a) => { - // Effective ppi: pixels per inch at the placed size. Placed PDF and - // SVG pages export as vectors; their `pixels` is only the preview's. - let vector = designcraft_images::is_pdf(&a.data) || designcraft_images::is_svg(&a.data); - if let Some((pw, _)) = a.pixels.filter(|_| !vector) { - let placed_w = (g.xf * it.xf).as_coeffs()[0].hypot((g.xf * it.xf).as_coeffs()[1]) * g.size.0; - let ppi = pw as f64 / (placed_w / 72.0).max(1e-6); - if d.settings.intent == Intent::Print && ppi < min_ppi { - out.push(Issue { - severity: "warning", - kind: "lowResolution", - message: format!("{}: effective {ppi:.0} ppi (< {min_ppi:.0})", a.name), - item: Some(it.id.0), - page, - }); - } + let mut pending: Vec<_> = sp.items.iter().rev().map(|it| (it.as_ref(), Affine::IDENTITY)).collect(); + while let Some((it, parent)) = pending.pop() { + let xf = parent * it.xf; + pending.extend(it.children().iter().rev().map(|child| (child.as_ref(), xf))); + let page = page_of(d, SpreadRef::Doc(si), it, parent); + match &it.content { + Content::Graphic(g) => match d.assets.get(&g.asset) { + None => out.push(Issue { severity: "error", kind: "missingLink", message: "Missing graphic".into(), item: Some(it.id.0), page }), + Some(a) if a.data.is_empty() => out.push(Issue { + severity: "error", + kind: "missingLink", + message: format!("Missing link: {}", a.link.as_deref().unwrap_or(&a.name)), + item: Some(it.id.0), + page, + }), + Some(a) => { + // Placed PDF and SVG pages export as vectors; `pixels` is only the preview's. + let vector = designcraft_images::is_pdf(&a.data) || designcraft_images::is_svg(&a.data); + if let Some((pw, ph)) = a.pixels.filter(|_| !vector) { + // Measure each image axis in points, not its rotated bounding box or + // cropped frame. Either axis can fall below the minimum resolution. + let [xx, yx, xy, yy, _, _] = (xf * g.xf).as_coeffs(); + let placed_w = xx.hypot(yx) * g.size.0; + let placed_h = xy.hypot(yy) * g.size.1; + let ppi_x = pw as f64 / (placed_w / 72.0).max(1e-6); + let ppi_y = ph as f64 / (placed_h / 72.0).max(1e-6); + let ppi = ppi_x.min(ppi_y); + if d.settings.intent == Intent::Print && ppi < min_ppi { + out.push(Issue { + severity: "warning", + kind: "lowResolution", + message: format!("{}: effective {ppi:.0} ppi (< {min_ppi:.0})", a.name), + item: Some(it.id.0), + page, + }); } } - }, - Content::Text(tf) if d.story(tf.story).is_some_and(|s| s.is_empty()) => { - out.push(Issue { severity: "warning", kind: "emptyText", message: "Empty text frame".into(), item: Some(it.id.0), page }); } - _ => {} + }, + Content::Text(tf) if d.story(tf.story).is_some_and(|s| s.is_empty()) => { + out.push(Issue { severity: "warning", kind: "emptyText", message: "Empty text frame".into(), item: Some(it.id.0), page }); } - }); + _ => {} + } } } // RGB swatches in use in a print document. diff --git a/crates/engine/tests/preflight_resolution.rs b/crates/engine/tests/preflight_resolution.rs new file mode 100644 index 0000000..f4562c5 --- /dev/null +++ b/crates/engine/tests/preflight_resolution.rs @@ -0,0 +1,217 @@ +//! Effective raster resolution follows image, frame and ancestor transforms in spread space. +use std::sync::Arc; + +use designcraft_doc::{Content, Intent, ItemId}; +use designcraft_engine::{Session, cmd::base64_encode}; +use designcraft_geom::Affine; +use serde_json::{Value, json}; + +fn place(s: &mut Session, name: &str, bytes: &[u8]) -> ItemId { + s.execute("selection.set", &json!({"ids": []})).unwrap(); + let placed = s.execute("file.place", &json!({"base64": base64_encode(bytes), "name": name, "x": 0, "y": 0, "width": 144})).unwrap(); + ItemId(placed["id"].as_u64().unwrap()) +} + +fn raster() -> (Session, ItemId) { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 1000, "height": 1000, "facingPages": false})).unwrap(); + // 600 × 300 pixels at 144 × 72 points: 300 ppi in both directions. + let png = designcraft_render::Rendered { width: 600, height: 300, pixels: vec![200; 600 * 300 * 4] }.to_png(); + let id = place(&mut s, "photo.png", &png); + assert_eq!(s.doc().unwrap().doc.item(id).unwrap().inner_bounds().size(), designcraft_geom::Size::new(144.0, 72.0)); + (s, id) +} + +fn facing_raster() -> (Session, ItemId) { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 612, "height": 792, "pages": 3, "facingPages": true})).unwrap(); + // Page 0 is the cover; spread 1 contains absolute pages 1 (left) and 2 (right). + let d = &s.doc().unwrap().doc; + assert_eq!(d.spreads.len(), 2); + assert_eq!(d.spreads[0].pages.len(), 1); + assert_eq!(d.spreads[1].pages.len(), 2); + assert_eq!(d.first_page_of_spread(1), 1); + assert_eq!(d.spreads[1].pages[0].x, 0.0); + assert_eq!(d.spreads[1].pages[1].x, 612.0); + let png = designcraft_render::Rendered { width: 600, height: 300, pixels: vec![200; 600 * 300 * 4] }.to_png(); + let placed = s + .execute( + "file.place", + &json!({ + "base64": base64_encode(&png), "name": "photo.png", "spread": 1, + "x": 0, "y": 0, "width": 144 + }), + ) + .unwrap(); + let id = ItemId(placed["id"].as_u64().unwrap()); + let d = &s.doc().unwrap().doc; + assert_eq!(d.find(id).unwrap().spread, designcraft_doc::SpreadRef::Doc(1)); + assert_eq!(d.item(id).unwrap().inner_bounds().size(), designcraft_geom::Size::new(144.0, 72.0)); + (s, id) +} + +fn transform(s: &mut Session, id: ItemId, frame: Affine, image: Affine) { + s.edit(|d, _| { + let it = d.item_mut(id).unwrap(); + it.xf = frame; + let Content::Graphic(g) = &mut it.content else { panic!("graphic fixture") }; + g.xf = image * g.xf; + Ok(()) + }) + .unwrap(); +} + +fn group(s: &mut Session, child: ItemId, xf: Affine) -> ItemId { + let spread = s.doc().unwrap().doc.find(child).unwrap().spread; + let marker = s.execute("frame.create", &json!({"spread": spread, "rect": [0, 0, 1, 1]})).unwrap()["id"].clone(); + let group = s.execute("object.group", &json!({"ids": [child.0, marker]})).unwrap(); + let id = ItemId(group["id"].as_u64().unwrap()); + s.edit(|d, _| { + d.item_mut(id).unwrap().xf = xf; + Ok(()) + }) + .unwrap(); + id +} + +fn resolutions(s: &mut Session, params: Value) -> Vec { + let before = s.doc().unwrap(); + let doc = before.doc.clone(); + let selection = before.selection.clone(); + let state = (before.revision, before.history.undo.len(), before.history.redo.len(), s.journal.len()); + let result = s.execute("preflight.run", ¶ms).unwrap(); + let after = s.doc().unwrap(); + assert!(Arc::ptr_eq(&doc, &after.doc), "preflight must leave the document and assets untouched"); + assert_eq!(selection, after.selection, "preflight must leave the selection untouched"); + assert_eq!(state, (after.revision, after.history.undo.len(), after.history.redo.len(), s.journal.len())); + result["issues"].as_array().unwrap().iter().filter(|i| i["kind"] == "lowResolution").cloned().collect() +} + +fn warning(s: &mut Session, id: ItemId, params: Value, ppi: u32, threshold: u32) { + warning_on_page(s, id, params, ppi, threshold, 0); +} + +fn warning_on_page(s: &mut Session, id: ItemId, params: Value, ppi: u32, threshold: u32, page: usize) { + assert_eq!( + resolutions(s, params), + vec![json!({ + "severity": "warning", "kind": "lowResolution", "item": id.0, "page": page, + "message": format!("photo.png: effective {ppi} ppi (< {threshold})") + })] + ); +} + +#[test] +fn either_raster_axis_can_fail_after_image_or_frame_scaling() { + for scale in [Affine::scale_non_uniform(4.0, 1.0), Affine::scale_non_uniform(1.0, 4.0)] { + for image_only in [false, true] { + let (mut s, id) = raster(); + let (frame, image) = if image_only { (Affine::IDENTITY, scale) } else { (scale, Affine::IDENTITY) }; + transform(&mut s, id, frame, image); + // One axis remains 300 ppi, the other is 75 ppi: report the worse axis once. + warning(&mut s, id, json!({}), 75, 150); + } + } +} + +#[test] +fn nested_groups_accumulate_scale_and_keep_image_frame_parent_order() { + let (mut s, id) = raster(); + let inner = group(&mut s, id, Affine::scale(2.0)); + group(&mut s, inner, Affine::scale(2.0)); + warning(&mut s, id, json!({}), 75, 150); + + let (mut s, id) = raster(); + // Double image X, then turn the frame 90 degrees, then triple spread X. + // The image axes become 288 and 216 pt, giving 150 and 100 ppi respectively. + transform(&mut s, id, Affine::new([0.0, 1.0, -1.0, 0.0, 400.0, 20.0]), Affine::scale_non_uniform(2.0, 1.0)); + group(&mut s, id, Affine::scale_non_uniform(3.0, 1.0)); + warning(&mut s, id, json!({}), 100, 150); +} + +#[test] +fn translated_scaled_nested_raster_reports_right_facing_page() { + let (mut s, id) = facing_raster(); + let inner = group(&mut s, id, Affine::scale(2.0)); + group(&mut s, inner, Affine::translate((640.0, 0.0)) * Affine::scale(2.0)); + // Local frame center x=72 is on the left page. Ancestors move its bounds to + // x=640..1216 on the right page, while scaling 300 ppi down to 75 ppi. + warning_on_page(&mut s, id, json!({}), 75, 150, 2); +} + +#[test] +fn translating_group_changes_warning_page_without_changing_resolution() { + let (mut s, id) = facing_raster(); + // A nonidentity frame transform also detects applying the frame transform twice. + transform(&mut s, id, Affine::translate((-240.0, 0.0)), Affine::IDENTITY); + warning_on_page(&mut s, id, json!({"minPpi": 301}), 300, 301, 1); + group(&mut s, id, Affine::translate((880.0, 0.0))); + // Translation moves the center to x=712, but does not change either image axis. + assert!(resolutions(&mut s, json!({})).is_empty()); + warning_on_page(&mut s, id, json!({"minPpi": 301}), 300, 301, 2); +} + +#[test] +fn rotation_translation_and_reflection_do_not_change_resolution() { + for xf in [ + Affine::translate((250.0, 300.0)) * Affine::rotate(std::f64::consts::FRAC_PI_4), + Affine::translate((250.0, 300.0)) * Affine::scale_non_uniform(-1.0, 1.0), + ] { + let (mut s, id) = raster(); + transform(&mut s, id, xf, Affine::IDENTITY); + // Measuring the rotated bounding box instead would incorrectly warn here. + assert!(resolutions(&mut s, json!({"minPpi": 250})).is_empty()); + warning(&mut s, id, json!({"minPpi": 301}), 300, 301); + } +} + +#[test] +fn cropping_keeps_resolution_and_fitting_changes_it_with_content_scale() { + let (mut s, id) = raster(); + s.execute("transform.resize", &json!({"ids": [id.0], "from": [0, 0, 144, 72], "to": [0, 0, 36, 18]})).unwrap(); + warning(&mut s, id, json!({"minPpi": 301}), 300, 301); + s.execute("transform.resize", &json!({"ids": [id.0], "from": [0, 0, 36, 18], "to": [0, 0, 144, 216]})).unwrap(); + warning(&mut s, id, json!({"minPpi": 301}), 300, 301); + + s.execute("object.fit", &json!({"ids": [id.0], "mode": "fitProportionally"})).unwrap(); + warning(&mut s, id, json!({"minPpi": 301}), 300, 301); + // Stretching just Y to 216 pt leaves X at 300 ppi but reduces Y to 100 ppi. + s.execute("object.fit", &json!({"ids": [id.0], "mode": "fitContentToFrame"})).unwrap(); + warning(&mut s, id, json!({}), 100, 150); + // Proportional fill is 432 × 216 pt: 100 ppi in both directions. + s.execute("object.fit", &json!({"ids": [id.0], "mode": "fillProportionally"})).unwrap(); + warning(&mut s, id, json!({}), 100, 150); + s.execute("object.fit", &json!({"ids": [id.0], "mode": "centerContent"})).unwrap(); + warning(&mut s, id, json!({}), 100, 150); +} + +#[test] +fn threshold_is_strict_and_resolution_remains_print_only() { + let (mut s, id) = raster(); + transform(&mut s, id, Affine::scale_non_uniform(1.0, 2.0), Affine::IDENTITY); + assert!(resolutions(&mut s, json!({})).is_empty(), "150 ppi meets the default threshold"); + assert!(resolutions(&mut s, json!({"minPpi": 149})).is_empty()); + warning(&mut s, id, json!({"minPpi": 151}), 150, 151); + for intent in [Intent::Web, Intent::Mobile] { + s.edit(|d, _| { + d.settings.intent = intent; + Ok(()) + }) + .unwrap(); + assert!(resolutions(&mut s, json!({"minPpi": 151})).is_empty()); + } +} + +#[test] +fn transformed_vector_previews_are_excluded_beside_rasters() { + let (mut s, id) = raster(); + let svg = br#""#; + let vector = place(&mut s, "vector.svg", svg); + let parent = s.execute("object.group", &json!({"ids": [id.0, vector.0]})).unwrap()["id"].as_u64().unwrap(); + s.edit(|d, _| { + d.item_mut(ItemId(parent)).unwrap().xf = Affine::scale_non_uniform(1.0, 4.0); + Ok(()) + }) + .unwrap(); + warning(&mut s, id, json!({}), 75, 150); +}