From 75e012de9a354b51944f581d9ea80c8f7093bd68 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:40:52 +0100 Subject: [PATCH] fix(edit): compose inside-scope group translations in order Fold a leading transform inside the image scope as cm followed by the local transform; an enclosing transform uses the opposite multiplication order. This preserves the complete requested image and crop displacement under an outer scale. The exact crop-arrange-group-move fixture previously moved only five points for a ten-point request; saved geometry and rasters now match an independent direct Transform control across both axes and signed offsets. Full scoped Linux native, wasm and repository gates pass. Co-Authored-By: Codex GPT-6.1 --- crates/edit/src/images.rs | 8 ++++++-- crates/edit/src/tests.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index a2fe8e03e8..2920a2a9ad 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -1894,8 +1894,12 @@ pub(crate) fn translate_images(doc: &mut Document, page: usize, indexes: &[usize } ops.get(at)?.nums::<6>().map(|m| (at, Matrix(m))) }; - if let Some((at, cm)) = enclosing_transform(&ops, start, end).or_else(leading_transform) { - let folded = local.then(&cm); + // An enclosing cm is before the captured placement; an inside-scope cm is after it. + // Their multiplication orders differ when that cm scales or shears a translation. + let folded = enclosing_transform(&ops, start, end) + .map(|(at, cm)| (at, local.then(&cm))) + .or_else(|| leading_transform().map(|(at, cm)| (at, cm.then(&local)))); + if let Some((at, folded)) = folded { if !folded.0.iter().all(|v| v.is_finite() && v.abs() <= 1e12) { return Err(EditError::Invalid("the image move is too large".into())); } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 72e0a232f8..f7740d60cd 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -10275,3 +10275,31 @@ fn review_soft_masks_sharing_a_group_keep_distinct_configurations() { assert!(state.visually_differs(&luminosity), "mask backdrop and transfer function belong to its definition"); } } + +#[test] +fn review_inside_scope_group_move_matches_direct_transform_under_outer_scale() { + for scale in [1, 2] { + let content = format!("{scale} 0 0 {scale} 0 0 cm q 40 0 0 40 10 10 cm /Im0 Do Q q 0 0 1 rg 75 75 5 5 re f Q"); + let mut cropped = image_page_with(&content); + let keep = if scale == 2 { [30.0, 30.0, 70.0, 70.0] } else { [15.0, 15.0, 35.0, 35.0] }; + crop(&mut cropped, 0, 0, keep).unwrap(); + arrange(&mut cropped, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); + for offset in [[10.0, 0.0], [0.0, 10.0], [-10.0, 3.0]] { + let mut grouped = reopen(&cropped); + let mut direct = reopen(&cropped); + let before = images::page_images(&grouped, 0).unwrap()[0].clone(); + images::translate_images(&mut grouped, 0, &[0], offset).unwrap(); + images::change_image(&mut direct, 0, 0, &images::ImageChange::Transform([1.0, 0.0, 0.0, 1.0, offset[0], offset[1]])).unwrap(); + let moved = |r: [f64; 4]| [r[0] + offset[0], r[1] + offset[1], r[2] + offset[0], r[3] + offset[1]]; + let saved = reopen(&grouped); + let image = &images::page_images(&saved, 0).unwrap()[0]; + assert!(close(image.rect, moved(before.rect)), "scale {scale} offset {offset:?}: {:?}", image.rect); + assert!(close(image.visible_rect.unwrap(), moved(before.visible_rect.unwrap())), "the crop follows the complete displacement"); + assert_eq!( + review_state_raster(&saved), + review_state_raster(&reopen(&direct)), + "group and direct transforms must agree, preserving other artwork" + ); + } + } +}