fix(ui): keep image grips and flips aligned with the visible page

Use matrix resizing whenever image axes are turned or reflected on screen, including document and view rotation. Share the visible grip geometry for cursor claims and hit routing, and use only the drawn rotate knob. Map flip labels to the visible axis and defer Cut selection clearing to its result.

Regressions cover all page/view rotation combinations and 64 outward drags across every grip, asserting fixed anchors and pointer tracking for upright and rotated images.

Co-Authored-By: Codex GPT-6.1 <noreply@openai.com>
This commit is contained in:
FredApps
2026-11-01 14:58:21 +01:00
co-authored by Codex GPT-6.1
parent 0046177f47
commit d423dd03ec
5 changed files with 179 additions and 48 deletions
+25 -5
View File
@@ -152,6 +152,21 @@ pub struct ImageObb {
}
impl ImageObb {
pub(crate) fn handle_cursor(&self, handle: ImageHandle) -> egui::CursorIcon {
let direction = self.handle_pos(handle) - self.screen_center;
match handle {
ImageHandle::TopLeft | ImageHandle::TopRight | ImageHandle::BottomLeft | ImageHandle::BottomRight => {
if direction.x * direction.y > 0.0 {
egui::CursorIcon::ResizeNwSe
} else {
egui::CursorIcon::ResizeNeSw
}
}
_ if direction.x.abs() > direction.y.abs() => egui::CursorIcon::ResizeHorizontal,
_ => egui::CursorIcon::ResizeVertical,
}
}
pub(crate) fn contains(&self, visible: Rect, p: Pos2) -> bool {
(visible.is_positive() && visible.contains(p)) || crate::edit_text_ui::quad_contains(self.corners(), p)
}
@@ -184,14 +199,19 @@ impl ImageObb {
let h_v = (c * c + d * d).sqrt().max(1e-6);
let u_axis = (a / w_u, b / w_u);
let v_axis = (c / h_v, d / h_v);
let is_rotated = b.abs() > 1e-4 || c.abs() > 1e-4;
// The fast rectangle path interprets labels in screen space. Use matrix
// resizing whenever the image's user-space axes are turned or reflected
// on screen, including page /Rotate and View > Rotate View.
let u = s10 - s00;
let v = s01 - s00;
let is_rotated = u.y.abs() > 1e-4 || v.x.abs() > 1e-4 || u.x < 0.0 || v.y > 0.0;
Self { s00, s10, s11, s01, s_bc, s_rc, s_tc, s_lc, s_rot, screen_center, u_axis, v_axis, is_rotated }
}
pub fn from_image(im: &pdfcraft_engine::PageImage, xf: &PageXform, info: &DocInfo, page: usize) -> Self {
let [_, b, c, ..] = im.matrix;
let is_rot = b.abs() > 1e-4 || c.abs() > 1e-4;
let [a, b, c, d, ..] = im.matrix;
let is_rot = b.abs() > 1e-4 || c.abs() > 1e-4 || a < 0.0 || d < 0.0;
let bounds = image_selectable_bounds(im);
let matrix = if !is_rot && bounds[2] > bounds[0] && bounds[3] > bounds[1] {
[bounds[2] - bounds[0], 0.0, 0.0, bounds[3] - bounds[1], bounds[0], bounds[1]]
@@ -2594,7 +2614,8 @@ fn draw_pages(
if is_image_menu && let Some(sel) = view.image_selection.clone() {
let images = cached_edit_images(view, doc, sel.page).to_vec();
let at = view.comments.context_at.map(|(_, at)| at);
crate::edit_text_ui::image_context_menu(ui, view, &images, sel.page, sel.index, can_paste_image, &mut image_action, at);
let rotation = (view.rotation % 360 + info.pages.get(sel.page).map_or(0, |p| p.rotation % 360)) % 360;
crate::edit_text_ui::image_context_menu(ui, view, &images, sel.page, sel.index, can_paste_image, &mut image_action, at, rotation);
return;
}
if view.comments.selected.is_some() {
@@ -3868,7 +3889,6 @@ fn apply_frame_requests(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui,
}
Some(crate::edit_text_ui::ImageAction::CopyFile(page, index)) => app.copy_page_image_file(page, index),
Some(crate::edit_text_ui::ImageAction::Cut(page, image)) => {
app.views[index].image_selection = None;
app.cut_page_image(page, image);
}
Some(crate::edit_text_ui::ImageAction::Paste(page, at)) => {
+1 -2
View File
@@ -42,8 +42,7 @@ pub(crate) enum Owner {
pub(crate) enum PicturePart {
/// The picture: a click selects it, a drag moves the selected one.
Body,
/// A corner of the selected picture (left top, right top, left bottom, right bottom): a drag
/// resizes it.
/// A resize grip in `ImageObb::handles` order (four corners and four edge midpoints).
Corner(usize),
/// The selected picture's rotate knob: a drag turns it.
Knob,
+35
View File
@@ -2002,3 +2002,38 @@ fn a_moved_picture_is_drawn_from_the_fresh_raster_while_a_tile_is_still_stale()
h.state_mut().views[0].stale_tiles.clear();
h.state_mut().views[0].tiles.clear();
}
#[test]
fn image_handles_account_for_document_and_view_rotation() {
for page_rotation in [0, 90, 180, 270] {
for view_rotation in [0, 90, 180, 270] {
let (width, height) = if page_rotation % 180 == 0 { (300.0, 400.0) } else { (400.0, 300.0) };
let info = DocInfo {
pages: vec![pdfcraft_render::PageInfo {
width,
height,
label: String::new(),
crop: [0.0, 0.0, 300.0, 400.0],
rotation: page_rotation,
}],
..Default::default()
};
let size = if view_rotation % 180 == 0 { vec2(width, height) } else { vec2(height, width) };
let xf = PageXform { rect: Rect::from_min_size(pos2(10.0, 20.0), size), rot: view_rotation, pw: width, ph: height };
let obb = ImageObb::from_matrix([100.0, 0.0, 0.0, 50.0, 50.0, 70.0], &xf, &info, 0);
let total = (page_rotation + view_rotation) % 360;
assert_eq!(obb.is_rotated, total != 0, "page={page_rotation} view={view_rotation}: user-labelled grips need matrix resize when turned");
for (handle, point) in obb.handles() {
let direction = point - obb.screen_center;
let cursor = match handle {
ImageHandle::TopLeft | ImageHandle::TopRight | ImageHandle::BottomLeft | ImageHandle::BottomRight => {
if direction.x * direction.y > 0.0 { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }
}
_ if direction.x.abs() > direction.y.abs() => egui::CursorIcon::ResizeHorizontal,
_ => egui::CursorIcon::ResizeVertical,
};
assert_eq!(obb.handle_cursor(handle), cursor, "page={page_rotation} view={view_rotation} handle={handle:?}");
}
}
}
}
+42 -41
View File
@@ -1,5 +1,5 @@
use super::{ACCENT, Color32, CornerRadius, DocInfo, DocView, Edit, MAX_OUTLINES, PagePointer, PageXform, Pos2, Rect, Stroke, pos2, user_box};
use crate::canvas::{ImageObb, PainterExt};
use crate::canvas::{ImageObb, PainterExt, image_screen_rect};
use crate::image_engine::{CROP_ACCENT, apply_crop, apply_resize, apply_resize_proportional, draw_handle, draw_rule_of_thirds};
use crate::page_cursor::Layer;
use crate::page_router::{Hit, Owner, PicturePart};
@@ -319,7 +319,6 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
let done_resp = ui.interact(done, done_id, egui::Sense::click());
let cancel_resp = ui.interact(cancel, cancel_id, egui::Sense::click());
let reset_resp = reset.map(|r| ui.interact(r, reset_id, egui::Sense::click()));
let t = crate::theme::Tokens::get(ui.ctx());
// Done button (accent colored)
@@ -450,14 +449,16 @@ pub(crate) fn handle_hit(
p: Pos2,
) -> Option<Hit> {
let s = view.image_selection.as_ref().filter(|s| s.page == page && s.crop.is_none())?;
let b = images.get(s.index)?.visible_rect.map(|r| xf.user_rect(info, page, r.map(|v| v as f32)))?;
let image = images.get(s.index)?;
let b = image_screen_rect(image, xf, info, page);
let obb = ImageObb::from_image(image, xf, info, page);
let picture = |part| Owner::Picture { index: s.index, part };
if rotate_knob(b, clip).0.distance(p) < 8.0 {
let knob = if obb.is_rotated { obb.s_rot } else { rotate_knob(b, clip).0 };
if knob.distance(p) < 8.0 {
return Some(Hit::new(picture(PicturePart::Knob), Layer::Handle, egui::CursorIcon::Grab));
}
let corner = [b.left_top(), b.right_top(), b.left_bottom(), b.right_bottom()].iter().position(|c| c.distance(p) < 8.0)?;
let icon = if matches!(corner, 0 | 3) { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw };
Some(Hit::new(picture(PicturePart::Corner(corner)), Layer::Handle, icon))
let (grip, (handle, _)) = obb.handles().into_iter().enumerate().find(|(_, (_, point))| point.distance(p) < 8.0)?;
Some(Hit::new(picture(PicturePart::Corner(grip)), Layer::Handle, obb.handle_cursor(handle)))
}
/// The picture under `p` (its shape, not only its box): a click selects it, a drag moves the
@@ -619,6 +620,7 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
return crop_input(cx, view, i, image);
}
let (knob, stem) = rotate_knob(b, ui.clip_rect());
let shown_knob = if obb.is_rotated { obb.s_rot } else { knob };
if view.image_selection.as_ref().is_some_and(|s| s.drag.is_none()) {
picture_frame(painter, &obb, b, Color32::TRANSPARENT, Stroke::new(1.5, ACCENT));
if obb.is_rotated {
@@ -643,31 +645,15 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
}
}
let rot_id = ui.scope_id().with("img_handle").with(8);
let rot_rect = Rect::from_center_size(if obb.is_rotated { obb.s_rot } else { knob }, egui::vec2(12.0, 12.0));
let rot_rect = Rect::from_center_size(shown_knob, egui::vec2(12.0, 12.0));
let rot_resp = ui.interact(rot_rect, rot_id, egui::Sense::drag());
let rot_drag = allow_interaction && rot_resp.drag_started();
if allow_interaction && let Some(p) = hover_pos {
if obb.s_rot.distance(p) < 8.0 || knob.distance(p) < 8.0 {
if shown_knob.distance(p) < 8.0 {
view.cursor.claim(Layer::Handle, egui::CursorIcon::Grab);
} else if let Some((h, _)) = obb.handles().iter().find(|(_, pos)| pos.distance(p) < 8.0) {
let icon = if obb.is_rotated {
match h {
ImageHandle::TopLeft | ImageHandle::TopRight | ImageHandle::BottomLeft | ImageHandle::BottomRight => {
let v = obb.handle_pos(*h) - obb.screen_center;
if v.x * v.y > 0.0 { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }
}
ImageHandle::TopCenter | ImageHandle::BottomCenter if obb.v_axis.0.abs() > obb.v_axis.1.abs() => {
egui::CursorIcon::ResizeHorizontal
}
ImageHandle::RightCenter | ImageHandle::LeftCenter if obb.u_axis.1.abs() > obb.u_axis.0.abs() => {
egui::CursorIcon::ResizeVertical
}
_ => h.cursor_icon(),
}
} else {
h.cursor_icon()
};
let icon = obb.handle_cursor(*h);
view.cursor.claim(Layer::Handle, icon);
}
}
@@ -681,14 +667,14 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
}
} else if rot_drag {
if let Some(s) = view.image_selection.as_mut() {
let o = origin.unwrap_or(if obb.is_rotated { obb.s_rot } else { knob });
let o = origin.unwrap_or(shown_knob);
s.drag = Some(ImageDrag { start: o, target: DragTarget::Rotate, initial_rect: b, initial_matrix: image.matrix });
s.turning = true;
}
} else if resp.drag_started()
&& let Some(o) = origin
{
if obb.s_rot.distance(o) < 8.0 || knob.distance(o) < 8.0 {
if shown_knob.distance(o) < 8.0 {
if let Some(s) = view.image_selection.as_mut() {
s.drag = Some(ImageDrag { start: o, target: DragTarget::Rotate, initial_rect: b, initial_matrix: image.matrix });
s.turning = true;
@@ -708,7 +694,7 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
if view.image_selection.as_ref().is_some_and(|s| s.turning) {
let shift = ui.input(|i| i.modifiers.shift);
let centre = obb.screen_center;
let rot_knob = if obb.is_rotated { obb.s_rot } else { knob };
let rot_knob = shown_knob;
let drag_start = view.image_selection.as_ref().and_then(|s| s.drag).map_or(rot_knob, |d| d.start);
let degrees = hover_pos.map_or(0.0, |p| turn_degrees_delta(centre, drag_start, p, shift));
let (sin, cos) = degrees.to_radians().sin_cos();
@@ -804,7 +790,7 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
}
let icon = match drag.target {
DragTarget::Handle(handle) => handle.cursor_icon(),
DragTarget::Handle(handle) => obb.handle_cursor(handle),
DragTarget::Move => egui::CursorIcon::Grabbing,
_ => egui::CursorIcon::Move,
};
@@ -864,7 +850,7 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
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 = handle.cursor_icon();
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)
}
@@ -941,14 +927,11 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
painter.rect_stroke(b, CornerRadius::ZERO, Stroke::new(1.5, ACCENT.gamma_multiply(0.7)), egui::StrokeKind::Outside);
}
}
let corner = hit_obb.handles().iter().position(|(_, c)| c.distance(p) < 10.0);
let cursor = match (selected == Some(hit), corner) {
(true, Some(0 | 4)) => egui::CursorIcon::ResizeNwSe,
(true, Some(2 | 6)) => egui::CursorIcon::ResizeNeSw,
(true, Some(1 | 5)) => egui::CursorIcon::ResizeVertical,
(true, Some(3 | 7)) => egui::CursorIcon::ResizeHorizontal,
(true, None) => egui::CursorIcon::Grab,
_ => egui::CursorIcon::PointingHand,
let grip = hit_obb.handles().into_iter().find(|(_, point)| point.distance(p) < 10.0);
let cursor = if selected == Some(hit) {
grip.map_or(egui::CursorIcon::Grab, |(handle, _)| hit_obb.handle_cursor(handle))
} else {
egui::CursorIcon::PointingHand
};
view.cursor.claim(Layer::Text, cursor);
let clicked = match view.router.owner(page) {
@@ -975,6 +958,7 @@ pub(crate) fn image_context_menu(
can_paste: bool,
action: &mut Option<ImageAction>,
at: Option<[f64; 2]>,
rotation: u16,
) {
let Some(img) = images.get(hit) else { return };
use pdfcraft_engine::ImageEdit as E;
@@ -1039,10 +1023,10 @@ pub(crate) fn image_context_menu(
view.pending_edit = edit(E::RemoveCrop);
}
if item(ui, true, tl!("Flip horizontal")) {
view.pending_edit = edit(E::Flip { horizontal: true });
view.pending_edit = edit(E::Flip { horizontal: flip_user_axis(true, rotation) });
}
if item(ui, true, tl!("Flip vertical")) {
view.pending_edit = edit(E::Flip { horizontal: false });
view.pending_edit = edit(E::Flip { horizontal: flip_user_axis(false, rotation) });
}
ui.menu_button(tl!("Arrange image"), |ui| {
@@ -1091,3 +1075,20 @@ pub(crate) fn image_context_menu(
view.pending_edit = edit(E::Delete);
}
}
fn flip_user_axis(horizontal: bool, rotation: u16) -> bool {
horizontal ^ (rotation % 180 == 90)
}
#[cfg(test)]
mod screen_orientation_tests {
use super::*;
#[test]
fn flip_labels_follow_the_screen_axis() {
for rotation in [0, 90, 180, 270] {
assert_eq!(flip_user_axis(true, rotation), rotation % 180 == 0);
assert_eq!(flip_user_axis(false, rotation), rotation % 180 != 0);
}
}
}
+76
View File
@@ -9289,3 +9289,79 @@ fn escape_in_an_open_paragraph_selects_its_box_for_delete() {
assert_eq!(blocks(&h)[0].text, "Page 1");
}
}
#[test]
fn outward_image_grip_drags_grow_on_rotated_pages() {
for rotation in [0, 90, 180, 270] {
for image_turn in [0.0, 25.0] {
for grip in 0..8 {
let mut png = Vec::new();
image::RgbImage::from_pixel(100, 80, image::Rgb([100, 150, 200]))
.write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png)
.unwrap();
let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| {
let mut app = PdfCraftApp::new();
app.set_option("language", "en").unwrap();
app.open_bytes("grips.png", None, png.clone()).unwrap();
app
});
h.run_steps(4);
h.state_mut().apply_edit(pdfcraft_engine::Edit::EditPageImage {
page: 0,
index: 0,
change: pdfcraft_engine::ImageEdit::Move([20.0, 20.0, 80.0, 60.0]),
});
h.state_mut().apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: rotation });
if image_turn != 0.0 {
h.state_mut().apply_edit(pdfcraft_engine::Edit::EditPageImage {
page: 0,
index: 0,
change: pdfcraft_engine::ImageEdit::RotateBy(image_turn),
});
}
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);
let view = &h.state().views[0];
let doc = h.state().session.get(view.id).unwrap();
let before = doc.page_images(0)[0].clone();
let obb = pdfcraft_ui_egui::canvas::ImageObb::from_image(&before, &view.page_xform(0).unwrap(), &doc.info, 0);
let (handle, start) = obb.handles()[grip];
let opposite = obb.handles()[(grip + 4) % 8].0;
let finish = start + (start - obb.screen_center) * 0.15;
assert!(view.viewport_rect().contains(start) && view.viewport_rect().contains(finish), "test grips must be visible");
h.hover_at(start);
h.run_steps(1);
h.drag_at(start);
h.run_steps(1);
let started = h.state().views[0].image_selection.clone();
h.hover_at(finish);
h.run_steps(2);
let moved = h.state().views[0].image_selection.clone();
h.drop_at(finish);
h.run_steps(3);
let view = &h.state().views[0];
let doc = h.state().session.get(view.id).unwrap();
let images = doc.page_images(0);
let after = &images[0];
let after_obb = pdfcraft_ui_egui::canvas::ImageObb::from_image(after, &view.page_xform(0).unwrap(), &doc.info, 0);
assert!(
after_obb.handle_pos(opposite).distance(obb.handle_pos(opposite)) < 1.0,
"fixed grip moved: rotation={rotation} image_turn={image_turn} grip={grip}"
);
assert!(
after_obb.handle_pos(handle).distance(finish) < 1.0,
"dragged grip missed pointer: rotation={rotation} image_turn={image_turn} grip={grip}"
);
let area = |m: [f64; 6]| (m[0] * m[3] - m[1] * m[2]).abs();
assert!(
area(after.matrix) > area(before.matrix) * 1.05,
"outward grip shrank or ignored: rotation={rotation} image_turn={image_turn} grip={grip}: {:?} => {:?}; started={started:?} moved={moved:?}",
before.matrix,
after.matrix
);
}
}
}
}