From 85fbcc436bbf3d5a329b9094c9815d237ff4836d Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 08:06:30 +0100 Subject: [PATCH] refactor(ui): the text layer, image pick and link following are told the press The text layer asked the router whether the press's owner kept its drag, link following asked it which link held the press, and the Link tool started a new link from a press any link held, since the router kept a declined drag's capture. - `PageRouter::drag_started`: a drag the owner declines (text markup; with the Link tool, a link) is nobody's once it starts, on the page it began on. The Link tool draws a new link only from a drag nobody holds. - `PageInput` carries the press (`PagePress`) and whether its drag is the page's text's (`text_drag`): the text layer and link following read them, and `PageRouter::owner` goes. Only the page view reads the router now. - Image pick acts on a press nobody holds: a link's click follows the link only, and picks no picture under it (the user's ruling). - The I-beam drops over a link only where a press would go to it, the top hit under the pointer (the user's ruling): the tools that select a link's text show it there. - `PageText::glyph_at` is the one test for the text under a point, for the text layer and Redact's boxes. Co-Authored-By: Claude Opus 5.5 --- crates/render/src/text.rs | 5 ++ crates/render/src/text/tests.rs | 15 ++++++ crates/ui-egui/src/canvas.rs | 9 +++- crates/ui-egui/src/canvas/page_input.rs | 49 +++++++++++++------ .../ui-egui/src/canvas/page_pointer/tests.rs | 24 ++++----- crates/ui-egui/src/canvas/page_router.rs | 43 +++++++++++----- crates/ui-egui/src/tools/link_ui.rs | 11 +++-- crates/ui-egui/tests/it/pointer.rs | 45 +++++++++++++++++ 8 files changed, 156 insertions(+), 45 deletions(-) diff --git a/crates/render/src/text.rs b/crates/render/src/text.rs index 50257d8248..12a1e3aecd 100644 --- a/crates/render/src/text.rs +++ b/crates/render/src/text.rs @@ -175,6 +175,11 @@ impl PageText { .collect() } + /// Index of the first glyph whose box holds a view-space point: the text under the pointer. + pub fn glyph_at(&self, x: f32, y: f32) -> Option { + self.glyphs.iter().position(|g| x >= g.rect[0] && x <= g.rect[2] && y >= g.rect[1] && y <= g.rect[3]) + } + /// Index of the glyph nearest to a view-space point (for selection anchors). /// /// A glyph whose drawn em box (`quad`) holds the point wins. Skewed or turned text has bounding diff --git a/crates/render/src/text/tests.rs b/crates/render/src/text/tests.rs index a1af42a8d0..8776dfeb72 100644 --- a/crates/render/src/text/tests.rs +++ b/crates/render/src/text/tests.rs @@ -1965,3 +1965,18 @@ fn text_outside_the_crop_box_is_clipped() { assert!(text.is_empty(), "expected no glyphs on {crop:?}, got {:?}", text.plain_text()); } } + +/// The glyph under a point is the first whose box holds it, edges included; between glyphs, none. +#[test] +fn the_glyph_at_a_point_is_the_first_whose_box_holds_it() { + let text = PageText { + glyphs: vec![em_box("a", 0.0, 0.0, 10.0, 10.0), em_box("b", 8.0, 0.0, 20.0, 10.0)], + line_of: vec![0, 0], + space_before: vec![false, false], + }; + assert_eq!(text.glyph_at(5.0, 5.0), Some(0)); + assert_eq!(text.glyph_at(9.0, 5.0), Some(0), "the first of overlapping boxes"); + assert_eq!(text.glyph_at(20.0, 10.0), Some(1), "an edge is in"); + assert_eq!(text.glyph_at(25.0, 5.0), None); + assert_eq!(PageText::default().glyph_at(0.0, 0.0), None); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 7e77d473be..14f9a92545 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1546,7 +1546,14 @@ fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i if let Some(p) = frame.right_click_on(xf.rect) { comments::anchor(&pcx, p, view); } - PageInput { consumed: true, area_tool: false, hover: None, released_over: false } + PageInput { + press: crate::page_router::PagePress::Elsewhere, + text_drag: false, + consumed: true, + area_tool: false, + hover: None, + released_over: false, + } } else { page_tool_input(ui, view, frame, &slot) }; diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index 4750004e35..36f6f9d3f9 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -12,7 +12,13 @@ use crate::{QuickTool, comments, icons}; /// What the page's tools did with the pointer this frame. pub(super) struct PageInput { - /// A tool, a field or a comment took the pointer: text selection must leave it alone. + /// What the press the pages hold is to this page. + pub(super) press: PagePress, + /// The press's drag is the page's text's: nobody holds it, or its owner lets its drag go (text + /// markup: a click selects it, a drag selects the text under it). + pub(super) text_drag: bool, + /// A tool, a field or a comment took the pointer this frame (its own press, or a gesture of the + /// active tool's): text selection and image pick leave it alone. pub(super) consumed: bool, /// Redact or Highlight draws boxes off text. pub(super) area_tool: bool, @@ -108,6 +114,10 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut view.router.press(i, at, tool); } } + // A drag that the press's owner declines is nobody's once it starts. + if resp.drag_started_by(egui::PointerButton::Primary) { + view.router.drag_started(); + } // What this page has to do with the press: its owner's, the active tool's, or another page's. let press = view.router.press_for(i); // What lies under the pointer before the tools act, on the frames that need it: for a click's @@ -157,7 +167,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut let text = view.page_text(i); let over_text = |p: Pos2| { let (vx, vy) = xf.screen_to_view(p); - text.as_ref().is_some_and(|t| t.glyphs.iter().any(|g| vx >= g.rect[0] && vx <= g.rect[2] && vy >= g.rect[1] && vy <= g.rect[3])) + text.as_ref().is_some_and(|t| t.glyph_at(vx, vy).is_some()) }; crate::redact_ui::page_input(cx, over_text, press == PagePress::Free, view) }; @@ -216,7 +226,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut let on_comment = comments::page_input(ui, resp, &pcx, press, taken, view); let consumed = taken || on_comment; let hover = pointer.and_then(|p| claim_under(&hits, view, p, frame.env.unobstructed)); - PageInput { consumed, area_tool, hover, released_over } + PageInput { press, text_drag: !view.router.keeps_drag(), consumed, area_tool, hover, released_over } } /// What lies under the pointer at `p`, after the tools act: its top, what a press there would take, @@ -529,7 +539,7 @@ impl PageHits<'_> { /// Text layer: find matches, selection, I-beam and drag-to-select. pub(super) fn text_layer(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, slot: &PageSlot, input: &PageInput) { - let PageEnv { info, selects_text, tool, .. } = frame.env; + let PageEnv { selects_text, tool, .. } = frame.env; let PageSlot { i, r, xf } = *slot; let PageFrame { resp, ptr, pointer, .. } = *frame; if let Some(text) = view.texts.get(&i).cloned() { @@ -538,7 +548,11 @@ pub(super) fn text_layer(ui: &mut egui::Ui, view: &mut DocView, frame: &mut Page && !input.consumed && let Some(p) = pointer.filter(|p| r.contains(*p)) { - text_pointer_input(ui, view, &text, TextPointer { page: i, xf, rect: r, info, tool, resp, ptr: ptr.clone(), pointer: p }); + // The I-beam drops where a press would go to a link (ruling E: the tools that select a + // link's text show it there). + let over_link = input.hover.as_ref().is_some_and(|h| matches!(h.owner, Owner::Link { .. })); + let pointer = TextPointer { page: i, xf, rect: r, tool, resp, ptr: ptr.clone(), pointer: p, over_link, text_drag: input.text_drag }; + text_pointer_input(ui, view, &text, pointer); } } } @@ -583,11 +597,14 @@ struct TextPointer<'a> { page: usize, xf: PageXform, rect: Rect, - info: &'a DocInfo, tool: QuickTool, resp: &'a egui::Response, ptr: PagePointer, pointer: Pos2, + /// A press under the pointer would go to a link. + over_link: bool, + /// The press's drag is the page's text's ([`PageInput::text_drag`]). + text_drag: bool, } struct TextGesture<'a> { @@ -602,16 +619,16 @@ struct TextGesture<'a> { vy: f32, over_text: bool, over_link: bool, + text_drag: bool, column: bool, } fn text_pointer_input(ui: &mut egui::Ui, view: &mut DocView, text: &PageText, pointer: TextPointer<'_>) { - let TextPointer { page, xf, rect, info, tool, resp, ptr, pointer: screen_pointer } = pointer; + let TextPointer { page, xf, rect, tool, resp, ptr, pointer: screen_pointer, over_link, text_drag } = pointer; let (vx, vy) = xf.screen_to_view(screen_pointer); - let over_text = text.glyphs.iter().any(|glyph| vx >= glyph.rect[0] && vx <= glyph.rect[2] && vy >= glyph.rect[1] && vy <= glyph.rect[3]); - let over_link = info.links.iter().any(|link| link.page == page && xf.user_rect(info, page, link.rect).contains(screen_pointer)); + let over_text = text.glyph_at(vx, vy).is_some(); let column = tool == QuickTool::ColumnSelect || ui.input(|input| input.modifiers.alt); - let gesture = TextGesture { page, xf, rect, tool, ptr, resp, pointer: screen_pointer, vx, vy, over_text, over_link, column }; + let gesture = TextGesture { page, xf, rect, tool, ptr, resp, pointer: screen_pointer, vx, vy, over_text, over_link, text_drag, column }; update_text_cursor(view, &gesture); update_selection_on_press(ui, view, text, &gesture); update_dragged_selection(view, text, &gesture); @@ -631,7 +648,7 @@ fn update_selection_on_press(ui: &egui::Ui, view: &mut DocView, text: &PageText, // A press held by what keeps its drag (a link, a comment that moves) selects no text; one on // text markup lets its drag select the text underneath. let press_here = gesture.ptr.origin.is_some_and(|origin| gesture.rect.contains(origin)); - if gesture.ptr.pressed().is_some() && press_here && !view.router.keeps_drag() { + if gesture.ptr.pressed().is_some() && press_here && gesture.text_drag { let now = ui.input(|input| input.time); let (max_delay, max_dist) = ui.ctx().options(|options| (options.input_options.max_double_click_delay, options.input_options.max_click_dist)); let consecutive = now - view.last_press_time <= max_delay && view.last_press_pos.is_some_and(|pos| pos.distance(gesture.pointer) <= max_dist); @@ -646,7 +663,7 @@ fn update_selection_on_press(ui: &egui::Ui, view: &mut DocView, text: &PageText, view.selection = Some(Selection::run(gesture.page, first, last)); } } - if gesture.resp.drag_started() && press_here && !view.router.keeps_drag() { + if gesture.resp.drag_started() && press_here && gesture.text_drag { view.actions_dismissed = None; view.selection_drag = true; let origin = gesture.ptr.origin.unwrap_or(gesture.pointer); @@ -718,7 +735,7 @@ fn finish_text_pointer_gesture(ui: &egui::Ui, view: &mut DocView, text: &PageTex && let Some((first, last)) = text.nearest(gesture.vx, gesture.vy).and_then(|anchor| click_unit(text, clicks, anchor)) { view.selection = Some(Selection::run(gesture.page, first, last)); - } else if gesture.resp.clicked() && view.clicks < 2 && !view.router.keeps_drag() { + } else if gesture.resp.clicked() && view.clicks < 2 && gesture.text_drag { let shift = ui.input(|input| input.modifiers.shift); match ( view.selection.as_mut().filter(|selection| shift && selection.page == gesture.page).map(|selection| &mut selection.span), @@ -758,7 +775,9 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P let painter = ui.painter(); let tool_cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info }; let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark }; - if tool == QuickTool::Select && !input.consumed { + // A press nobody holds picks the picture under it: a link's click follows the link only (ruling + // 5). + if tool == QuickTool::Select && input.press == PagePress::Free && !input.consumed { crate::image_pick::page_input(resp, &xf, i, info, doc, pointer, view); } crate::image_pick::drop_stale(doc, view); @@ -815,7 +834,7 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P && resp.clicked() && tool == QuickTool::Select && !input.consumed - && let Some(Owner::Link { index }) = view.router.owner(i) + && let PagePress::Mine(Owner::Link { index }) = input.press && input.released_over && let Some(l) = info.links.iter().find(|l| l.page == i && l.index == index) { diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index de4e0fb9ab..a3a3d8e882 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -483,20 +483,22 @@ fn page_modules_claim_the_cursor_and_never_set_it() { } /// What decides, keeps or drops the press the pages hold. -const ROUTER_WRITES: [&str; 8] = [ +const ROUTER_WRITES: [&str; 10] = [ "router.begin(", "router.press(", + "router.drag_started(", "router.settle(", "router.cancel(", "PageRouter::begin(", "PageRouter::press(", + "PageRouter::drag_started(", "PageRouter::settle(", "PageRouter::cancel(", ]; /// The functions that decide, keep or drop the press the pages hold: (file, function, why). const ROUTER_WRITERS: [(&str, &str, &str); 3] = [ - ("canvas/page_input.rs", "page_tool_input", "the hits at the press point decide its owner, once per press"), + ("canvas/page_input.rs", "page_tool_input", "the hits at the press point decide its owner, once per press, and a declined drag lets it go"), ("canvas.rs", "draw_pages", "the press is over once the button is up and its release seen, and a new one drops the last owner"), ("canvas.rs", "cancel_pointer_gestures", "every pointer gesture's cancel: Escape, automation's cancel"), ]; @@ -566,18 +568,16 @@ const COMMENT_POINT_TESTERS: [(&str, &str, &str); 3] = [ ]; /// What reads who holds the press the pages hold. -const ROUTER_READS: [&str; 6] = - ["router.press_for(", "router.owner(", "router.keeps_drag(", "PageRouter::press_for(", "PageRouter::owner(", "PageRouter::keeps_drag("]; +const ROUTER_READS: [&str; 4] = ["router.press_for(", "router.keeps_drag(", "PageRouter::press_for(", "PageRouter::keeps_drag("]; /// The functions that read who holds the press: (file, function, why). The page view asks once per -/// page and tells each module what the press is to its page (`PagePress`); the text layer and link -/// following still ask, until they are told too (PR 4b), and an unused entry fails the test. -const ROUTER_READERS: [(&str, &str, &str); 4] = [ - ("canvas/page_input.rs", "page_tool_input", "the page view: what the press is to each page (`press_for`)"), - ("canvas/page_input.rs", "update_selection_on_press", "the text layer selects no text under a press held by what keeps its drag (4b)"), - ("canvas/page_input.rs", "finish_text_pointer_gesture", "a click held by what keeps its drag leaves the text selection alone (4b)"), - ("canvas/page_input.rs", "page_overlays", "a click follows the link that holds its press (4b)"), -]; +/// page and tells each module what the press is to its page (`PagePress`); an unused entry fails +/// the test. +const ROUTER_READERS: [(&str, &str, &str); 1] = [( + "canvas/page_input.rs", + "page_tool_input", + "the page view: what the press is to each page (`press_for`), and whether its drag is the text's (`keeps_drag`)", +)]; /// What ignores the press the pages hold until the button is up. const PRESS_IGNORES: [&str; 1] = ["press_cancelled = true"]; diff --git a/crates/ui-egui/src/canvas/page_router.rs b/crates/ui-egui/src/canvas/page_router.rs index 1302901136..5f4a08a56d 100644 --- a/crates/ui-egui/src/canvas/page_router.rs +++ b/crates/ui-egui/src/canvas/page_router.rs @@ -210,17 +210,21 @@ impl PageRouter { } } - /// Who holds the press on `page`. - pub(crate) fn owner(&self, page: usize) -> Option { - self.press_for(page).owner() - } - /// Whether the press is held by someone who keeps its drag: then the page's text is not /// selected by it. pub(crate) fn keeps_drag(&self) -> bool { self.capture.as_ref().is_some_and(|c| !c.declines_drag) } + /// The press turned into a drag: an owner that declines its drag lets it go, and from then on + /// the press is nobody's (a drag across a highlight selects the text under it; with the Link + /// tool, a drag from a link draws a new one). Its click stayed the owner's. + pub(crate) fn drag_started(&mut self) { + if self.capture.as_ref().is_some_and(|c| c.declines_drag) { + self.capture = None; + } + } + /// The press is over once the button is up and its release has been seen. pub(crate) fn settle(&mut self, any_down: bool, edges: bool) { if !any_down && !edges { @@ -260,16 +264,16 @@ mod tests { let mut r = PageRouter::default(); r.begin(); r.press(2, [hit(Layer::Comment, 7)], QuickTool::Select); - assert_eq!(r.owner(2), Some(hit(Layer::Comment, 7).owner)); - assert_eq!(r.owner(1), None, "only on its page"); + assert_eq!(r.press_for(2).owner(), Some(hit(Layer::Comment, 7).owner)); + assert_eq!(r.press_for(1).owner(), None, "only on its page"); assert!(r.keeps_drag()); r.settle(false, true); - assert!(r.owner(2).is_some(), "the release frame still belongs to the press"); + assert!(r.press_for(2).owner().is_some(), "the release frame still belongs to the press"); r.settle(false, false); - assert_eq!(r.owner(2), None); + assert_eq!(r.press_for(2).owner(), None); r.begin(); r.press(0, [], QuickTool::Select); - assert_eq!(r.owner(0), None, "a press nobody claims is held by nobody"); + assert_eq!(r.press_for(0).owner(), None, "a press nobody claims is held by nobody"); assert!(!r.keeps_drag()); } @@ -311,8 +315,23 @@ mod tests { let mut r = PageRouter::default(); r.begin(); r.press(0, [Hit { declines_drag: true, ..hit(Layer::Comment, 0) }], QuickTool::Select); - assert!(r.owner(0).is_some() && !r.keeps_drag()); + assert!(r.press_for(0).owner().is_some() && !r.keeps_drag()); r.cancel(); - assert_eq!(r.owner(0), None); + assert_eq!(r.press_for(0).owner(), None); + } + + /// A drag the owner declines is nobody's once it starts, on the page it began on; a drag the + /// owner keeps stays its own. + #[test] + fn a_declined_drag_is_nobodys_once_it_starts() { + let mut r = PageRouter::default(); + r.begin(); + r.press(1, [Hit { declines_drag: true, ..hit(Layer::Comment, 0) }], QuickTool::Select); + r.drag_started(); + assert_eq!((r.press_for(1), r.press_for(0)), (PagePress::Free, PagePress::Elsewhere)); + r.begin(); + r.press(1, [hit(Layer::Comment, 2)], QuickTool::Select); + r.drag_started(); + assert_eq!(r.press_for(1), PagePress::Mine(hit(Layer::Comment, 2).owner)); } } diff --git a/crates/ui-egui/src/tools/link_ui.rs b/crates/ui-egui/src/tools/link_ui.rs index 63db81b9f3..e30614900d 100644 --- a/crates/ui-egui/src/tools/link_ui.rs +++ b/crates/ui-egui/src/tools/link_ui.rs @@ -95,7 +95,8 @@ fn target_label(info: &DocInfo, target: &LinkTarget) -> String { /// Link tool input on one page: `free` when nobody holds the press and it began on this page, /// `owner` the link that holds it (by its `/Annots` index), and `context` the link a right-click is -/// on. A drag from either draws a new link. Returns `true` when the gesture belongs to the tool. +/// on. A drag nobody holds draws a new link: a link lets its drag go. Returns `true` when the +/// gesture belongs to the tool. pub(crate) fn page_input(cx: PageToolContext<'_>, free: bool, owner: Option, context: Option, view: &mut DocView) -> bool { let PageToolContext { ui, resp, xf, ptr, page, info, .. } = cx; let pointer = ptr.pos; @@ -114,11 +115,11 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, free: bool, owner: Option Vec { ]) } +/// A 300×400 page with a picture at x 50..250, y 250..350 and a web link over its middle, at +/// x 100..200, y 280..320. +fn linked_picture_pdf() -> Vec { + let picture = + "<< /Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 1 >>\nstream\nA\nendstream"; + let content = stream("q 200 0 0 100 50 250 cm /Im0 Do Q"); + pdf(&[ + CATALOG, + PAGES, + "<< /Type /Page /Parent 2 0 R /Contents 5 0 R /Resources << /XObject << /Im0 6 0 R >> >> /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Link /Rect [100 280 200 320] /Border [0 0 0] /A << /S /URI /URI (https://example.org/) >> >>", + content.as_str(), + picture, + ]) +} + /// A 300×400 page with a line of text at y≈300 and a rectangle comment over it. fn comment_on_text_pdf() -> Vec { let text = stream(LINE); @@ -344,6 +360,35 @@ fn a_drag_from_text_onto_a_link_selects_the_text() { assert!(selected.contains("example.org"), "the drag selected across the link: {selected:?}"); } +/// With the Select tool a click on a link over a picture follows the link and picks no picture; one +/// on the picture beside the link picks it. The link's click used to pick the picture under it too. +#[test] +fn a_click_on_a_link_over_a_picture_follows_it_and_picks_no_picture() { + let mut h = open(linked_picture_pdf(), "150", false); + let beside = at(&h, 70.0, 300.0); + mouse_click(&mut h, beside); + assert!(h.state().views[0].picked_image.is_some(), "the picture beside the link is picked"); + h.state_mut().views[0].picked_image = None; + let on_link = at(&h, 150.0, 300.0); + mouse_click(&mut h, on_link); + assert_eq!(pending_url(&h).as_deref(), Some("https://example.org/"), "the link is followed"); + assert!(h.state().views[0].picked_image.is_none(), "and no picture is picked"); +} + +/// The I-beam drops over a link only where a press would go to it: with the Select tool the link's +/// hand shows there, and with the highlighter, whose drag selects the link's text, the I-beam. The +/// highlighter's I-beam used to drop over every link. +#[test] +fn the_i_beam_drops_over_a_link_only_where_the_press_goes_to_it() { + let mut h = open(linked_text_pdf(), "150", false); + let (plain, linked) = (at(&h, 200.0, 303.0), at(&h, 100.0, 303.0)); + assert_eq!(wait_for_cursor(&mut h, plain, |c| c == egui::CursorIcon::Text), egui::CursorIcon::Text, "the page's text has arrived"); + assert_eq!(cursor_at(&mut h, linked), egui::CursorIcon::PointingHand, "the Select tool follows the link"); + h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Highlight); + h.run_steps(2); + assert_eq!(cursor_at(&mut h, linked), egui::CursorIcon::Text, "the highlighter selects its text"); +} + /// Over two comments the tip names the one drawn on top (#3620), and a click should select that /// one too. The renderer paints in `/Annots` order, but the hit test takes the last comment in /// inspection's order, which is by top edge, and selects the one underneath.