From 00442a0e9beeef5f2c9ad035df72140e419dbe70 Mon Sep 17 00:00:00 2001 From: cortenburg28-arch Date: Thu, 15 Oct 2026 02:57:54 -0400 Subject: [PATCH] fix(ui): a shape just drawn resizes from its handles without the Select tool (#2362) Drawing a rectangle or oval selects it and paints its eight handles, but only the Select tool answered them: with the shape tool still on, dragging a handle drew a second shape on top of the first. The handles were a promise the tool did not keep, and anyone marking up a drawing had to switch tools after every shape to fix its size. A shape tool now hands a press on the selected comment's handles to the same select_input the Select tool uses, as the Fill & Sign tools already do for their marks. Only the handles: a drag anywhere else, over the shape included, still draws, so drawing one shape inside another keeps working, and a polygon being clicked out keeps its clicks. The Eraser is left out. The handle test is shared with `grabs` rather than copied. Lines, arrows and polygons are unchanged: they have no resize handles with the Select tool either. Fixes storytold/pdfcraft#2338 Claude-Session: https://claude.ai/code/session_01DsT1y8xBvkXeBxEUQqdDtj Co-authored-by: Claude --- crates/ui-egui/src/comments.rs | 44 +++++++++++++++++++++++++++----- crates/ui-egui/tests/comments.rs | 22 ++++++++++++++++ parity/acrobat-features.toml | 4 +-- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index c911d386b..0789d16ae 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -691,6 +691,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, let pressed_here = origin.is_some_and(|o| page_rect.contains(o)); let over_page = pointer.is_some_and(|p| page_rect.contains(p)); let fill_grab = fill_grabs(ui, cx, view); + let draw_grab = draw_grabs(ui, cx, view); let caret_text = if cx.tool == QuickTool::Comment(CommentTool::Caret) { view.page_text(cx.page) } else { None }; let cv = &mut view.comments; if cv.gesture.is_some() && ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { @@ -708,6 +709,9 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, cv.selected = cx.hit(p).map(|a| (cx.page, a.index)); } match cx.tool { + // The shape just drawn is selected with its handles showing: they resize it without a + // trip to the Select tool (#2338). Anywhere else the tool keeps drawing. + QuickTool::Comment(_) if draw_grab => select_input(ui, resp, cx, view, pointer, origin, pressed_here), QuickTool::Comment(tool) if tool.draws() => { if !cx.allowed { return false; @@ -923,25 +927,53 @@ pub(crate) fn select_grabs(ui: &egui::Ui, cx: &PageCx<'_>, view: &DocView) -> bo cx.tool == QuickTool::Select && grabs(ui, cx, view, |a| !is_markup(&a.subtype)) } +/// With a shape tool (rectangle, oval, line, polygon…), the selected comment's resize handles +/// are picked up as with the Select tool, so the shape just drawn can be resized straight away +/// (#2338). Only the handles: a press anywhere else, the shape itself included, still draws. +pub(crate) fn draw_grabs(ui: &egui::Ui, cx: &PageCx<'_>, view: &DocView) -> bool { + let QuickTool::Comment(tool) = cx.tool else { return false }; + if !(tool.draws() || tool.clicks_points()) || tool == CommentTool::Eraser || cx.hidden { + return false; + } + let cv = &view.comments; + if matches!(cv.gesture, Some(Gesture::Resize { page, .. }) if page == cx.page) { + return true; + } + // A polygon being clicked out keeps its clicks, even near an old shape's handle. + if cv.gesture.is_some() { + return false; + } + press_point(ui, cx).is_some_and(|p| on_handle(cx, view, p, |_| true)) +} + /// Whether the press (or, with no button down, the pointer) is on a `pick` comment or on the /// selected one's resize handles. fn grabs(ui: &egui::Ui, cx: &PageCx<'_>, view: &DocView, pick: impl Fn(&Annotation) -> bool) -> bool { if cx.hidden { return false; } - let cv = &view.comments; - if matches!(cv.gesture, Some(Gesture::Move { page, .. } | Gesture::Resize { page, .. }) if page == cx.page) { + if matches!(view.comments.gesture, Some(Gesture::Move { page, .. } | Gesture::Resize { page, .. }) if page == cx.page) { return true; } + let Some(p) = press_point(ui, cx) else { return false }; + on_handle(cx, view, p, &pick) || cx.hit(p).is_some_and(pick) +} + +/// Where a gesture on this page is decided: the press while the button is down, otherwise the +/// pointer (egui clears the press origin on release). `None` off the page. +fn press_point(ui: &egui::Ui, cx: &PageCx<'_>) -> Option { let (pointer, origin, down) = ui.input(|i| (i.pointer.hover_pos(), i.pointer.press_origin(), i.pointer.any_down())); - let Some(p) = (if down { origin } else { pointer }).filter(|p| cx.xf.rect.contains(*p)) else { return false }; - let on_handle = cv + (if down { origin } else { pointer }).filter(|p| cx.xf.rect.contains(*p)) +} + +/// Whether `p` is on a resize handle of the selected comment, when it is a resizable `pick`. +fn on_handle(cx: &PageCx<'_>, view: &DocView, p: Pos2, pick: impl Fn(&Annotation) -> bool) -> bool { + view.comments .selected .filter(|(page, _)| *page == cx.page) .and_then(|(_, i)| cx.get(i)) .filter(|a| pick(a) && cx.allowed && resizable(a)) - .is_some_and(|a| HANDLES.into_iter().any(|h| handle_pos(cx.screen_rect(a), h).distance(p) <= 7.0)); - on_handle || cx.hit(p).is_some_and(pick) + .is_some_and(|a| HANDLES.into_iter().any(|h| handle_pos(cx.screen_rect(a), h).distance(p) <= 7.0)) } fn clamp_to(r: Rect, p: Pos2) -> Pos2 { diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 46c6c216e..b2d09cad7 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/crates/ui-egui/tests/comments.rs @@ -510,6 +510,28 @@ fn comments_are_selected_moved_and_deleted_with_the_select_tool() { assert_eq!(comments(&h).len(), 1); } +#[test] +fn a_shape_just_drawn_resizes_from_its_handles_without_the_select_tool() { + // #2338: the new shape showed its handles, but only the Select tool answered them. + let square = QuickTool::Comment(pdfcraft_ui_egui::comments::CommentTool::Rectangle); + let mut h = harness(|app| app.set_option("quick", "square").unwrap()); + drag_pt(&mut h, (40.0, 100.0), (140.0, 40.0)); + assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the new rectangle is selected"); + let before = comments(&h)[0].rect; + // Drag its bottom-right handle 40 pt right and 20 pt down. + drag_pt(&mut h, (before[2], before[1]), (before[2] + 40.0, before[1] - 20.0)); + let c = comments(&h); + assert_eq!(c.len(), 1, "the handle resized the rectangle instead of drawing another: {c:?}"); + let r = c[0].rect; + assert!((r[2] - (before[2] + 40.0)).abs() < 3.0 && (r[1] - (before[1] - 20.0)).abs() < 3.0, "resized: {before:?} → {r:?}"); + assert!((r[0] - before[0]).abs() < 1.0 && (r[3] - before[3]).abs() < 1.0, "the opposite corner stays: {before:?} → {r:?}"); + assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().can_undo(), Some("Resize comment")); + assert_eq!(h.state().quick_tool, square, "the rectangle tool stays on"); + // Away from the handles the tool still draws, over the shape too. + drag_pt(&mut h, (60.0, 90.0), (120.0, 60.0)); + assert_eq!(comments(&h).len(), 2, "a drag inside the rectangle draws a second one"); +} + #[test] fn the_panel_posts_comments_and_replies() { let mut h = harness(|app| app.set_option("panel", "comments").unwrap()); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 61a2fb5af..e8b7f64e8 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3745,8 +3745,8 @@ milestone = "M5" status = "shipped" commands = ["comment.square"] tools = ["comment_add", "comment_edit"] -evidence = ["crates/ui-egui/tests/comments.rs::shapes_and_ink_are_drawn_by_dragging", "crates/annot/src/tests.rs::restyle_and_resize_redraw_the_appearance", "crates/annot/src/tests.rs::shapes_can_be_filled_and_unfilled", "crates/ui-egui/tests/comments.rs::shapes_take_a_fill_colour_from_the_quick_bar_and_properties"] -notes = "resize with the 8 handles; fill colour or no fill (#686) from the quick bar, Properties and comment_edit `fill`; fill swatches are the comment palette only (no custom colour picker yet)" +evidence = ["crates/ui-egui/tests/comments.rs::shapes_and_ink_are_drawn_by_dragging", "crates/annot/src/tests.rs::restyle_and_resize_redraw_the_appearance", "crates/annot/src/tests.rs::shapes_can_be_filled_and_unfilled", "crates/ui-egui/tests/comments.rs::shapes_take_a_fill_colour_from_the_quick_bar_and_properties", "crates/ui-egui/tests/comments.rs::a_shape_just_drawn_resizes_from_its_handles_without_the_select_tool"] +notes = "resize with the 8 handles, straight after drawing with the tool still on (#2338); fill colour or no fill (#686) from the quick bar, Properties and comment_edit `fill`; fill swatches are the comment palette only (no custom colour picker yet)" [[feature]] id = "comment.oval"