From c0a5a5cbf8a9a6529a2e8af7972e7214054e7f6e Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:33:45 +0100 Subject: [PATCH] fix(ui): resize image frames relative to the captured press Invert the actual affine basis instead of projecting onto assumed orthogonal axes. Capture the displayed crop frame for handles and apply its relative transform to the full artwork, preserving author clips on rotated pages. Exact out-and-back gestures preserve bytes and undo state, including off-centre presses. Synthetic headless regressions verify sheared and rotated images, crop-frame anchors, press-offset independence and saved/reopened placement. Degenerate and nonfinite bases are refused; the inverse guard has a negative-control test. Co-Authored-By: Codex GPT-6.1 --- .../ui-egui/src/tools/edit_text_ui/image.rs | 125 ++++++++++------- crates/ui-egui/tests/it/editing.rs | 132 ++++++++++++++++++ 2 files changed, 210 insertions(+), 47 deletions(-) diff --git a/crates/ui-egui/src/tools/edit_text_ui/image.rs b/crates/ui-egui/src/tools/edit_text_ui/image.rs index caef96330a..38c54c0b7c 100644 --- a/crates/ui-egui/src/tools/edit_text_ui/image.rs +++ b/crates/ui-egui/src/tools/edit_text_ui/image.rs @@ -66,6 +66,7 @@ pub struct ImageDrag { pub start: Pos2, pub target: DragTarget, pub initial_rect: Rect, + /// Captured user-space frame: the visible crop for resizing, full placement for moving. pub initial_matrix: [f64; 6], } @@ -125,6 +126,57 @@ impl CropMode { pub(crate) use pdfcraft_content::Matrix; +/// The displayed crop frame, expressed in page user space, rather than the full image placement. +fn resize_frame(obb: &ImageObb, xf: &PageXform, info: &DocInfo, page: usize) -> Matrix { + let p00 = screen_to_user(xf, info, page, obb.s00); + let p10 = screen_to_user(xf, info, page, obb.s10); + let p01 = screen_to_user(xf, info, page, obb.s01); + Matrix([p10.0 - p00.0, p10.1 - p00.1, p01.0 - p00.0, p01.1 - p00.1, p00.0, p00.1]) +} + +/// Resize from the captured press displacement in the frame's actual affine basis. +fn resize_affine_frame(frame: Matrix, handle: ImageHandle, delta: (f64, f64), proportional: bool) -> Option { + if !frame.0.iter().all(|v| v.is_finite()) || !delta.0.is_finite() || !delta.1.is_finite() { + return None; + } + if delta == (0.0, 0.0) { + return Some(frame); + } + let [a, b, c, d, _, _] = frame.0; + let inverse_basis = Matrix([a, b, c, d, 0.0, 0.0]).invert()?; + let (du, dv) = inverse_basis.apply(delta.0, delta.1); + if !du.is_finite() || !dv.is_finite() { + return None; + } + let (u, v) = match handle { + ImageHandle::TopRight => (1.0, 1.0), + ImageHandle::TopLeft => (-1.0, 1.0), + ImageHandle::BottomLeft => (-1.0, -1.0), + ImageHandle::BottomRight => (1.0, -1.0), + ImageHandle::RightCenter => (1.0, 0.0), + ImageHandle::LeftCenter => (-1.0, 0.0), + ImageHandle::TopCenter => (0.0, 1.0), + ImageHandle::BottomCenter => (0.0, -1.0), + }; + let (minimum_u, minimum_v) = (5.0 / a.hypot(b), 5.0 / c.hypot(d)); + let (mut su, mut sv) = (1.0 + u * du, 1.0 + v * dv); + if proportional && u != 0.0 && v != 0.0 { + let scale = if (su - 1.0).abs() >= (sv - 1.0).abs() { su } else { sv }; + su = scale.max(minimum_u).max(minimum_v); + sv = su; + } else { + if u != 0.0 { + su = su.max(minimum_u); + } + if v != 0.0 { + sv = sv.max(minimum_v); + } + } + let local = Matrix([su, 0.0, 0.0, sv, if u < 0.0 { 1.0 - su } else { 0.0 }, if v < 0.0 { 1.0 - sv } else { 0.0 }]); + let resized = local.then(&frame); + resized.0.iter().all(|v| v.is_finite()).then_some(resized) +} + pub(crate) fn user_to_screen(xf: &PageXform, info: &DocInfo, page: usize, x: f64, y: f64) -> Pos2 { let Some(p) = info.pages.get(page) else { return Pos2::ZERO }; let v = p.user_to_view(x as f32, y as f32); @@ -669,7 +721,12 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage if let Some(h) = handle_drag { if let Some(s) = view.image_selection.as_mut() { let o = origin.unwrap_or_else(|| obb.handle_pos(h)); - s.drag = Some(ImageDrag { start: o, target: DragTarget::Handle(h), initial_rect: b, initial_matrix: image.matrix }); + s.drag = Some(ImageDrag { + start: o, + target: DragTarget::Handle(h), + initial_rect: b, + initial_matrix: resize_frame(&obb, xf, info, page).0, + }); } } else if rot_drag { if let Some(s) = view.image_selection.as_mut() { @@ -687,7 +744,12 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage } } else if let Some((h, _)) = obb.handles().iter().find(|(_, pos)| pos.distance(o) < 8.0) { if let Some(s) = view.image_selection.as_mut() { - s.drag = Some(ImageDrag { start: o, target: DragTarget::Handle(*h), initial_rect: b, initial_matrix: image.matrix }); + s.drag = Some(ImageDrag { + start: o, + target: DragTarget::Handle(*h), + initial_rect: b, + initial_matrix: resize_frame(&obb, xf, info, page).0, + }); } } else if (image_contains(i, o) || quad_contains(obb.corners(), o)) && let Some(s) = view.image_selection.as_mut() @@ -805,11 +867,6 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage } let m_init = Matrix(drag.initial_matrix); let [a0, b0, c0, d0, e0, f0] = drag.initial_matrix; - let w0 = (a0 * a0 + b0 * b0).sqrt().max(1e-6); - let h0 = (c0 * c0 + d0 * d0).sqrt().max(1e-6); - let u_axis = (a0 / w0, b0 / w0); - let v_axis = (c0 / h0, d0 / h0); - let (sx, sy) = screen_to_user(xf, info, page, drag.start); let (px, py) = screen_to_user(xf, info, page, p); @@ -819,46 +876,9 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage (Matrix([a0, b0, c0, d0, e0 + dx, f0 + dy]), egui::CursorIcon::Move) } DragTarget::Handle(handle) => { - let p00 = (e0, f0); - let p10 = (e0 + a0, f0 + b0); - let p11 = (e0 + a0 + c0, f0 + b0 + d0); - let p01 = (e0 + c0, f0 + d0); - let p_curr = (px, py); - - // Signs point from the fixed opposite edge/corner toward the dragged grip. - let (u, v) = match handle { - ImageHandle::TopRight => (1.0, 1.0), - ImageHandle::TopLeft => (-1.0, 1.0), - ImageHandle::BottomLeft => (-1.0, -1.0), - ImageHandle::BottomRight => (1.0, -1.0), - ImageHandle::RightCenter => (1.0, 0.0), - ImageHandle::LeftCenter => (-1.0, 0.0), - ImageHandle::TopCenter => (0.0, 1.0), - ImageHandle::BottomCenter => (0.0, -1.0), - }; - let anchor = match (u < 0.0, v < 0.0) { - (false, false) => p00, - (true, false) => p10, - (true, true) => p11, - (false, true) => p01, - }; - let d = (p_curr.0 - anchor.0, p_curr.1 - anchor.1); - let du = (d.0 * u_axis.0 + d.1 * u_axis.1) * u; - let dv = (d.0 * v_axis.0 + d.1 * v_axis.1) * v; - let (new_w, new_h) = if u != 0.0 && v != 0.0 { - let (su, sv) = (du / w0, dv / h0); - let scale = if (su - 1.0).abs() >= (sv - 1.0).abs() { su } else { sv }; - let scale = scale.max(5.0 / w0).max(5.0 / h0); - (w0 * scale, h0 * scale) - } else { - (if u == 0.0 { w0 } else { du.max(5.0) }, if v == 0.0 { h0 } else { dv.max(5.0) }) - }; - let offset_u = if u < 0.0 { new_w } else { 0.0 }; - let offset_v = if v < 0.0 { new_h } else { 0.0 }; - let p00_new = (anchor.0 - offset_u * u_axis.0 - offset_v * v_axis.0, anchor.1 - offset_u * u_axis.1 - offset_v * v_axis.1); - let icon = obb.handle_cursor(handle); - let mat = Matrix([new_w * u_axis.0, new_w * u_axis.1, new_h * v_axis.0, new_h * v_axis.1, p00_new.0, p00_new.1]); - (mat, icon) + let proportional = !ui.input(|input| input.modifiers.shift); + let resized = resize_affine_frame(m_init, handle, (px - sx, py - sy), proportional).unwrap_or(m_init); + (resized, obb.handle_cursor(handle)) } // Turning is handled above. DragTarget::Rotate => (m_init, egui::CursorIcon::Default), @@ -1089,6 +1109,17 @@ fn flip_user_axis(horizontal: bool, rotation: u16) -> bool { mod screen_orientation_tests { use super::*; + #[test] + fn affine_resize_rejects_degenerate_or_nonfinite_frames() { + for frame in [[1.0, 1.0, 1.0, 1.0, 0.0, 0.0], [1.0, 1.0, 1.0, 1.0 + 1e-13, 0.0, 0.0], [f64::INFINITY, 0.0, 0.0, 1.0, 0.0, 0.0]] { + assert!(resize_affine_frame(Matrix(frame), ImageHandle::TopRight, (1.0, 1.0), true).is_none(), "{frame:?}"); + } + let frame = Matrix([100.0, 0.0, 50.0, 100.0, 100.0, 100.0]); + assert!(resize_affine_frame(frame, ImageHandle::TopRight, (10.0, 10.0), true).is_some()); + assert!(resize_affine_frame(frame, ImageHandle::TopRight, (f64::NAN, 0.0), true).is_none()); + assert!(resize_affine_frame(frame, ImageHandle::TopRight, (0.0, f64::INFINITY), true).is_none()); + } + #[test] fn flip_labels_follow_the_screen_axis() { for rotation in [0, 90, 180, 270] { diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 4e67e0056a..173e373a3b 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -9417,3 +9417,135 @@ fn author_image_clip_is_not_offered_as_a_removable_editor_crop() { h.run_steps(2); assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().page_images(0)[0].visible_rect, Some(visible)); } + +fn affine_review_image_fixture(matrix: [f64; 6], clip: Option<[f64; 4]>, rotation: i64) -> Vec { + use pdfcraft_cos::{Dict, Document, Object, Stream}; + let mut doc = Document::open(std::sync::Arc::new(fixture(1))).unwrap(); + let page = pdfcraft_model::pages(&doc).remove(0); + let mut image = Dict::new(); + image.set(b"Type".to_vec(), Object::name("XObject")); + image.set(b"Subtype".to_vec(), Object::name("Image")); + image.set(b"Width".to_vec(), Object::Int(1)); + image.set(b"Height".to_vec(), Object::Int(1)); + image.set(b"BitsPerComponent".to_vec(), Object::Int(8)); + image.set(b"ColorSpace".to_vec(), Object::name("DeviceRGB")); + let image = doc.add(Object::Stream(Stream::flate(image, &[220, 80, 50]))); + let mut xobjects = Dict::new(); + xobjects.set(b"ImReview".to_vec(), Object::Ref(image)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xobjects)); + let mut content = String::from("q "); + if let Some([x0, y0, x1, y1]) = clip { + content.push_str(&format!("{x0} {y0} {} {} re W n ", x1 - x0, y1 - y0)); + } + let [a, b, c, d, e, f] = matrix; + content.push_str(&format!("{a} {b} {c} {d} {e} {f} cm /ImReview Do Q")); + let stream = doc.add(Object::Stream(Stream::flate(Dict::new(), content.as_bytes()))); + doc.update_dict(page.obj, |dict| { + dict.set(b"Contents".to_vec(), Object::Ref(stream)); + dict.set(b"Resources".to_vec(), Object::Dict(resources)); + dict.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(360), Object::Int(360)])); + dict.set(b"Rotate".to_vec(), Object::Int(rotation)); + }) + .unwrap(); + pdfcraft_cos::write_incremental(&doc, &Default::default()).unwrap() +} + +fn affine_review_harness(bytes: Vec) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 1000.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("affine-review.pdf", None, bytes.clone()).unwrap(); + app + }); + h.run_steps(4); + h.state_mut().views[0].fit = pdfcraft_ui_egui::canvas::Fit::Page; + assert!(h.state_mut().execute("edit.edit_text")); + h.state_mut().views[0].image_selection = Some(pdfcraft_ui_egui::ImageSelection::at(0, 0)); + h.run_steps(3); + h +} + +#[test] +fn affine_review_resize_out_and_back_preserves_saved_placement() { + for (matrix, clip, rotation) in [ + ([100.0, 0.0, 0.0, 100.0, 100.0, 100.0], None, 0), + ([100.0, 0.0, 50.0, 100.0, 100.0, 100.0], None, 0), + ([86.60254, 50.0, -50.0, 86.60254, 150.0, 100.0], None, 0), + ([100.0, 0.0, 0.0, 100.0, 100.0, 100.0], Some([100.0, 100.0, 150.0, 200.0]), 90), + ] { + for offset in [egui::Vec2::ZERO, egui::vec2(3.0, 4.0)] { + let mut h = affine_review_harness(affine_review_image_fixture(matrix, clip, rotation)); + let view = &h.state().views[0]; + let id = view.id; + let doc = h.state().session.get(id).unwrap(); + let original = doc.page_images(0)[0].matrix; + let before = h.state().session.save_bytes(id).unwrap(); + let obb = pdfcraft_ui_egui::canvas::ImageObb::from_image(&doc.page_images(0)[0], &view.page_xform(0).unwrap(), &doc.info, 0); + let start = obb.handle_pos(pdfcraft_ui_egui::ImageHandle::TopRight) + offset; + h.hover_at(start); + h.run_steps(1); + h.drag_at(start); + h.run_steps(1); + h.hover_at(start + egui::vec2(30.0, -25.0)); + h.run_steps(2); + h.hover_at(start); + h.run_steps(2); + h.drop_at(start); + h.run_steps(3); + let saved = h.state().session.save_bytes(id).unwrap(); + assert!(saved == before, "out-and-back must not create an edit: matrix={matrix:?} clip={clip:?} rotation={rotation} offset={offset:?}"); + assert_eq!(h.state().session.get(id).unwrap().can_undo(), None); + let mut reopened = pdfcraft_engine::Session::new(); + let reopened_id = reopened.open("saved.pdf", None, saved, None).unwrap(); + assert_eq!(reopened.get(reopened_id).unwrap().page_images(0)[0].matrix, original); + } + } +} + +#[test] +fn affine_review_resize_uses_press_delta_and_affine_frame() { + for (matrix, clip, rotation) in [ + ([100.0, 0.0, 0.0, 100.0, 100.0, 100.0], None, 0), + ([100.0, 0.0, 50.0, 100.0, 100.0, 100.0], None, 0), + ([86.60254, 50.0, -50.0, 86.60254, 150.0, 100.0], None, 0), + ([100.0, 0.0, 0.0, 100.0, 100.0, 100.0], Some([100.0, 100.0, 150.0, 200.0]), 90), + ] { + let mut placements = Vec::new(); + for offset in [egui::Vec2::ZERO, egui::vec2(3.0, 4.0)] { + let mut h = affine_review_harness(affine_review_image_fixture(matrix, clip, rotation)); + let view = &h.state().views[0]; + let id = view.id; + let doc = h.state().session.get(id).unwrap(); + let xf = view.page_xform(0).unwrap(); + let obb = pdfcraft_ui_egui::canvas::ImageObb::from_image(&doc.page_images(0)[0], &xf, &doc.info, 0); + let grip = obb.handle_pos(pdfcraft_ui_egui::ImageHandle::TopRight); + let start = grip + offset; + let finish = start + (grip - obb.screen_center) * 0.2; + h.hover_at(start); + h.run_steps(1); + h.drag_at(start); + h.run_steps(1); + h.hover_at(finish); + h.run_steps(2); + h.drop_at(finish); + h.run_steps(3); + let saved = h.state().session.save_bytes(id).unwrap(); + let mut reopened = pdfcraft_engine::Session::new(); + let reopened_id = reopened.open("saved.pdf", None, saved, None).unwrap(); + let after = reopened.get(reopened_id).unwrap().page_images(0)[0].clone(); + let after_obb = pdfcraft_ui_egui::canvas::ImageObb::from_image(&after, &xf, &reopened.get(reopened_id).unwrap().info, 0); + assert!( + after_obb.handle_pos(pdfcraft_ui_egui::ImageHandle::BottomLeft).distance(obb.handle_pos(pdfcraft_ui_egui::ImageHandle::BottomLeft)) + < 0.1, + "opposite affine grip moved: {matrix:?} clip={clip:?} rotation={rotation}" + ); + assert!( + after_obb.handle_pos(pdfcraft_ui_egui::ImageHandle::TopRight).distance(grip + (grip - obb.screen_center) * 0.2) < 0.2, + "resized affine frame missed the press-relative target: {matrix:?} clip={clip:?} rotation={rotation} offset={offset:?}" + ); + placements.push(after.matrix); + } + assert!(placements[0].into_iter().zip(placements[1]).all(|(a, b)| (a - b).abs() < 1e-3), "press offset changed placement: {placements:?}"); + } +}