From ce90c81ffb065e2d4f48bcb0926732c92773e2e1 Mon Sep 17 00:00:00 2001 From: alehel Date: Mon, 2 Nov 2026 13:06:29 +0100 Subject: [PATCH] fix(ui): a drag seen past its page's edge still draws on that page egui reports a drag once the pointer has moved a few points from the press. A press near a page's bottom edge, flicked onto the next page in one frame, is seen as a drag only with the pointer on the next page. Redact (and the area Highlight), the Link tool and Prepare returned before their drag start unless the pointer was on their own page, and the next page is `Elsewhere`, so the press drew nothing anywhere. (On main the next page took it, with the box's start on the page before.) They now start the gesture from where the press began on their own page, as zoom, snapshot, crop and Sign area already did, so the box, link rectangle, marquee, field drawn or field moved belongs to the page the press began on. Tests: one per tool, each seen failing with the fix reverted. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tools/link_ui.rs | 19 ++++--- crates/ui-egui/src/tools/prepare.rs | 73 +++++++++++++-------------- crates/ui-egui/src/tools/redact_ui.rs | 20 ++++---- crates/ui-egui/tests/it/pointer.rs | 58 +++++++++++++++++++++ 4 files changed, 117 insertions(+), 53 deletions(-) diff --git a/crates/ui-egui/src/tools/link_ui.rs b/crates/ui-egui/src/tools/link_ui.rs index d380d1b71b..9cd62d6969 100644 --- a/crates/ui-egui/src/tools/link_ui.rs +++ b/crates/ui-egui/src/tools/link_ui.rs @@ -108,7 +108,19 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, free: bool, owner: Option, free: bool, owner: Option form.get(field).map(|f| (f, widget)), _ => None, }; - if resp.drag_started() && (held || free) { - let origin = ptr.origin.unwrap_or(p); - if let PagePress::Mine(Owner::PrepareCorner { left, top }) = press { + // A drag starts from where its press began on this page: egui may see it only once the pointer + // is past the page's edge. + if allowed + && resp.drag_started() + && (held || free) + && let Some(origin) = ptr.origin.filter(|o| xf.rect.contains(*o)) + { + out.consumed = true; + if tool.is_some() { + prep.grab = Some((page, Grab::Draw(origin), origin)); + } else if let PagePress::Mine(Owner::PrepareCorner { left, top }) = press { prep.grab = Some((page, Grab::Corner(left, top), origin)); - out.consumed = true; } else if let Some((f, wi)) = hit { prep.selected = Some((f.name.clone(), wi)); prep.grab = Some((page, Grab::Move, origin)); - out.consumed = true; } else { prep.marquee_before = Some(prep.selected.iter().chain(prep.also.iter()).cloned().collect()); if !ui.input(|i| i.modifiers.shift || i.modifiers.command) { @@ -432,7 +409,29 @@ pub(crate) fn page_input( prep.also.clear(); } prep.grab = Some((page, Grab::Select(origin), origin)); + } + return out; + } + let Some(p) = pointer.filter(|p| xf.rect.contains(*p)) else { return out }; + if !allowed { + return out; + } + if let Some(tool) = tool { + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); + // A click without a drag places the field at once. + if free && resp.clicked() { out.consumed = true; + if tool == FieldTool::TextTable { + out.table = Some((page, table_rect(xf, Rect::from_min_size(p, egui::Vec2::ZERO)))); + out.placed = true; + return out; + } + let at = to_user(xf, info, page, p); + let (w, h) = tool.default_size(); + let join = prep.selected.as_ref().and_then(|(n, _)| form.iter().find(|f| &f.name == n)); + prep.select_added = Some(form.len()); + view.pending_edit = Some(Edit::AddField { page, rect: [at[0], at[1] - h, at[0] + w, at[1]], kind: tool.new_field(join), name: None }); + out.placed = true; } return out; } diff --git a/crates/ui-egui/src/tools/redact_ui.rs b/crates/ui-egui/src/tools/redact_ui.rs index 0cca746bf8..345e7e5d77 100644 --- a/crates/ui-egui/src/tools/redact_ui.rs +++ b/crates/ui-egui/src/tools/redact_ui.rs @@ -252,17 +252,17 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, over_text: impl Fn(Pos2) -> bo } return true; } - let Some(p) = pointer.filter(|p| xf.rect.contains(*p)) else { return false }; - if !over_text(p) { - view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); + // The tool's own press, of the primary button: one nobody holds, which began on this page off + // its text. egui may see the drag only once the pointer is past the page's edge. + if resp.drag_started_by(egui::PointerButton::Primary) + && free + && let Some(origin) = ptr.origin.filter(|o| xf.rect.contains(*o) && !over_text(*o)) + { + view.redact_drag = Some((page, origin)); + return true; } - // The tool's own press, of the primary button: one nobody holds. - if resp.drag_started_by(egui::PointerButton::Primary) && free { - let origin = ptr.origin.unwrap_or(p); - if !over_text(origin) { - view.redact_drag = Some((page, origin)); - return true; - } + if pointer.is_some_and(|p| xf.rect.contains(p) && !over_text(p)) { + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } false } diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index 0e5a7e3af1..8a020cc4ea 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1298,3 +1298,61 @@ fn a_fields_tooltip_shows_beside_the_open_palette() { rest_at(&mut h, on_field); assert!(h.query_all_by_label("Your name").next().is_some(), "the field's tooltip shows"); } + +/// Two blank 300×400 pages. +fn two_pages_pdf() -> Vec { + pdf(&[ + CATALOG, + "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R >>", + "<< /Type /Page /Parent 2 0 R >>", + ]) +} + +/// Sets a tool up (`tool`), presses above page 1's bottom edge and drags onto page 2 in one frame, +/// so egui sees the drag only once the pointer is on page 2, and returns the app after the release. +fn drag_seen_only_past_page_ones_edge(tool: impl FnOnce(&mut PdfCraftApp)) -> Harness<'static, PdfCraftApp> { + let mut h = open(two_pages_pdf(), "100", false); + tool(h.state_mut()); + h.run_steps(4); + let (one, two) = { + let v = &h.state().views[0]; + (v.page_screen_rect(0).expect("page 1 on screen"), v.page_screen_rect(1).expect("page 2 on screen")) + }; + let press = egui::pos2(one.min.x + 50.0, one.max.y - 30.0); + let release = egui::pos2(one.min.x + 150.0, two.min.y + 20.0); + h.hover_at(press); + h.run_steps(1); + h.drag_at(press); + h.run_steps(1); + h.hover_at(release); + h.run_steps(1); + h.drop_at(release); + h.run_steps(4); + h +} + +// A drag seen only past its page's edge still draws on the page its press began on, as zoom, crop +// and Sign area do. With these three it used to draw nothing anywhere. + +#[test] +fn a_redaction_box_dragged_off_its_page_is_drawn_on_it() { + let h = drag_seen_only_past_page_ones_edge(|app| app.quick_tool = QuickTool::Redact); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert!(doc.info.annotations.iter().any(|a| a.subtype == "Redact" && a.page == 0), "a redaction mark on page 1"); +} + +#[test] +fn a_link_rectangle_dragged_off_its_page_is_drawn_on_it() { + let h = drag_seen_only_past_page_ones_edge(|app| app.quick_tool = QuickTool::Link); + assert_eq!(h.state().link_draft.as_ref().map(|d| (d.page, d.index)), Some((0, None)), "a new link on page 1"); +} + +#[test] +fn a_field_dragged_off_its_page_is_drawn_on_it() { + let h = drag_seen_only_past_page_ones_edge(|app| assert!(app.execute("form.add.text"), "Prepare a form, with the text field tool")); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert!(doc.form.iter().any(|f| f.widgets.iter().any(|w| w.page == Some(0))), "a field on page 1"); +}