From 0f5364ee9f655021ab23a5b4503d8ca4f5d65071 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Mon, 2 Nov 2026 22:05:35 -0600 Subject: [PATCH] picture: use displayed group size for compression --- ROADMAP.md | 1 + crates/engine/src/cmd/compress.rs | 140 ++++++++++++++++++++++++++++-- docs/parity-checklist.md | 3 +- 3 files changed, 134 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 2c8c6571..e1d1b772 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -149,6 +149,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-11-02 | Picture compression measures displayed dimensions through nested group transforms; checklist score unchanged | | 2026-11-02 | File › About replaces the unused Account page, with version, project links, contributor/model credits and appearance preferences | | 2026-10-31 | No Fill shape interiors allow selection and editing of underlying objects, even when a quick style remains; hit-testing uses the renderer’s fill resolution. Outlines, rotation, text targets and actual fills keep their hit behaviour (#1621) | | 2026-10-30 | Print-preview tests use the URL emitted by egui and its exact file path; previews are explicitly removed before the macOS process exits | diff --git a/crates/engine/src/cmd/compress.rs b/crates/engine/src/cmd/compress.rs index 5708efbb..1f795967 100644 --- a/crates/engine/src/cmd/compress.rs +++ b/crates/engine/src/cmd/compress.rs @@ -116,16 +116,31 @@ fn pack(data: &[u8], crop: [f64; 4], w_pt: f64, h_pt: f64, ppi: f64, delete_crop Some(Packed { bytes, content_type, crop: new_crop }) } -/// Picture shapes in `shapes` (groups included) that `want` accepts. -fn pictures_mut<'a>(shapes: &'a mut [Shape], want: &dyn Fn(ShapeId) -> bool, out: &mut Vec<&'a mut Shape>, depth: usize) { +/// Pictures accepted by `want`, with width and height measured in slide space through group transforms. +fn pictures_mut<'a>( + shapes: &'a mut [Shape], + want: &dyn Fn(ShapeId) -> bool, + out: &mut Vec<(&'a mut Shape, f64, f64)>, + to_slide: deckcraft_geom::Affine, + depth: usize, +) { if depth > 64 { return; } for sh in shapes { - if matches!(sh.kind, ShapeKind::Picture { .. }) && want(sh.id) { - out.push(sh); - } else if let Some(ch) = sh.children_mut() { - pictures_mut(ch, want, out, depth + 1); + let Some(xfrm) = sh.xfrm else { continue }; + let local_to_slide = to_slide * xfrm.affine(); + if matches!(&sh.kind, ShapeKind::Picture { .. }) && want(sh.id) { + let origin = local_to_slide * deckcraft_geom::Point::ZERO; + let across = local_to_slide * deckcraft_geom::Point::new(xfrm.w, 0.0); + let down = local_to_slide * deckcraft_geom::Point::new(0.0, xfrm.h); + out.push((sh, (across - origin).hypot(), (down - origin).hypot())); + continue; + } + if let ShapeKind::Group { children, child } = &mut sh.kind { + let child_rect = deckcraft_geom::Rect::new(child.x, child.y, child.x + child.w, child.y + child.h); + let child_to_slide = to_slide * deckcraft_geom::group_child_affine(&xfrm, child_rect); + pictures_mut(children, want, out, child_to_slide, depth + 1); } } } @@ -155,10 +170,9 @@ fn compress(s: &mut Session, p: &Value) -> Result { let Some(slide) = doc.slides.get(i).cloned() else { continue }; let mut slide = slide.as_ref().clone(); let mut pics = vec![]; - pictures_mut(&mut slide.shapes, &want, &mut pics, 0); + pictures_mut(&mut slide.shapes, &want, &mut pics, deckcraft_geom::Affine::IDENTITY, 0); let mut touched = false; - for sh in pics { - let (w, h) = sh.xfrm.map(|x| (x.w, x.h)).unwrap_or((0.0, 0.0)); + for (sh, w, h) in pics { let ShapeKind::Picture { fill } = &mut sh.kind else { continue }; let PictureFill { media, crop, .. } = fill.clone(); let key = (media.0, crop.map(f64::to_bits), w.to_bits(), h.to_bits()); @@ -232,6 +246,114 @@ mod tests { assert!(pack(b"", [0.0; 4], 72.0, 72.0, 150.0, true).is_none()); } + #[test] + fn grouped_pictures_use_their_displayed_size() { + let mut s = Session::with_new(); + let original = png(600, 600); + s.edit(|doc, _| { + let media = doc.add_media("grouped.png", "image/png", original.clone()).ok_or_else(|| bad("test", "media"))?; + let picture = Shape { + id: ShapeId(70_101), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0)), + kind: ShapeKind::Picture { fill: PictureFill { media, ..Default::default() } }, + ..Default::default() + }; + let nested = Shape { + id: ShapeId(70_102), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0)), + kind: ShapeKind::Group { children: vec![picture], child: deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0) }, + ..Default::default() + }; + let group = Shape { + id: ShapeId(70_103), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 720.0, 720.0)), + kind: ShapeKind::Group { children: vec![nested], child: deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0) }, + ..Default::default() + }; + let slide = doc.slides.first_mut().ok_or_else(|| bad("test", "slide"))?; + Arc::make_mut(slide).shapes.push(group); + Ok(()) + }) + .unwrap(); + + let result = s.execute("picture.compress", &json!({"all": true, "ppi": 96, "deleteCropped": false})).unwrap(); + assert_eq!(result["pictures"], 0, "a 600 px image displayed at 720×720 pt is already below the requested sampling density"); + assert!( + s.doc().unwrap().doc.media.iter().any(|m| m.data.as_slice() == original.as_slice()), + "the under-target original media must remain untouched" + ); + } + + #[test] + fn anisotropic_nested_group_scaling_controls_sampling_dimensions() { + let mut s = Session::with_new(); + let original = png(2000, 1000); + s.edit(|doc, _| { + let media = doc.add_media("scaled-group.png", "image/png", original).ok_or_else(|| bad("test", "media"))?; + let picture = Shape { + id: ShapeId(70_111), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0)), + kind: ShapeKind::Picture { fill: PictureFill { media, ..Default::default() } }, + ..Default::default() + }; + let nested = Shape { + id: ShapeId(70_112), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0)), + kind: ShapeKind::Group { children: vec![picture], child: deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0) }, + ..Default::default() + }; + let group = Shape { + id: ShapeId(70_113), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 720.0, 360.0)), + kind: ShapeKind::Group { children: vec![nested], child: deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0) }, + ..Default::default() + }; + let slide = doc.slides.first_mut().ok_or_else(|| bad("test", "slide"))?; + Arc::make_mut(slide).shapes.push(group); + Ok(()) + }) + .unwrap(); + + let result = s.execute("picture.compress", &json!({"all": true, "ppi": 96, "deleteCropped": false})).unwrap(); + assert_eq!(result["pictures"], 1); + let slide = s.doc().unwrap().doc.slides.first().unwrap(); + let picture = slide.shapes.iter().find(|sh| sh.id == ShapeId(70_113)).unwrap(); + let ShapeKind::Group { children, .. } = &picture.kind else { panic!("outer group expected") }; + let ShapeKind::Group { children, .. } = &children[0].kind else { panic!("nested group expected") }; + let ShapeKind::Picture { fill } = &children[0].kind else { panic!("picture expected") }; + let media = s.doc().unwrap().doc.media(fill.media).unwrap(); + let image = image::load_from_memory(&media.data).unwrap(); + assert_eq!((image.width(), image.height()), (960, 480)); + } + + #[test] + fn ungrouped_picture_still_compresses_and_deletes_its_crop() { + let mut s = Session::with_new(); + let original = png(800, 800); + s.edit(|doc, _| { + let media = doc.add_media("cropped.png", "image/png", original).ok_or_else(|| bad("test", "media"))?; + let slide = doc.slides.first_mut().ok_or_else(|| bad("test", "slide"))?; + Arc::make_mut(slide).shapes.push(Shape { + id: ShapeId(70_104), + xfrm: Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 72.0)), + kind: ShapeKind::Picture { fill: PictureFill { media, crop: [0.5, 0.0, 0.0, 0.0], ..Default::default() } }, + ..Default::default() + }); + Ok(()) + }) + .unwrap(); + + let result = s.execute("picture.compress", &json!({"all": true, "ppi": 96, "deleteCropped": true})).unwrap(); + assert_eq!(result["pictures"], 1); + let slide = s.doc().unwrap().doc.slides.first().unwrap(); + let picture = slide.shapes.iter().find(|sh| sh.id == ShapeId(70_104)).unwrap(); + let ShapeKind::Picture { fill } = &picture.kind else { panic!("picture expected") }; + assert_eq!(fill.crop, [0.0; 4]); + let media = s.doc().unwrap().doc.media(fill.media).unwrap(); + let image = image::load_from_memory(&media.data).unwrap(); + assert_eq!((image.width(), image.height()), (96, 192)); + } + #[test] fn pack_deletes_cropped_areas_or_keeps_them() { let src = png(400, 400); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 606f1228..46db1300 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -2,7 +2,7 @@ > **Generated:** 2026-10-14 (bullets/numbering remain D after empty-line marker review; summary by `cargo xtask parity`; rows re-scored by the 2026-10-14 community PR wave, duplicate Thesaurus, AutoCorrect, Artistic effects and Corrections rows folded; Preview in editor re-scored 2026-10-13; earlier rows re-scored 2026-10-10 against PowerPoint for Mac 16.113.4; video/animated GIF row re-scored 2026-10-11, #171) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) -> **Last updated:** 2026-11-01 · Locked-shape and no-outline regressions added for #1621; fill row re-assessed: remains D; No Fill hit-testing now follows resolved appearance. Scores unchanged. +> **Last updated:** 2026-11-02 · Picture compression now measures displayed dimensions through nested group transforms; Compress pictures remains D. Scores unchanged. The summary between ` **Weighted breadth parity: 84%** over 191 features. @@ -240,4 +240,5 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Date | Change | Summary | |---|---|---| +| 2026-11-02 | trivial | Picture compression uses displayed dimensions through nested group transforms; Compress pictures remains D and summary scores are unchanged | | 2026-10-31 | trivial | No Fill interior hit-testing follows resolved fill (#1621); fill row remains D and summary scores are unchanged |