diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index 2ca3337a3f..e253fd6b59 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -103,10 +103,13 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut }; if let Some(o) = ptr.pressed() { // A press with Shift or ⌘ in Edit text is the objects', whatever lies under it - // (`object_ui::modified_hit`): the page's objects are read for it alone. + // (`object_ui::modified_hit`): the page's objects are read for it alone. Crop mode holds + // it on its page as any other press, so a modified click away applies the crop rather + // than taking the picture into a group (#6422). let adds = tool == QuickTool::EditText && can_modify && xf.rect.contains(o) + && !hits.cropping(view) && crate::object_ui::adds(ptr.modifiers) && !crate::object_ui::menu_open(ui, resp); let objects = adds.then(|| view.objects.inventory(doc, i)); @@ -532,7 +535,7 @@ impl PageHits<'_> { images, clip, } = *self; - // A press with Shift or ⌘ is the objects', crop mode's page included (`object_ui::modified_hit`). + // A press with Shift or ⌘ is the objects' (`object_ui::modified_hit`), off crop mode's page. if let Some(objects) = objects { return crate::object_ui::modified_hit(view, xf, info, page, objects, clip, p).into_iter().collect(); } 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 61b8fa866f..ba3601d0da 100644 --- a/crates/ui-egui/src/tools/edit_text_ui/image.rs +++ b/crates/ui-egui/src/tools/edit_text_ui/image.rs @@ -441,12 +441,25 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e let reset_clicked = gate && reset .is_some_and(|r| reset_resp.as_ref().is_some_and(|res| res.clicked()) || (resp.clicked() && hover_pos.is_some_and(|p| r.contains(p)))); + // A click away from the picture (its shape, uncropped, not its bounds) and its crop box and + // handles applies the crop and lets the picture go, as Done and then that click would + // (#6422). It is pressed away too ([`CropPart::Page`]), so a press on the picture let go just + // past it is not one; a control's click is the control's. + let pressed_away = matches!(press, PagePress::Mine(Owner::Picture { part: PicturePart::Crop(CropPart::Page), .. })); + let away_clicked = gate + && resp.clicked() + && pressed_away + && !(done_clicked || cancel_clicked || reset_clicked) + && hover_pos.is_some_and(|p| matches!(crop_part(xf, info, page, Some(image), mode, p), None | Some(CropPart::Page))); if reset_clicked { mode.keep = if image.has_editor_crop() { image.rect } else { initial_keep }; mode.drag = None; - } else if done_clicked { + } else if done_clicked || away_clicked { apply_crop_mode(view, image); + if away_clicked { + view.image_selection = None; + } } else if cancel_clicked && let Some(s) = view.image_selection.as_mut() { s.crop = None; } @@ -580,28 +593,40 @@ pub(crate) fn handle_hit( pub(crate) fn crop_hit(xf: &PageXform, page: usize, info: &DocInfo, images: &[pdfcraft_engine::PageImage], view: &DocView, p: Pos2) -> Option { let s = view.image_selection.as_ref().filter(|s| s.page == page)?; let mode = s.crop.as_ref()?; - let crop = |part, icon| Some(Hit::new(Owner::Picture { index: s.index, part: PicturePart::Crop(part) }, Layer::Editor, icon)); + let part = crop_part(xf, info, page, images.get(s.index), mode, p)?; + let icon = match part { + CropPart::Page if mode.controls.iter().any(|r| r.contains(p)) => egui::CursorIcon::PointingHand, + CropPart::Grip(g) => GRIPS.get(g).map_or(egui::CursorIcon::Default, |grip| grip.cursor_icon()), + CropPart::Keep => egui::CursorIcon::Grab, + CropPart::Page | CropPart::Cut => egui::CursorIcon::Default, + }; + Some(Hit::new(Owner::Picture { index: s.index, part: PicturePart::Crop(part) }, Layer::Editor, icon)) +} + +/// What of crop mode `mode` on the picture `image` lies under `p` ([`crop_hit`]): a control (as +/// [`CropPart::Page`]), a grip, the box kept, the picture outside it, or the rest of the page; +/// `None` off the page, its grips and its controls. +fn crop_part(xf: &PageXform, info: &DocInfo, page: usize, image: Option<&pdfcraft_engine::PageImage>, mode: &CropMode, p: Pos2) -> Option { if mode.controls.iter().any(|r| r.contains(p)) { - return crop(CropPart::Page, egui::CursorIcon::PointingHand); + return Some(CropPart::Page); } - let image = images.get(s.index); let kept = image.and_then(|image| crop_frames(xf, info, page, image, mode.keep)).map(|(_, k)| k); // The grips reach past the page's edge, where those of a picture filling its page lie half. if let Some(g) = kept.and_then(|k| crop_handles(k).iter().position(|h| h.distance(p) < CROP_GRIP_REACH)) { - return crop(CropPart::Grip(g), GRIPS.get(g).map_or(egui::CursorIcon::Default, |grip| grip.cursor_icon())); + return Some(CropPart::Grip(g)); } if !xf.rect.contains(p) { return None; } if kept.is_some_and(|k| k.contains(p)) { - return crop(CropPart::Keep, egui::CursorIcon::Grab); + return Some(CropPart::Keep); } let on_picture = image.is_some_and(|image| { let shown = ImageObb::from_image(image, xf, info, page); let whole = ImageObb::from_matrix(image.matrix, xf, info, page); quad_contains(shown.corners(), p) || quad_contains(whole.corners(), p) }); - crop(if on_picture { CropPart::Cut } else { CropPart::Page }, egui::CursorIcon::Default) + Some(if on_picture { CropPart::Cut } else { CropPart::Page }) } /// The picture under `p` (its shape, not only its box; for the selected one, its frame, a hole in it diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 500d504403..c5d197f64c 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -9155,6 +9155,129 @@ fn test_rotated_image_crop_secondary_click_in_outer_bounding_box_clears_selectio assert!(h.query_by_label("Rotate view clockwise").is_some()); } +/// A turned picture is its shape, not its bounds: a click in a corner of its bounds, off the +/// picture and its crop box, is a click away that applies the crop (#6422 review). +#[test] +fn a_click_beside_a_turned_picture_in_its_bounds_applies_its_crop() { + let mut png = Vec::new(); + image::RgbImage::from_pixel(80, 80, image::Rgb([200, 40, 40])).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 1400.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("picture.png", None, png.clone()).expect("opens"); + app + }); + h.run_steps(4); + let id = h.state().views[0].id; + let full = h.state().session.get(id).unwrap().page_images(0)[0].rect; + let (qw, qh) = ((full[2] - full[0]) / 4.0, (full[3] - full[1]) / 4.0); + let (mx, my) = ((full[0] + full[2]) / 2.0, (full[1] + full[3]) / 2.0); + let edit = |change| pdfcraft_engine::Edit::EditPageImage { page: 0, index: 0, change }; + h.state_mut().apply_edit(edit(pdfcraft_engine::ImageEdit::Move([mx - qw, my - qh, mx + qw, my + qh]))); + h.run_steps(4); + h.state_mut().apply_edit(edit(pdfcraft_engine::ImageEdit::RotateBy(45.0))); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + let c = h.state().views[0].page_screen_rect(0).expect("on screen").center(); + click_at(&mut h, c); + assert!(h.state().views[0].image_selection.is_some(), "selected"); + h.state_mut().views[0].image_selection.as_mut().unwrap().crop = Some(pdfcraft_ui_egui::CropMode::new([mx - 5.0, my - 5.0, mx + 5.0, my + 5.0])); + h.run_steps(2); + + let img = h.state().session.get(id).unwrap().page_images(0)[0].clone(); + let xf = h.state().views[0].page_xform(0).unwrap(); + let b = pdfcraft_ui_egui::canvas::image_screen_rect(&img, &xf, &h.state().session.get(id).unwrap().info, 0); + // The bounds' top-left corner lies outside the 45° diamond. + click_at(&mut h, b.left_top() + egui::vec2(2.0, 2.0)); + assert!(h.state().views[0].image_selection.is_none(), "a click off the turned picture lets it go"); + assert_ne!(h.state().session.get(id).unwrap().page_images(0)[0].visible_rect, img.visible_rect, "the crop is applied"); +} + +/// A picture alone in the middle of a page, empty page around it (#6422). +fn picture_with_room_around() -> Vec { + use pdfcraft_cos::{Dict, Document, Object, Stream, write_incremental}; + let mut doc = Document::open(std::sync::Arc::new(fixture(1))).unwrap(); + 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, &[60, 110, 200]))); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + let contents = doc.add(Object::Stream(Stream::flate(Dict::new(), b"q 100 0 0 80 50 150 cm /Im0 Do Q"))); + let mut resources = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().unwrap().clone(); + let mut xo = Dict::new(); + xo.set(b"Im0".to_vec(), Object::Ref(image)); + resources.set(b"XObject".to_vec(), Object::Dict(xo)); + doc.update_dict(page.obj, |d| { + d.set(b"Resources".to_vec(), Object::Dict(resources)); + d.set(b"Contents".to_vec(), Object::Ref(contents)); + }) + .unwrap(); + write_incremental(&doc, &Default::default()).unwrap() +} + +/// In crop mode a click on the empty page applies the crop and lets the picture go; a click on the +/// part being cropped away keeps crop mode (#6422). +#[test] +fn a_click_away_from_the_picture_applies_its_crop() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 1100.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("room.pdf", None, picture_with_room_around()).unwrap(); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.state_mut().views[0].fit = pdfcraft_ui_egui::canvas::Fit::Page; + h.run_steps(3); + let r = h.state().views[0].page_screen_rect(0).unwrap(); + let at = |x: f32, y: f32| egui::pos2(r.left() + x / 200.0 * r.width(), r.top() + (300.0 - y) / 300.0 * r.height()); + let doc_id = h.state().views[0].id; + let full = h.state().session.get(doc_id).unwrap().page_images(0)[0].rect; + + click_at(&mut h, at(100.0, 190.0)); + assert!(h.state().views[0].image_selection.as_ref().is_some_and(|s| s.index == 0), "the picture is selected"); + h.state_mut().views[0].image_selection.as_mut().unwrap().crop = Some(pdfcraft_ui_egui::CropMode::new(full)); + h.run_steps(2); + let keep = [full[0], full[1], full[0] + 60.0, full[3]]; + h.state_mut().views[0].image_selection.as_mut().unwrap().crop.as_mut().unwrap().keep = keep; + h.run_steps(2); + + // On the dimmed part of the picture: still cropping, nothing applied. + click_at(&mut h, at(135.0, 190.0)); + assert!(h.state().views[0].image_selection.as_ref().is_some_and(|s| s.crop.is_some()), "a click on the picture keeps crop mode"); + assert_eq!(h.state().session.get(doc_id).unwrap().page_images(0)[0].visible_rect, Some(full), "nothing is applied yet"); + + // On a handle, where it reaches past the picture's frame: still cropping. + let handle = at(50.0, 190.0) - egui::vec2(6.0, 0.0); + click_at(&mut h, handle); + assert!(h.state().views[0].image_selection.as_ref().is_some_and(|s| s.crop.is_some()), "a click on a handle keeps crop mode"); + assert_eq!(h.state().session.get(doc_id).unwrap().page_images(0)[0].visible_rect, Some(full), "nothing is applied yet"); + + // A click pressed on the picture and let go just past its frame: still cropping. + let edge = at(150.0, 190.0); + h.hover_at(edge - egui::vec2(2.0, 0.0)); + h.run_steps(1); + h.drag_at(edge - egui::vec2(2.0, 0.0)); + h.run_steps(1); + h.hover_at(edge + egui::vec2(2.0, 0.0)); + h.run_steps(1); + h.drop_at(edge + egui::vec2(2.0, 0.0)); + h.run_steps(4); + assert!(h.state().views[0].image_selection.as_ref().is_some_and(|s| s.crop.is_some()), "a press on the picture keeps crop mode"); + assert_eq!(h.state().session.get(doc_id).unwrap().page_images(0)[0].visible_rect, Some(full), "nothing is applied yet"); + + // On the empty page: applied, and the picture is let go. + click_at(&mut h, at(100.0, 60.0)); + assert!(h.state().views[0].image_selection.is_none(), "a click away lets the picture go"); + let visible = h.state().session.get(doc_id).unwrap().page_images(0)[0].visible_rect.unwrap(); + assert!(keep.iter().zip(visible).all(|(a, b)| (a - b).abs() < 0.5), "the crop is applied: {visible:?}"); +} + #[test] fn crop_action_pills_interactive_and_apply_cancel_reset() { let mut png = Vec::new(); @@ -11052,3 +11175,42 @@ fn affine_review_cropped_rotation_preview_matches_saved_corners() { ); } } + +#[test] +fn a_modified_click_away_from_a_cropped_picture_applies_its_crop() { + // Shift-, Ctrl- or Command-clicking away is still a click away: it applies the crop and lets + // the picture go, rather than taking the picture into a group and dropping its crop (#6422 + // review). + for mods in [egui::Modifiers::SHIFT, egui::Modifiers::COMMAND] { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 1100.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("room.pdf", None, picture_with_room_around()).unwrap(); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.state_mut().views[0].fit = pdfcraft_ui_egui::canvas::Fit::Page; + h.run_steps(3); + let r = h.state().views[0].page_screen_rect(0).unwrap(); + let at = |x: f32, y: f32| egui::pos2(r.left() + x / 200.0 * r.width(), r.top() + (300.0 - y) / 300.0 * r.height()); + let doc_id = h.state().views[0].id; + let full = h.state().session.get(doc_id).unwrap().page_images(0)[0].rect; + click_at(&mut h, at(100.0, 190.0)); + h.state_mut().views[0].image_selection.as_mut().unwrap().crop = Some(pdfcraft_ui_egui::CropMode::new(full)); + h.run_steps(2); + let keep = [full[0], full[1], full[0] + 60.0, full[3]]; + h.state_mut().views[0].image_selection.as_mut().unwrap().crop.as_mut().unwrap().keep = keep; + h.run_steps(2); + let p = at(100.0, 60.0); + h.hover_at(p); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: mods }); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: mods }); + h.run_steps(4); + assert!(h.state().views[0].image_selection.is_none(), "{mods:?}: the picture is let go"); + let visible = h.state().session.get(doc_id).unwrap().page_images(0)[0].visible_rect.unwrap(); + assert!(keep.iter().zip(visible).all(|(a, b)| (a - b).abs() < 0.5), "{mods:?}: the crop is applied: {visible:?}"); + } +}