From 24eb327a809c177f183bd697068fb4d90121c89d Mon Sep 17 00:00:00 2001 From: alehel Date: Thu, 22 Oct 2026 17:57:29 +0200 Subject: [PATCH 1/4] fix(ui): the page sets its cursor once, from the top claim the tool reaches Every page module set egui's cursor itself and the last write of a frame won, so the order of document_area's per-page chain decided the cursor, not what a press there does. The link hover loop ran last and showed the hand over any link for every tool but Fill & Sign, though only the Select tool follows links: a click with the stamp tool placed a stamp, a drag with the rectangle tool drew, and in Edit text a drag from linked words moved the paragraph, all under the hand. The text layer ran after the comments and showed the I-beam over a movable comment lying on text, which a drag there moves. Page modules now claim the cursor instead (src/page_cursor.rs): a layer, an icon and optionally a tip. The layers are the review's priority table, Editor > Handle > Placed > Field > Comment > Link > Text > tool area, with Drag on top for a drag in progress, whose owner names the cursor until the release. document_area resolves the claims once per frame, after the pages, and calls set_cursor_icon once: the claim on the highest layer the active tool reaches wins. Links count only for the Select tool (follow) and the Link tool (edit), comments only for the Select tool; the other layers are claimed only by the modules of the tools that act on them. The claims go into a per-frame collector on DocView, which every page module already takes, so the calls in the dispatch block keep their arguments (camera, content and redact lose a `ui` they took only for the cursor). A link's tip goes with its claim, so it shows only where the link wins: no longer with a stamp, drawing or other tool that does not follow links. A comment's tip is a description instead (comments::describe), shown with any tool beside that tool's cursor unless the winning claim has a tip of its own: Acrobat shows a comment's tooltip with every tool (observed with Select, Hand, Add comment, Highlight, Rectangle and Stamp). It keeps the old exceptions (Fill & Sign, freehand drawing for #429, a comment gesture in progress) and comments. A signed field's validation badge (#1895) says its verdict the same way, with any tool, ranked below a comment's description as it was below the comment's tip; the topmost of overlapping badges still speaks. FieldPointer::Over existed only to keep the text layer from overwriting a field's cursor (#2099); the Field layer above Text does that now, so forms_ui::page_input returns whether a field owns the press. Who acts on a press is unchanged (step 3 of the router). The picture's rotate knob claims too: Grab as a handle, Grabbing as a drag while it turns, which dragging_the_rotate_knob_turns_a_page_image now checks. The contains_pointer checks #2755 put before the cursor in content_ui, edit_text_ui, forms_ui and the text layer go: the pages' pointer has no position where the mode bar or a side panel covers a page, so nothing is claimed there, and #2755's tests pass unchanged. During a press the pages hold, those four now keep their cursor over the chrome too, as the other page modules and a text selection being dragged already did. Un-ignores the four step-2 probes in tests/it/pointer.rs, each seen failing on the previous commit at its cursor assertion, after its press assertions passed. A new test, also seen failing there: a link's tip shows only with its hand. Another pins a comment's tip with every tool beside its cursor, and its exceptions; it was seen failing with comment tips tied to the cursor's winner instead, which shows them with the Select tool only. In tests/it/signing.rs, a signed field's badge says its verdict with the Select tool and with a stamp; it fails when the badge is left out of the resolver, as resolving after the page loop would otherwise do. A new ignored probe pins a step-3 bug: over stacked comments the tip names the one drawn on top, but a click selects the one underneath, because PageCx::hit takes the last in inspection's top-edge order. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/camera_ui.rs | 9 +- crates/ui-egui/src/canvas.rs | 139 +++--------- crates/ui-egui/src/comments.rs | 126 +++++++++-- crates/ui-egui/src/content_ui.rs | 17 +- crates/ui-egui/src/crop.rs | 3 +- crates/ui-egui/src/edit_text_ui.rs | 59 ++--- crates/ui-egui/src/fill_sign.rs | 5 +- crates/ui-egui/src/forms_ui.rs | 41 ++-- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/link_ui.rs | 6 +- crates/ui-egui/src/measure_ui.rs | 4 +- crates/ui-egui/src/object_ui.rs | 5 +- crates/ui-egui/src/page_cursor.rs | 332 +++++++++++++++++++++++++++++ crates/ui-egui/src/prepare.rs | 7 +- crates/ui-egui/src/redact_ui.rs | 4 +- crates/ui-egui/src/sign_ui.rs | 3 +- crates/ui-egui/src/zoom_snap.rs | 3 +- crates/ui-egui/tests/it/editing.rs | 2 + crates/ui-egui/tests/it/pointer.rs | 93 +++++++- crates/ui-egui/tests/it/signing.rs | 56 ++++- parity/acrobat-features.toml | 4 +- 21 files changed, 693 insertions(+), 226 deletions(-) create mode 100644 crates/ui-egui/src/page_cursor.rs diff --git a/crates/ui-egui/src/camera_ui.rs b/crates/ui-egui/src/camera_ui.rs index d470378070..abaad1ed5c 100644 --- a/crates/ui-egui/src/camera_ui.rs +++ b/crates/ui-egui/src/camera_ui.rs @@ -5,6 +5,7 @@ use egui::{Align2, Color32, CornerRadius, Pos2, Rect, Shape, Stroke, vec2}; use crate::canvas::{DocView, PageXform, ViewAction}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::Tokens; pub use pdfcraft_engine::camera_clean::Look; @@ -59,11 +60,11 @@ fn screen(xf: &PageXform, c: [f32; 2]) -> Pos2 { } /// Corner dragging on one page. A press on a page without the quadrilateral moves it there. -pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, ptr: &PagePointer, page: usize, view: &mut DocView, allowed: bool) { +pub(crate) fn page_input(resp: &egui::Response, xf: &PageXform, ptr: &PagePointer, page: usize, view: &mut DocView, allowed: bool) { let (pointer, origin) = (ptr.pos, ptr.origin); if !allowed { if pointer.is_some_and(|p| xf.rect.contains(p)) { - ui.ctx().set_cursor_icon(egui::CursorIcon::NotAllowed); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::NotAllowed); } return; } @@ -93,7 +94,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, p let Some(q) = view.camera.as_mut().filter(|q| q.page == page) else { return }; let near = pointer.is_some_and(|p| q.corners.iter().any(|&c| screen(xf, c).distance(p) <= GRAB)); if let Some(i) = q.grab { - ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + view.cursor.claim(Layer::Drag, egui::CursorIcon::Grabbing); if let Some(p) = pointer { let (x, y) = xf.screen_to_view(p); if let Some(c) = q.corners.get_mut(i) { @@ -104,7 +105,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, p q.grab = None; } } else if near { - ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + view.cursor.claim(Layer::Handle, egui::CursorIcon::Grab); } } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 4f00aa6bec..1b9dce8285 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -12,6 +12,7 @@ use egui::{Align2, Color32, CornerRadius, Pos2, Rect, Sense, Stroke, TextureHand use pdfcraft_engine::{DocId, Edit}; use pdfcraft_render::{DestView, DocInfo, LayerOp, LinkTarget, PRINT_TAG, PageText, RenderPool, RenderRequest, RequestKind, Tile, device_pixels}; +use crate::page_cursor::{Layer, Subject}; use crate::page_pointer::PagePointer; use crate::theme::{self, Tokens}; use crate::{PdfCraftApp, QuickTool, RightPanel, comments, icons, widgets}; @@ -641,6 +642,8 @@ pub struct DocView { /// A queued Fill & Sign signature: select it after its edit succeeds, then leave placement. pub(crate) fill_signature_page: Option, pub(crate) signature_drag: crate::signature_drag::SignatureDrag, + /// What the page modules claim for the cursor this frame; `document_area` sets it once. + pub(crate) cursor: crate::page_cursor::PageCursor, /// A non-edit action requested by the organize toolbar, handled by the app. pub pending_action: Option, /// The grid gap the next inserted files go to (set by a "+" between pages); otherwise they @@ -947,6 +950,7 @@ impl DocView { fill_text: None, fill_signature_page: None, signature_drag: Default::default(), + cursor: Default::default(), } } @@ -2500,17 +2504,6 @@ impl DocView { } } -/// The hover popup for an annotation. Replies and Fill & Sign marks (typed text, checks, signatures) -/// show none: they are page content being edited, not comments. -fn annotation_hover_text(a: &pdfcraft_render::Annotation) -> Option { - if a.is_reply() || a.fill_sign { - return None; - } - let who = a.author.clone().unwrap_or_else(|| a.subtype.clone()); - let body = a.contents.clone().unwrap_or_default(); - Some(if body.is_empty() { who } else { format!("{who}\n{body}") }) -} - /// The scroll offset that puts `point` (content coordinates) at `rel` in the viewport, kept within /// the content as egui keeps it. egui clamps only after drawing, so an offset past the end showed /// the page misplaced for a frame and then jumped (zooming out beside a page that fits). @@ -3309,7 +3302,6 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { }; let mut open_signature = false; let mut hover_text: Option<(Pos2, String)> = None; - let mut badge_hover: Option<(Pos2, String)> = None; let mut clicked_link: Option = None; let mut remote_open: Option<(pdfcraft_engine::DocId, String, pdfcraft_render::RemoteDestination)> = None; let mut canvas_action: Option = None; @@ -3379,9 +3371,9 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // The Hand tool pans by this widget's drag, before the pages are laid out (above). if hand { if resp.dragged() { - ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + view.cursor.claim(Layer::Drag, egui::CursorIcon::Grabbing); } else if resp.hovered() { - ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Grab); } } let origin = resp_rect.min - vec2(0.0, y_shift); @@ -3413,7 +3405,6 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } let mut calc_hover = None; for &i in &visible_pages { - badge_hover = None; let r = snap_to_pixels(rects[i].translate(origin.to_vec2()), ppp); if !r.intersects(visible.expand(400.0)) { continue; @@ -3578,7 +3569,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { && allowed && let Some(p) = ptr.within(xf.rect) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); if resp.clicked() { // Centred on the click, upright as the page is shown. let (vx, vy) = xf.screen_to_view(p); @@ -3602,7 +3593,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { && allowed && let Some(p) = ptr.within(xf.rect) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); if resp.clicked() { // Centred on the click at its natural size (the engine sizes it). let (vx, vy) = xf.screen_to_view(p); @@ -3657,14 +3648,13 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { crate::zoom_snap::page_input(ui, &resp, &xf, &ptr, i, view); } if tool == QuickTool::EnhanceCamera { - crate::camera_ui::page_input(ui, &resp, &xf, &ptr, i, view, can_modify); + crate::camera_ui::page_input(&resp, &xf, &ptr, i, view, can_modify); } if tool == QuickTool::Crop && crate::crop::page_input(ui, &resp, &xf, &ptr, i, info, view, can_crop) { view.current = i; open_boxes = true; } - // `over_field`: a form field is under the pointer and has set its cursor. - let (on_field, over_field) = if preparing { + let on_field = if preparing { let field_tool = match tool { QuickTool::Field(f) => Some(f), _ => None, @@ -3675,18 +3665,12 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { field_props |= o.properties; field_placed |= o.placed; open_table = open_table.or(o.table); - (o.consumed || field_tool.is_some(), false) + o.consumed || field_tool.is_some() } else { - use crate::forms_ui::FieldPointer; - let field = if tool == QuickTool::Select + tool == QuickTool::Select && !(editing_content && can_modify && crate::content_ui::select_grabs(&xf, &ptr, i, info, &added, view)) && !comments::select_grabs(&pcx, view) - { - crate::forms_ui::page_input(ui, &resp, &xf, &ptr, i, info, &form, can_fill, view) - } else { - FieldPointer::Away - }; - (field == FieldPointer::Owned, field == FieldPointer::Over) + && crate::forms_ui::page_input(ui, &resp, &xf, &ptr, i, info, &form, can_fill, view) }; // Redact draws boxes off text; so does Highlight (an area highlight, as in Acrobat). let area_tool = tool == QuickTool::Redact && can_modify || tool == QuickTool::Comment(comments::CommentTool::Highlight) && allowed; @@ -3696,7 +3680,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { 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])) }; - crate::redact_ui::page_input(ui, &resp, &xf, &ptr, i, info, over_text, view) + crate::redact_ui::page_input(&resp, &xf, &ptr, i, info, over_text, view) }; let image_owns_pointer = tool == QuickTool::EditText && crate::edit_text_ui::image_owns_pointer(ui, &xf, &ptr, i, info, view); let on_objects = @@ -3729,7 +3713,6 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let on_content = !on_objects && editing_content && can_modify && { source_blocked || crate::content_ui::page_input( - ui, &resp, &xf, &ptr, @@ -3845,15 +3828,12 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // Column select (#740): the Column select tool, or Alt (Option on macOS) held // while a drag starts, as in Acrobat. let column = tool == QuickTool::ColumnSelect || ui.input(|inp| inp.modifiers.alt); - // A link or a form field under the pointer keeps its own cursor: a check box - // drawn with a glyph, or set on printed text, shows the hand, not the I-beam. - // So do the mode bar and side panels over a page scrolled under them (#2755), - // unless a selection is being dragged there. - let cursor_free = !over_link && !over_field && (resp.contains_pointer() || resp.dragged()); - if column && cursor_free { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); - } else if over_text && cursor_free { - ui.ctx().set_cursor_icon(egui::CursorIcon::Text); + // A press on a link selects nothing (it is the link's), so no cursor here says + // it would. + if column && !over_link { + view.cursor.claim(Layer::Text, egui::CursorIcon::Crosshair); + } else if over_text && !over_link { + view.cursor.claim(Layer::Text, egui::CursorIcon::Text); } // The pages' press, kept when its release came in the same frame (#2100): it // still starts multi-click selection. @@ -4029,16 +4009,13 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } } - // Fill & Sign keeps its placement cursor clear of link/comment hover feedback. Tips - // draw above the palette, so none shows while it is open, even beside it. - if !matches!(tool, QuickTool::Fill(_)) - && unobstructed - && let Some(p) = pointer - { + // Links: the hand and the link's target, which count only for the tools that follow + // or edit links (`Layer::reached_by`). Tips draw above the palette, so none shows + // while it is open, even beside it. + if unobstructed && let Some(p) = pointer { for l in info.links.iter().filter(|l| l.page == i) { let sr = xf.user_rect(info, i, l.rect); if sr.contains(p) { - ui.ctx().set_cursor_icon(egui::CursorIcon::PointingHand); let label = match &l.target { LinkTarget::Page(n, _) => { crate::i18n::fmt(tl!("Go to page {p}"), &[("p", info.pages.get(*n).map(|p| p.label.as_str()).unwrap_or("?"))]) @@ -4048,34 +4025,15 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { LinkTarget::RemoteGoTo { file, .. } => file.clone(), LinkTarget::Other(s) => crate::i18n::fmt(tl!("{s} action"), &[("s", s)]), }; - hover_text = Some((p, label)); + view.cursor.claim_with_tip(Layer::Link, egui::CursorIcon::PointingHand, Some(label)); if resp.clicked() && tool == QuickTool::Select && !consumed { clicked_link = Some(l.target.clone()); } } } - // Annotation hover shows the comment, as Acrobat's popups do; not while drawing - // freehand, where it would cover the next stroke (#429). - let freehand = matches!(tool, QuickTool::Comment(comments::CommentTool::Ink | comments::CommentTool::Eraser)); - let gesturing = view.comments.gesture.is_some() || freehand; - // Inspection sorts annotations spatially, but the renderer paints in /Annots - // order. Keep the shared list intact and choose the last eligible raw index. - if hover_text.is_none() - && !gesturing - && let Some((_, text)) = info - .annotations - .iter() - .filter(|a| a.page == i) - .filter(|a| { - !(prefs.hide_empty_highlight_hover && a.subtype == "Highlight" && a.contents.as_deref().is_none_or(str::is_empty)) - }) - .filter(|a| pcx.screen_rects(a).any(|r| r.contains(p))) - .filter_map(|a| annotation_hover_text(a).map(|text| (a.index, text))) - .max_by_key(|(index, _)| *index) - { - hover_text = Some((p, text)); - } } + // Comments say what they are with any tool, beside its cursor, as Acrobat's do. + comments::describe(&pcx, view); for (mp, mr, mc) in &view.compare_marks { if *mp == i { let sr = xf.user_rect(info, i, *mr).expand(1.5); @@ -4120,21 +4078,20 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let side = (sr.width().min(sr.height()) * 0.8).clamp(24.0, 192.0); let mark = Rect::from_center_size(sr.center(), vec2(side, side)); icons::paint(ui, mark, icon, side * 0.64, color); - if let Some(p) = pointer.filter(|p| mark.expand(2.0).contains(*p)) { - badge_hover = Some((p, badge.summary.to_string())); + // Overlapping widgets paint in order, so the topmost badge's verdict is the tip, + // with any tool; a link's or a comment's tip wins over any badge. + if pointer.is_some_and(|p| mark.expand(2.0).contains(p)) { + view.cursor.describe(Subject::SignatureBadge, badge.summary.to_string()); } } - // Overlapping widgets paint in order, so the topmost badge owns the hover; an - // earlier link or annotation hover still wins over any badge. - if hover_text.is_none() { - hover_text = badge_hover.take(); - } } if calc_order { view.calc_order.set_hover_page(calc_hover); let selected = view.prepare.selected.as_ref().map(|(name, _)| name.as_str()); crate::calc_order_ui::paint_trace(ui, painter, visible, &view.calc_order, generation, selected); } + // The pages' one cursor, from the claim on the highest layer the tool reaches, and the tip. + hover_text = pointer.filter(|_| unobstructed).zip(view.cursor.resolve(ui.ctx(), tool)); // ScrollArea ignores the wheel while any widget is dragged, so a text-selection drag // scrolls the page itself (#721). After the page loop, so it keeps scrolling once the // pointer leaves the selection's page for the gap or the next page; over the whole @@ -6044,36 +6001,6 @@ mod tests { } } - fn annotation(fill_sign: bool) -> pdfcraft_render::Annotation { - pdfcraft_render::Annotation { - page: 0, - subtype: "FreeText".into(), - author: None, - contents: Some("Hello".into()), - modified: None, - name: None, - in_reply_to: None, - parent: None, - grouped: false, - rect: [0.0; 4], - color: None, - index: 0, - state: None, - quads: Vec::new(), - locked: false, - intent: None, - fill_sign, - no_rotate: false, - points: Vec::new(), - } - } - - #[test] - fn hover_popup_skips_fill_and_sign_marks() { - assert_eq!(annotation_hover_text(&annotation(false)).as_deref(), Some("FreeText\nHello")); - assert_eq!(annotation_hover_text(&annotation(true)), None); - } - fn signature(signed: bool, visible: bool, page: Option, rect: Option<[f64; 4]>) -> pdfcraft_engine::SignatureInfo { pdfcraft_engine::SignatureInfo { field: "Signature1".into(), diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index bf9b5fe72c..927d1b511c 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -17,6 +17,7 @@ use pdfcraft_engine::{Edit, LineEnding, Markup, NewAnnotation, NoteIcon, ReviewS use pdfcraft_render::{Annotation, DocInfo, PageInfo, PageText}; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::{Layer, Subject}; use crate::page_pointer::PagePointer; use crate::theme::{self, Tokens}; use crate::{QuickTool, icons}; @@ -842,7 +843,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.drag_started() && pressed_here @@ -901,7 +902,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } let finish = resp.double_clicked() || ui.input(|i| i.key_pressed(egui::Key::Enter)); // Backspace (or Delete) takes back the last point, e.g. after a wrong click; Undo does too. @@ -961,7 +962,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.drag_started() && pressed_here @@ -995,7 +996,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.clicked() && over_page @@ -1011,7 +1012,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Text); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Text); } if resp.clicked() && over_page @@ -1030,7 +1031,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, return false; } if over_page { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } // A text box also takes a drag, as in Acrobat: the box is drawn at that size and its // text wraps inside it (#2669). @@ -1146,6 +1147,46 @@ fn clamp_to(r: Rect, p: Pos2) -> Pos2 { pos2(p.x.clamp(r.left(), r.right()), p.y.clamp(r.top(), r.bottom())) } +/// Whether `p` is on one of this page's links. +fn on_link(cx: &PageCx<'_>, p: Pos2) -> bool { + cx.info.links.iter().any(|link| link.page == cx.page && cx.xf.user_rect(cx.info, cx.page, link.rect).contains(p)) +} + +/// The tip for the comment under the pointer, with any tool, beside that tool's own cursor: Acrobat +/// shows a comment's tooltip whatever tool is active. Not with Fill & Sign, whose placement stays +/// clear; not while drawing freehand, where it would cover the next stroke (#429); and not while a +/// comment is moved, resized or drawn. +pub(crate) fn describe(cx: &PageCx<'_>, view: &mut DocView) { + let freehand = matches!(cx.tool, QuickTool::Comment(CommentTool::Ink | CommentTool::Eraser)); + if matches!(cx.tool, QuickTool::Fill(_)) || freehand || view.comments.gesture.is_some() { + return; + } + let Some(p) = cx.ptr.pos else { return }; + // Inspection sorts comments by position, but the renderer paints them in `/Annots` order: the + // tip is that of the one drawn on top, among those with something to say (#3620). + let top = cx + .comments() + .filter(|a| cx.screen_rects(a).any(|r| r.contains(p))) + .filter_map(|a| hover_tip(a, cx.prefs).map(|text| (a.index, text))) + .max_by_key(|(index, _)| *index); + if let Some((_, text)) = top { + view.cursor.describe(Subject::Comment, text); + } +} + +/// What hovering `a` says, as Acrobat's popups do. Replies and Fill & Sign's marks (typed text, +/// checks, signatures) say nothing: they are page content being edited, not comments. Neither +/// does a highlight with no text of its own, when the preferences say so. +fn hover_tip(a: &Annotation, prefs: &CommentPrefs) -> Option { + let empty_highlight = a.subtype == "Highlight" && a.contents.as_deref().is_none_or(str::is_empty); + if a.is_reply() || a.fill_sign || prefs.hide_empty_highlight_hover && empty_highlight { + return None; + } + let who = a.author.clone().unwrap_or_else(|| a.subtype.clone()); + let body = a.contents.clone().unwrap_or_default(); + Some(if body.is_empty() { who } else { format!("{who}\n{body}") }) +} + /// The Select tool: hover, select, move, resize, edit, delete. fn select_input( ui: &egui::Ui, @@ -1170,17 +1211,26 @@ fn select_input( let mut consumed = false; if let Some(p) = pointer { if point_at(p).is_some() { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::Handle, egui::CursorIcon::Crosshair); } else if let Some(h) = handle_at(p) { - ui.ctx().set_cursor_icon(match h { - (0, _) => egui::CursorIcon::ResizeVertical, - (_, 0) => egui::CursorIcon::ResizeHorizontal, - (-1, -1) | (1, 1) => egui::CursorIcon::ResizeNwSe, - _ => egui::CursorIcon::ResizeNeSw, - }); - } else if let Some(a) = cx.hit(p) { + view.cursor.claim( + Layer::Handle, + match h { + (0, _) => egui::CursorIcon::ResizeVertical, + (_, 0) => egui::CursorIcon::ResizeHorizontal, + (-1, -1) | (1, 1) => egui::CursorIcon::ResizeNwSe, + _ => egui::CursorIcon::ResizeNeSw, + }, + ); + } else if let Some(a) = cx.hit(p) + // A click on a highlight over a link follows the link, so the link's hand and tip + // show there. + && !(a.subtype == "Highlight" && on_link(cx, p)) + { let movable = cx.allowed && !is_markup(&a.subtype); - ui.ctx().set_cursor_icon(if movable { egui::CursorIcon::Move } else { egui::CursorIcon::PointingHand }); + let icon = if movable { egui::CursorIcon::Move } else { egui::CursorIcon::PointingHand }; + // Signatures and Fill & Sign's marks are placed on the page rather than comments on it. + view.cursor.claim(if a.fill_sign { Layer::Placed } else { Layer::Comment }, icon); } } if resp.drag_started() @@ -1254,12 +1304,8 @@ fn select_input( // (egui clears the press origin on release, so clicks are located by the pointer.) let on_page = pointer.is_some_and(|p| cx.xf.rect.contains(p)); if resp.clicked() && on_page { - let follow_link = cx.tool == QuickTool::Select - && !consumed - && cv.gesture.is_none() - && !resp.double_clicked() - && pointer - .is_some_and(|p| cx.info.links.iter().any(|link| link.page == cx.page && cx.xf.user_rect(cx.info, cx.page, link.rect).contains(p))); + let follow_link = + cx.tool == QuickTool::Select && !consumed && cv.gesture.is_none() && !resp.double_clicked() && pointer.is_some_and(|p| on_link(cx, p)); match pointer.and_then(|p| cx.hit(p)) { Some(a) if a.subtype == "Highlight" && follow_link => { // A plain click follows the link beneath text highlighting. Keep other @@ -2383,6 +2429,44 @@ mod tests { PageText { glyphs, line_of: vec![0; n], space_before: vec![false; n] } } + fn annotation(subtype: &str, contents: Option<&str>, fill_sign: bool) -> Annotation { + Annotation { + page: 0, + subtype: subtype.into(), + author: None, + contents: contents.map(str::to_string), + modified: None, + name: None, + in_reply_to: None, + parent: None, + grouped: false, + rect: [0.0; 4], + color: None, + index: 0, + state: None, + quads: Vec::new(), + locked: false, + intent: None, + fill_sign, + no_rotate: false, + points: Vec::new(), + } + } + + #[test] + fn the_hover_tip_skips_fill_and_sign_marks_replies_and_bare_highlights_when_asked() { + let prefs = CommentPrefs::default(); + assert_eq!(hover_tip(&annotation("FreeText", Some("Hello"), false), &prefs).as_deref(), Some("FreeText\nHello")); + assert_eq!(hover_tip(&annotation("FreeText", Some("Hello"), true), &prefs), None); + let reply = Annotation { in_reply_to: Some("1 0 R".into()), parent: Some((0, 1)), ..annotation("Text", Some("Agreed"), false) }; + assert_eq!(hover_tip(&reply, &prefs), None); + let bare = Annotation { author: Some("Ana".into()), ..annotation("Highlight", None, false) }; + assert_eq!(hover_tip(&bare, &prefs).as_deref(), Some("Ana")); + let hide = CommentPrefs { hide_empty_highlight_hover: true, ..CommentPrefs::default() }; + assert_eq!(hover_tip(&bare, &hide), None); + assert_eq!(hover_tip(&annotation("Highlight", Some("Check this"), false), &hide).as_deref(), Some("Highlight\nCheck this")); + } + fn near(a: Option<([f32; 2], f32)>, b: ([f32; 2], f32)) -> bool { a.is_some_and(|([x, y], s)| (x - b.0[0]).abs() < 1e-3 && (y - b.0[1]).abs() < 1e-3 && (s - b.1).abs() < 1e-3) } diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 0da3b235c8..56f5ac2729 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -8,6 +8,7 @@ use pdfcraft_engine::{Added, AddedContent, AddedText, Edit}; use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::Tokens; use crate::widgets; @@ -347,7 +348,6 @@ pub(crate) fn select_grabs(xf: &PageXform, ptr: &PagePointer, page: usize, info: /// used the input. #[allow(clippy::too_many_arguments)] pub(crate) fn page_input( - ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, ptr: &PagePointer, @@ -398,13 +398,8 @@ pub(crate) fn page_input( .and_then(|(_, si)| items.iter().find(|(i, _)| *i == si)) .map(|(_, a)| screen_rect(xf, info, page, a.content.rect())); let corner = selected_rect.and_then(|r| handles(r).into_iter().find(|(c, ..)| c.distance(p) <= 6.0)); - // Cursors only where the page shows: the mode bar and side panels over a page scrolled under - // them keep their own (#2755). - let shows = resp.contains_pointer(); if adding_text && hit.is_none() { - if shows { - ui.ctx().set_cursor_icon(egui::CursorIcon::Text); - } + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Text); if resp.clicked() { // Text that can't be drawn stays where it is being typed (#125). if cv.hold_blocked() { @@ -429,10 +424,10 @@ pub(crate) fn page_input( } return consumed; } - if let Some((_, left, top)) = corner.filter(|_| shows) { - ui.ctx().set_cursor_icon(if left == top { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); - } else if hit.is_some() && shows { - ui.ctx().set_cursor_icon(egui::CursorIcon::Move); + if let Some((_, left, top)) = corner { + view.cursor.claim(Layer::Handle, if left == top { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); + } else if hit.is_some() { + view.cursor.claim(Layer::Placed, egui::CursorIcon::Move); } if resp.drag_started() { let origin = ptr.origin.unwrap_or(p); diff --git a/crates/ui-egui/src/crop.rs b/crates/ui-egui/src/crop.rs index 1f1e1aa131..9e0d69b26e 100644 --- a/crates/ui-egui/src/crop.rs +++ b/crates/ui-egui/src/crop.rs @@ -6,6 +6,7 @@ use pdfcraft_engine::{BoxSpec, Edit, PageBox}; use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; /// A crop rectangle being dragged: (page, start on screen). @@ -31,7 +32,7 @@ pub(crate) fn page_input( ) -> bool { let (pointer, origin) = (ptr.pos, ptr.origin); if pointer.is_some_and(|p| xf.rect.contains(p)) { - ui.ctx().set_cursor_icon(if allowed { egui::CursorIcon::Crosshair } else { egui::CursorIcon::NotAllowed }); + view.cursor.claim(Layer::ToolArea, if allowed { egui::CursorIcon::Crosshair } else { egui::CursorIcon::NotAllowed }); } if !allowed { return false; diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 4f5630c0b9..1f0e646094 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -12,6 +12,7 @@ use pdfcraft_engine::{Edit, ObjectKind, ObjectTarget}; use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; const ACCENT: Color32 = Color32::from_rgb(0x14, 0x73, 0xE6); @@ -617,7 +618,9 @@ fn crop_input( painter.text(done.center(), egui::Align2::CENTER_CENTER, tl!("Done"), font.clone(), Color32::WHITE); painter.rect(cancel, CornerRadius::same(11), t.panel, Stroke::new(1.0, t.border), egui::StrokeKind::Inside); painter.text(cancel.center(), egui::Align2::CENTER_CENTER, tl!("Cancel"), font, t.text); - if let Some(p) = hover_pos { + // Cropping takes over its page's cursor, and the buttons' wherever they are; other pages + // keep theirs. + if let Some(p) = hover_pos.filter(|p| xf.rect.contains(*p) || done.contains(*p) || cancel.contains(*p)) { let icon = if let Some(g) = crop_handles(k).iter().position(|h| h.distance(p) < 8.0) { match g { 0 | 3 => egui::CursorIcon::ResizeNwSe, @@ -632,7 +635,7 @@ fn crop_input( } else { egui::CursorIcon::Default }; - ui.ctx().set_cursor_icon(icon); + view.cursor.claim(Layer::Editor, icon); } let clicked = |r: Rect| allow_interaction && resp.clicked() && hover_pos.is_some_and(|p| r.contains(p)); let keys = |key: egui::Key| allow_interaction && ui.input(|inp| inp.key_pressed(key)) && !ui.ctx().egui_wants_keyboard_input(); @@ -911,7 +914,7 @@ pub(crate) fn image_input( handles(b); } if allow_interaction && let Some(corner) = hover_pos.and_then(|p| corners.iter().position(|c| c.distance(p) < 8.0)) { - ui.ctx().set_cursor_icon(if matches!(corner, 0 | 3) { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); + view.cursor.claim(Layer::Handle, if matches!(corner, 0 | 3) { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); } // The rotate handle: a knob above the top edge, joined to it by a short stem. let (knob, stem) = rotate_knob(b, ui.clip_rect()); @@ -919,7 +922,7 @@ pub(crate) fn image_input( painter.line_segment([stem, knob], Stroke::new(1.0, ACCENT)); painter.circle(knob, 5.0, Color32::WHITE, Stroke::new(1.0, ACCENT)); if allow_interaction && hover_pos.is_some_and(|p| p.distance(knob) < 8.0) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + view.cursor.claim(Layer::Handle, egui::CursorIcon::Grab); } } if allow_interaction @@ -959,7 +962,7 @@ pub(crate) fn image_input( let label = Rect::from_center_size(turn(knob) - egui::vec2(0.0, 14.0), galley.size() + egui::vec2(8.0, 4.0)); painter.rect(label, CornerRadius::same(3), Color32::WHITE, Stroke::new(1.0, ACCENT), egui::StrokeKind::Middle); painter.galley(label.center() - galley.size() / 2.0, galley, ACCENT); - ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + view.cursor.claim(Layer::Drag, egui::CursorIcon::Grabbing); if resp.drag_stopped() || !ptr.primary_down { if let Some(s) = view.image_selection.as_mut() { s.drag = None; @@ -1032,11 +1035,14 @@ pub(crate) fn image_input( Some(Edit::EditPageImage { page, index: i, change: pdfcraft_engine::ImageEdit::Move(user_box(xf, info, page, target)) }); } } - ui.ctx().set_cursor_icon(match opposite { - Some(fixed) if (start.x - fixed.x) * (start.y - fixed.y) < 0.0 => egui::CursorIcon::ResizeNeSw, - Some(_) => egui::CursorIcon::ResizeNwSe, - None => egui::CursorIcon::Move, - }); + view.cursor.claim( + Layer::Drag, + match opposite { + Some(fixed) if (start.x - fixed.x) * (start.y - fixed.y) < 0.0 => egui::CursorIcon::ResizeNeSw, + Some(_) => egui::CursorIcon::ResizeNwSe, + None => egui::CursorIcon::Move, + }, + ); return true; } if allow_interaction @@ -1074,12 +1080,13 @@ pub(crate) fn image_input( } let Some(b) = boxes.get(hit).copied() else { return false }; let corner = [b.left_top(), b.right_top(), b.left_bottom(), b.right_bottom()].iter().position(|c| c.distance(p) < 8.0); - ui.ctx().set_cursor_icon(match (selected == Some(hit), corner) { - (true, Some(0 | 3)) => egui::CursorIcon::ResizeNwSe, - (true, Some(_)) => egui::CursorIcon::ResizeNeSw, - (true, None) => egui::CursorIcon::Move, - _ => egui::CursorIcon::PointingHand, - }); + match (selected == Some(hit), corner) { + (true, Some(0 | 3)) => view.cursor.claim(Layer::Handle, egui::CursorIcon::ResizeNwSe), + (true, Some(_)) => view.cursor.claim(Layer::Handle, egui::CursorIcon::ResizeNeSw), + (true, None) => view.cursor.claim(Layer::Placed, egui::CursorIcon::Move), + // A click selects it. + _ => view.cursor.claim(Layer::Placed, egui::CursorIcon::PointingHand), + } if resp.clicked() || resp.secondary_clicked() { view.image_selection = Some(ImageSelection { page, index: hit, drag: None, crop: None, turning: false, settled: None }); } @@ -1288,7 +1295,7 @@ pub(crate) fn page_input( if resizing { draw_handles(preview); } - ui.ctx().set_cursor_icon(if resizing { egui::CursorIcon::ResizeHorizontal } else { egui::CursorIcon::Move }); + view.cursor.claim(Layer::Drag, if resizing { egui::CursorIcon::ResizeHorizontal } else { egui::CursorIcon::Move }); if resp.drag_stopped() || !ptr.any_down { view.block_drag = None; let (from, to) = (user_box(xf, info, page, b), user_box(xf, info, page, preview)); @@ -1337,17 +1344,13 @@ pub(crate) fn page_input( draw_handles(boxes[hit]); } // A closed paragraph moves when dragged (the four-way arrow says so; a click still opens it - // for typing); an edge rewraps it. The open one takes the text cursor. Only where the page - // shows, so the mode bar and side panels over a paragraph scrolled under them keep their own - // cursor (#2755). - if resp.contains_pointer() { - ui.ctx().set_cursor_icon(if on_edge { - egui::CursorIcon::ResizeHorizontal - } else if active.is_none() { - egui::CursorIcon::Move - } else { - egui::CursorIcon::Text - }); + // for typing); an edge rewraps it. The open one takes the text cursor. + if on_edge { + view.cursor.claim(Layer::Handle, egui::CursorIcon::ResizeHorizontal); + } else if active.is_none() { + view.cursor.claim(Layer::Text, egui::CursorIcon::Move); + } else { + view.cursor.claim(if active == Some(hit) { Layer::Editor } else { Layer::Text }, egui::CursorIcon::Text); } if resp.clicked() { view.image_selection = None; diff --git a/crates/ui-egui/src/fill_sign.rs b/crates/ui-egui/src/fill_sign.rs index aab071b508..0861cd3cdd 100644 --- a/crates/ui-egui/src/fill_sign.rs +++ b/crates/ui-egui/src/fill_sign.rs @@ -9,6 +9,7 @@ use pdfcraft_engine::{Edit, FillMark, NewAnnotation, PhotoRect, Shape, Signature use pdfcraft_render::{DocInfo, PageInfo}; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::Tokens; @@ -651,10 +652,10 @@ pub(crate) fn page_input( if let Some(rect) = image.rect(p, at, tool == FillTool::Initials) { let tex = image_texture(ui, image, preview); xf.paint_user_image(ui.painter(), tex, p, rect, i64::from(p.rotation), Color32::WHITE); - ui.ctx().set_cursor_icon(egui::CursorIcon::None); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::None); } } else { - ui.ctx().set_cursor_icon(if tool == FillTool::Text { egui::CursorIcon::Text } else { egui::CursorIcon::Crosshair }); + view.cursor.claim(Layer::ToolArea, if tool == FillTool::Text { egui::CursorIcon::Text } else { egui::CursorIcon::Crosshair }); } if !resp.clicked() { return None; diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index 28494160c5..e0d51a3da2 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -14,6 +14,7 @@ use pdfcraft_engine::{Edit, FieldValue, FormField, FormFieldKind, FormWidget, fi use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::Tokens; @@ -151,20 +152,7 @@ fn toggle_edit(f: &FormField, w: &FormWidget) -> Option { Some(Edit::SetFieldValue { name: f.name.clone(), value }) } -/// Where the pointer is relative to one page's form widgets. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum FieldPointer { - /// Not over a field. - Away, - /// Over a field, which has set the cursor. Nothing is pressed, so the layers below still see - /// the hover, but they must not replace the cursor: a check box drawn with a glyph, or set - /// on printed text, is a button, not text to select. - Over, - /// The field owns the press or click. - Owned, -} - -/// Clicks on form widgets of one page. +/// Clicks on form widgets of one page. `true` when a field owns the press or click. #[allow(clippy::too_many_arguments)] pub(crate) fn page_input( ui: &egui::Ui, @@ -176,8 +164,8 @@ pub(crate) fn page_input( form: &[FormField], allowed: bool, view: &mut DocView, -) -> FieldPointer { - let Some(p) = ptr.within(xf.rect) else { return FieldPointer::Away }; +) -> bool { + let Some(p) = ptr.within(xf.rect) else { return false }; let hit = form.iter().find_map(|f| { f.widgets .iter() @@ -185,7 +173,7 @@ pub(crate) fn page_input( .find(|(_, w)| w.page == Some(page) && !w.hidden && widget_rect(xf, info, page, w.rect).contains(p)) .map(|(i, w)| (f, i, w)) }); - let Some((f, wi, w)) = hit else { return FieldPointer::Away }; + let Some((f, wi, w)) = hit else { return false }; // The field's tooltip (`/TU`), as Acrobat shows it on hover (#3854). ISO 32000-2 Table 226 // makes it the name the user interface identifies the field by. if let Some(tip) = f.tooltip.as_deref().filter(|t| !t.trim().is_empty()) { @@ -196,15 +184,16 @@ pub(crate) fn page_input( let pressable = f.kind == FormFieldKind::PushButton && !f.read_only() && f.button.is_some(); // A radio button of a group can be read-only on its own. let usable = allowed && !w.read_only && (fillable(f) || pressable || f.kind == FormFieldKind::Signature); - // Only where the page shows: the mode bar and side panels over a field scrolled under them - // keep their own cursor (#2755). - if resp.contains_pointer() { - ui.ctx().set_cursor_icon(match (usable, f.kind) { + // Over the page's text too: a check box drawn with a glyph, or set on printed text, is a + // button, not text to select. + view.cursor.claim( + Layer::Field, + match (usable, f.kind) { (false, _) => egui::CursorIcon::NotAllowed, (true, FormFieldKind::Text) => egui::CursorIcon::Text, _ => egui::CursorIcon::PointingHand, - }); - } + }, + ); // A text field opens on the press rather than the click, so a drag that starts on it // selects its text (#2213). let pressed = f.kind == FormFieldKind::Text && usable && resp.is_pointer_button_down_on() && ptr.pressed().is_some(); @@ -214,11 +203,11 @@ pub(crate) fn page_input( view.forms.pressed = true; } if !resp.clicked() { - return if resp.is_pointer_button_down_on() { FieldPointer::Owned } else { FieldPointer::Over }; + return resp.is_pointer_button_down_on(); } if !allowed { view.forms.notice = Some(FormNotice::Security); - return FieldPointer::Owned; + return true; } match f.kind { _ if f.read_only() || w.read_only => view.forms.notice = Some(FormNotice::ReadOnly(f.name.clone())), @@ -289,7 +278,7 @@ pub(crate) fn page_input( } } } - FieldPointer::Owned + true } /// The edit the focused field's draft makes, if it differs from the field's value. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 79f46bee7b..f909698238 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -104,6 +104,7 @@ pub mod icons; pub mod last_session; pub mod last_view; pub mod motion; +mod page_cursor; mod page_pointer; mod page_selection_ui; mod pageboxes; diff --git a/crates/ui-egui/src/link_ui.rs b/crates/ui-egui/src/link_ui.rs index f7768912dd..012a95f95f 100644 --- a/crates/ui-egui/src/link_ui.rs +++ b/crates/ui-egui/src/link_ui.rs @@ -7,6 +7,7 @@ use pdfcraft_engine::{Edit, LinkAction, LinkHighlight, LinkItem, LinkStyle}; use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::{self, Tokens}; use crate::{PdfCraftApp, widgets}; @@ -86,7 +87,10 @@ pub(crate) fn page_input( } let Some(p) = pointer.filter(|p| xf.rect.contains(*p)) else { return false }; let hit = links.iter().filter(|l| l.page == page).find(|l| screen_rect(xf, info, page, l.rect).expand(2.0).contains(p)); - ui.ctx().set_cursor_icon(if hit.is_some() { egui::CursorIcon::PointingHand } else { egui::CursorIcon::Crosshair }); + match hit { + Some(_) => view.cursor.claim(Layer::Link, egui::CursorIcon::PointingHand), + None => view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair), + } if resp.secondary_clicked() && let Some(l) = hit { diff --git a/crates/ui-egui/src/measure_ui.rs b/crates/ui-egui/src/measure_ui.rs index 79670ad304..17e3ced693 100644 --- a/crates/ui-egui/src/measure_ui.rs +++ b/crates/ui-egui/src/measure_ui.rs @@ -1,5 +1,5 @@ //! Measure objects: page-space clicks, live readings and persistent drawing scales. -use crate::{LeftPanel, PdfCraftApp, QuickTool, canvas::DocView, theme::Tokens, widgets}; +use crate::{LeftPanel, PdfCraftApp, QuickTool, canvas::DocView, page_cursor::Layer, theme::Tokens, widgets}; use egui::{Color32, Pos2, Stroke}; use pdfcraft_engine::{ Document, Edit, Style, @@ -200,7 +200,7 @@ pub(crate) fn page_input( let mut hover = None; let mut snap = None; if let Some(pos) = pos { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); let (vx, vy) = xf.screen_to_view(pos); let [x, y] = info.view_to_user(vx, vy); let mut point = [f64::from(x), f64::from(y)]; diff --git a/crates/ui-egui/src/object_ui.rs b/crates/ui-egui/src/object_ui.rs index d495116c6c..8d4a1137e6 100644 --- a/crates/ui-egui/src/object_ui.rs +++ b/crates/ui-egui/src/object_ui.rs @@ -8,6 +8,7 @@ use pdfcraft_engine::{Document, Edit, EditableObject, MAX_MOVE_OBJECTS, ObjectKi use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; const BLUE: Color32 = Color32::from_rgb(0x14, 0x73, 0xE6); @@ -255,7 +256,7 @@ fn input( } else { s.gesture = Some(gesture); } - ui.ctx().set_cursor_icon(if gesture.marquee { egui::CursorIcon::Crosshair } else { egui::CursorIcon::Move }); + view.cursor.claim(Layer::Drag, if gesture.marquee { egui::CursorIcon::Crosshair } else { egui::CursorIcon::Move }); return true; } let Some(p) = pointer.filter(|p| xf.rect.contains(*p) && ui.clip_rect().contains(*p) && resp.rect.contains(*p)) else { @@ -265,7 +266,7 @@ fn input( if s.page == Some(page) && !s.selected.is_empty() { let union = s.selected.iter().fold(Rect::NOTHING, |r, o| r.union(screen(xf, info, page, o))); if union.contains(p) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Move); + view.cursor.claim(Layer::Placed, egui::CursorIcon::Move); } return true; } diff --git a/crates/ui-egui/src/page_cursor.rs b/crates/ui-egui/src/page_cursor.rs new file mode 100644 index 0000000000..9abfb7e994 --- /dev/null +++ b/crates/ui-egui/src/page_cursor.rs @@ -0,0 +1,332 @@ +//! The cursor the pages show, and the tip beside it, set once a frame. +//! +//! A page module claims the cursor for what it has under the pointer, on a [`Layer`], and never +//! sets egui's cursor itself: where each module set it, the last write of a frame won, so the +//! order of the per-page chain decided the cursor and not what a press there does. A link's hand +//! showed over every tool though only the Select tool follows links, and the I-beam of the text +//! under a comment hid that a drag there moves the comment. `canvas::document_area` sets the +//! cursor once, from the claim on the highest layer the active tool reaches. The tip beside it is +//! that claim's own (a link's target, so only where a press follows the link), or else what lies +//! under the pointer says (a comment, or a signature's validation badge), with any tool, as +//! Acrobat shows a comment's tooltip. A test in `page_pointer.rs` keeps page modules from setting +//! the cursor. + +use egui::CursorIcon; + +use crate::QuickTool; + +/// What a claim is for, from the bottom up: a claim on a higher layer wins. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum Layer { + /// The page under the active tool: the crosshair it places or draws with, its I-beam or hand. + ToolArea, + /// The page's text, to select, and Edit text's paragraphs. + Text, + /// Links. + Link, + /// Comments: notes, shapes and markup. + Comment, + /// Form fields. + Field, + /// What a drag moves whole: signatures and Fill & Sign's marks, added text and images, + /// selected objects, pictures in Edit text. + Placed, + /// The grips of the selected item: resize, rewrap and crop handles, paragraph edges. + Handle, + /// An open editor: a paragraph being typed in, a picture being cropped. + Editor, + /// A drag in progress: what is dragged keeps its cursor until the release. + Drag, +} + +/// Every layer, bottom first. +const LAYERS: [Layer; 9] = + [Layer::ToolArea, Layer::Text, Layer::Link, Layer::Comment, Layer::Field, Layer::Placed, Layer::Handle, Layer::Editor, Layer::Drag]; + +impl Layer { + /// Whether a press with `tool` acts on what lies on this layer, so that the layer's claims + /// count. Only the Select tool follows links (the Link tool edits them) and picks up + /// comments: every other tool draws, places or selects text over them. The other layers are + /// claimed only by the modules of the tools that act on them, so they count for every tool. + pub(crate) fn reached_by(self, tool: QuickTool) -> bool { + match self { + Layer::Link => matches!(tool, QuickTool::Select | QuickTool::Link), + Layer::Comment => tool == QuickTool::Select, + Layer::ToolArea | Layer::Text | Layer::Field | Layer::Placed | Layer::Handle | Layer::Editor | Layer::Drag => true, + } + } +} + +/// A claim on the cursor: the icon, and the tip that goes with it (a link's target). +#[derive(Clone, Debug, PartialEq)] +struct Claim { + icon: CursorIcon, + tip: Option, +} + +/// What a description tells about, from the least up: a higher one shows over a lower one. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum Subject { + /// A signature's validation badge: viewer chrome over the signer's own artwork. + SignatureBadge, + /// A comment: who wrote it, and what it says. + Comment, +} + +/// What the page modules claim for the cursor this frame. `canvas::document_area` sets the +/// cursor from it once, after the pages ([`PageCursor::resolve`]). +#[derive(Debug, Default)] +pub(crate) struct PageCursor { + /// The last claim on each layer, bottom layer first. + claims: [Option; LAYERS.len()], + /// What lies under the pointer says ([`PageCursor::describe`]). + description: Option<(Subject, String)>, +} + +impl PageCursor { + /// Shows `icon` while the pointer is over what lies on `layer`, unless the active tool + /// reaches a higher layer that is claimed too. + pub(crate) fn claim(&mut self, layer: Layer, icon: CursorIcon) { + self.claim_with_tip(layer, icon, None); + } + + /// As [`Self::claim`], with `tip` beside the pointer where the claim wins. + pub(crate) fn claim_with_tip(&mut self, layer: Layer, icon: CursorIcon, tip: Option) { + if let Some(slot) = self.claims.get_mut(layer as usize) { + *slot = Some(Claim { icon, tip }); + } + } + + /// Says what lies under the pointer without claiming the cursor: the tip shows with any tool, + /// beside that tool's cursor, as Acrobat shows a comment's tooltip, unless the winning claim + /// has a tip of its own. The description of the highest subject stands, and the last of + /// equals (the topmost of overlapping badges). + pub(crate) fn describe(&mut self, subject: Subject, text: String) { + if self.description.as_ref().is_none_or(|(held, _)| *held <= subject) { + self.description = Some((subject, text)); + } + } + + /// The claim on the highest layer `tool` reaches. + fn winner(claims: [Option; LAYERS.len()], tool: QuickTool) -> Option { + LAYERS.into_iter().zip(claims).rev().find_map(|(layer, claim)| claim.filter(|_| layer.reached_by(tool))) + } + + /// Sets the cursor, once, to the icon of the claim on the highest layer `tool` reaches, and + /// returns the tip: that claim's own, or else the description. With no such claim the cursor + /// is left as it is. The claims are spent: the next frame starts with none. + pub(crate) fn resolve(&mut self, ctx: &egui::Context, tool: QuickTool) -> Option { + let PageCursor { claims, description } = std::mem::take(self); + let winner = Self::winner(claims, tool); + if let Some(w) = &winner { + ctx.set_cursor_icon(w.icon); + } + winner.and_then(|w| w.tip).or(description.map(|(_, text)| text)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::comments::CommentTool; + use crate::fill_sign::FillTool; + use crate::prepare::FieldTool; + + /// One of each tool. The match stops compiling when a tool is added, so that it gets a row + /// in [`Layer::reached_by`]'s tests. + fn tools() -> Vec { + let tools = vec![ + QuickTool::Select, + QuickTool::Hand, + QuickTool::Comment(CommentTool::Rectangle), + QuickTool::Comment(CommentTool::Highlight), + QuickTool::Measure(crate::measure_ui::Tool::Distance), + QuickTool::Crop, + QuickTool::EnhanceCamera, + QuickTool::Fill(FillTool::Signature), + QuickTool::Field(FieldTool::Text), + QuickTool::Redact, + QuickTool::AddText, + QuickTool::EditText, + QuickTool::Stamp(pdfcraft_engine::StampKind::DynApproved), + QuickTool::CustomStamp(0), + QuickTool::Link, + QuickTool::SignArea { certify: false }, + QuickTool::MarqueeZoom, + QuickTool::Snapshot, + QuickTool::ColumnSelect, + ]; + for tool in &tools { + match tool { + QuickTool::Select + | QuickTool::Hand + | QuickTool::Comment(_) + | QuickTool::Measure(_) + | QuickTool::Crop + | QuickTool::EnhanceCamera + | QuickTool::Fill(_) + | QuickTool::Field(_) + | QuickTool::Redact + | QuickTool::AddText + | QuickTool::EditText + | QuickTool::Stamp(_) + | QuickTool::CustomStamp(_) + | QuickTool::Link + | QuickTool::SignArea { .. } + | QuickTool::MarqueeZoom + | QuickTool::Snapshot + | QuickTool::ColumnSelect => {} + } + } + tools + } + + #[test] + fn layers_are_listed_bottom_first_in_their_order() { + for (i, layer) in LAYERS.into_iter().enumerate() { + assert_eq!(layer as usize, i, "{layer:?}"); + } + assert!(LAYERS.windows(2).all(|w| w[0] < w[1])); + } + + #[test] + fn links_count_for_the_select_and_link_tools_only() { + for tool in tools() { + assert_eq!(Layer::Link.reached_by(tool), matches!(tool, QuickTool::Select | QuickTool::Link), "{tool:?}"); + } + } + + #[test] + fn comments_count_for_the_select_tool_only() { + for tool in tools() { + assert_eq!(Layer::Comment.reached_by(tool), tool == QuickTool::Select, "{tool:?}"); + } + } + + #[test] + fn the_other_layers_count_for_every_tool() { + for tool in tools() { + for layer in [Layer::ToolArea, Layer::Text, Layer::Field, Layer::Placed, Layer::Handle, Layer::Editor, Layer::Drag] { + assert!(layer.reached_by(tool), "{layer:?} for {tool:?}"); + } + } + } + + fn claims(claims: &[(Layer, CursorIcon, Option<&str>)]) -> PageCursor { + let mut cursor = PageCursor::default(); + for (layer, icon, tip) in claims { + cursor.claim_with_tip(*layer, *icon, tip.map(str::to_string)); + } + cursor + } + + fn winner(cursor: PageCursor, tool: QuickTool) -> Option { + PageCursor::winner(cursor.claims, tool) + } + + #[test] + fn the_highest_layer_the_tool_reaches_wins_with_its_tip() { + let on_text_under_a_link = [(Layer::Text, CursorIcon::Text, None), (Layer::Link, CursorIcon::PointingHand, Some("https://example.org/"))]; + let select = winner(claims(&on_text_under_a_link), QuickTool::Select); + assert_eq!(select, Some(Claim { icon: CursorIcon::PointingHand, tip: Some("https://example.org/".into()) })); + // A markup tool selects the text there, and the link's tip goes with its hand. + let highlight = winner(claims(&on_text_under_a_link), QuickTool::Comment(CommentTool::Highlight)); + assert_eq!(highlight, Some(Claim { icon: CursorIcon::Text, tip: None })); + // A tool reaching nothing claimed leaves the cursor alone. + assert_eq!(winner(claims(&[(Layer::Link, CursorIcon::PointingHand, None)]), QuickTool::Hand), None); + } + + #[test] + fn a_higher_claim_without_a_tip_hides_a_lower_ones_tip() { + let field_over_a_link = [(Layer::Link, CursorIcon::PointingHand, Some("Go to page 2")), (Layer::Field, CursorIcon::Text, None)]; + assert_eq!(winner(claims(&field_over_a_link), QuickTool::Select), Some(Claim { icon: CursorIcon::Text, tip: None })); + } + + #[test] + fn the_last_claim_on_a_layer_wins() { + let two_links = [(Layer::Link, CursorIcon::PointingHand, Some("first")), (Layer::Link, CursorIcon::PointingHand, Some("second"))]; + assert_eq!(winner(claims(&two_links), QuickTool::Select).and_then(|c| c.tip).as_deref(), Some("second")); + } + + /// Runs one frame that sets the cursor to `before`, if any, then resolves `claims` for + /// `tool`. Returns the frame's cursor and the tip. + fn frame(ctx: &egui::Context, before: Option, claims: &mut PageCursor, tool: QuickTool) -> (CursorIcon, Option) { + let mut tip = None; + let mut output = ctx.run_ui(egui::RawInput::default(), |ui| { + if let Some(icon) = before { + ui.ctx().set_cursor_icon(icon); + } + tip = claims.resolve(ui.ctx(), tool); + }); + // An input-only test has no renderer to take the font texture. + output.textures_delta.clear(); + (output.platform_output.cursor_icon, tip) + } + + #[test] + fn resolve_sets_the_winners_cursor_and_spends_the_claims() { + let ctx = egui::Context::default(); + let on_a_link = || claims(&[(Layer::ToolArea, CursorIcon::Crosshair, None), (Layer::Link, CursorIcon::PointingHand, Some("tip"))]); + assert_eq!(frame(&ctx, None, &mut on_a_link(), QuickTool::Select), (CursorIcon::PointingHand, Some("tip".into()))); + let stamp = QuickTool::Stamp(pdfcraft_engine::StampKind::DynApproved); + assert_eq!(frame(&ctx, None, &mut on_a_link(), stamp), (CursorIcon::Crosshair, None), "the stamp's crosshair, and no link tip"); + // Nothing the tool reaches: the cursor stays as the frame set it. + let mut only_a_link = claims(&[(Layer::Link, CursorIcon::PointingHand, Some("tip"))]); + only_a_link.describe(Subject::Comment, "Ana".into()); + assert_eq!(frame(&ctx, Some(CursorIcon::Help), &mut only_a_link, QuickTool::Hand), (CursorIcon::Help, Some("Ana".into()))); + // Resolving spends the claims, reached or not, and the description. + assert_eq!(only_a_link.description, None); + assert_eq!(winner(only_a_link, QuickTool::Select), None); + } + + #[test] + fn a_description_shows_with_any_tool_unless_the_winner_has_a_tip_of_its_own() { + let ctx = egui::Context::default(); + let stamp = QuickTool::Stamp(pdfcraft_engine::StampKind::DynApproved); + let link = (Layer::Link, CursorIcon::PointingHand, Some("https://example.org/")); + let described = |claimed: &[(Layer, CursorIcon, Option<&str>)]| { + let mut cursor = claims(claimed); + cursor.describe(Subject::Comment, "Ana\nCheck this".into()); + cursor + }; + let comment = Some("Ana\nCheck this".to_string()); + // A comment under the stamp's crosshair: the tool's cursor, and what the comment says. + let crosshair = (Layer::ToolArea, CursorIcon::Crosshair, None); + assert_eq!(frame(&ctx, None, &mut described(&[crosshair]), stamp), (CursorIcon::Crosshair, comment.clone())); + // A link the tool follows speaks for itself; one it doesn't keeps its tip to itself. + assert_eq!( + frame(&ctx, None, &mut described(&[crosshair, link]), QuickTool::Select), + (CursorIcon::PointingHand, Some("https://example.org/".into())) + ); + assert_eq!(frame(&ctx, None, &mut described(&[crosshair, link]), stamp), (CursorIcon::Crosshair, comment.clone())); + // A winner without a tip of its own, such as the Select tool's comment claim. + let picked_up = (Layer::Comment, CursorIcon::Move, None); + assert_eq!(frame(&ctx, None, &mut described(&[link, picked_up]), QuickTool::Select), (CursorIcon::Move, comment)); + } + + #[test] + fn a_comment_says_more_than_a_signature_badge_and_the_last_badge_stands() { + let badge_then_comment = { + let mut cursor = PageCursor::default(); + cursor.describe(Subject::SignatureBadge, "Signature is valid".into()); + cursor.describe(Subject::Comment, "Ana".into()); + cursor + }; + let comment_then_badges = { + let mut cursor = PageCursor::default(); + cursor.describe(Subject::Comment, "Ana".into()); + cursor.describe(Subject::SignatureBadge, "Signature is valid".into()); + cursor + }; + let two_badges = { + let mut cursor = PageCursor::default(); + cursor.describe(Subject::SignatureBadge, "Signature is valid".into()); + cursor.describe(Subject::SignatureBadge, "Signature is invalid".into()); + cursor + }; + let said = |cursor: PageCursor| cursor.description.map(|(_, text)| text); + assert_eq!(said(badge_then_comment).as_deref(), Some("Ana")); + assert_eq!(said(comment_then_badges).as_deref(), Some("Ana"), "whichever came first"); + assert_eq!(said(two_badges).as_deref(), Some("Signature is invalid"), "the badge painted last is on top"); + } +} diff --git a/crates/ui-egui/src/prepare.rs b/crates/ui-egui/src/prepare.rs index 2635693676..5d7cf72e1e 100644 --- a/crates/ui-egui/src/prepare.rs +++ b/crates/ui-egui/src/prepare.rs @@ -12,6 +12,7 @@ use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; use crate::comments; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme; use crate::widgets; @@ -344,7 +345,7 @@ pub(crate) fn page_input( return out; } if let Some(tool) = tool { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); if resp.drag_started() || resp.clicked() { let origin = ptr.origin.filter(|o| xf.rect.contains(*o)).unwrap_or(p); prep.grab = Some((page, Grab::Draw(origin), origin)); @@ -397,9 +398,9 @@ pub(crate) fn page_input( f.widgets.iter().enumerate().find(|(_, w)| w.page == Some(page) && screen_rect(xf, info, page, w.rect).contains(p)).map(|(i, _)| (f, i)) }); if let Some((_, left, top)) = corner { - ui.ctx().set_cursor_icon(if left == top { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); + view.cursor.claim(Layer::Handle, if left == top { egui::CursorIcon::ResizeNwSe } else { egui::CursorIcon::ResizeNeSw }); } else if hit.is_some() { - ui.ctx().set_cursor_icon(egui::CursorIcon::Move); + view.cursor.claim(Layer::Field, egui::CursorIcon::Move); } if resp.drag_started() { let origin = ptr.origin.unwrap_or(p); diff --git a/crates/ui-egui/src/redact_ui.rs b/crates/ui-egui/src/redact_ui.rs index 6a5bee7ea4..52cadc2aa2 100644 --- a/crates/ui-egui/src/redact_ui.rs +++ b/crates/ui-egui/src/redact_ui.rs @@ -8,6 +8,7 @@ use pdfcraft_engine::{Edit, NewAnnotation, REDACT_PATTERNS, REDACTION_CODE_SETS, use pdfcraft_render::DocInfo; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::Tokens; use crate::{PdfCraftApp, widgets}; @@ -237,7 +238,6 @@ fn to_user(xf: &PageXform, info: &DocInfo, page: usize, p: Pos2) -> [f64; 2] { /// the gesture is a box. #[allow(clippy::too_many_arguments)] pub(crate) fn page_input( - ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, ptr: &PagePointer, @@ -263,7 +263,7 @@ pub(crate) fn page_input( } let Some(p) = pointer.filter(|p| xf.rect.contains(*p)) else { return false }; if !over_text(p) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.drag_started() { let origin = ptr.origin.unwrap_or(p); diff --git a/crates/ui-egui/src/sign_ui.rs b/crates/ui-egui/src/sign_ui.rs index 8f6b2c4a0c..4df02cbf41 100644 --- a/crates/ui-egui/src/sign_ui.rs +++ b/crates/ui-egui/src/sign_ui.rs @@ -10,6 +10,7 @@ use pdfcraft_engine::{SignOptions, SignatureImage, SignatureInfo, SignatureStatu use crate::canvas::{DocView, PageXform, Snapshot, SnapshotKind}; use crate::fill_sign::SavedSig; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::theme::{self, Tokens}; use crate::{PdfCraftApp, icons, widgets}; @@ -163,7 +164,7 @@ pub(crate) fn page_input( ) { let (pointer, origin) = (ptr.pos, ptr.origin); if pointer.is_some_and(|p| xf.rect.contains(p)) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.drag_started() && let Some(o) = origin.filter(|o| xf.rect.contains(*o)) diff --git a/crates/ui-egui/src/zoom_snap.rs b/crates/ui-egui/src/zoom_snap.rs index e0315763fa..8e530ac58e 100644 --- a/crates/ui-egui/src/zoom_snap.rs +++ b/crates/ui-egui/src/zoom_snap.rs @@ -5,6 +5,7 @@ use egui::{Color32, CornerRadius, Pos2, Rect, Stroke}; use pdfcraft_render::{RenderConfig, RenderRequest, RequestKind, Tile}; use crate::canvas::{DocView, PageXform}; +use crate::page_cursor::Layer; use crate::page_pointer::PagePointer; use crate::{PdfCraftApp, QuickTool}; @@ -16,7 +17,7 @@ pub type Marquee = (usize, Rect, [f32; 4]); pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, xf: &PageXform, ptr: &PagePointer, page: usize, view: &mut DocView) { let (pointer, origin) = (ptr.pos, ptr.origin); if pointer.is_some_and(|p| xf.rect.contains(p)) { - ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair); + view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } if resp.drag_started() && let Some(o) = origin.filter(|o| xf.rect.contains(*o)) diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 9bf7f70813..1b5181ff60 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -3974,12 +3974,14 @@ fn dragging_the_rotate_knob_turns_a_page_image() { let to = egui::pos2(c.x + 100.0, c.y); h.hover_at(knob); h.run_steps(1); + assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::Grab, "the knob offers to turn the picture"); h.drag_at(knob); h.run_steps(1); for k in 1..=4 { h.hover_at(knob + (to - knob) * (k as f32 / 4.0)); h.run_steps(1); } + assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::Grabbing, "and holds it while it turns"); h.drop_at(to); h.run_steps(4); let doc = h.state().session.get(id).unwrap(); diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index f2b6c0e907..74120834bc 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -3,12 +3,14 @@ //! dialog, the palette, the quick bar, the chrome over a page scrolled under it) lets the page's //! cursor, tips or presses through. //! -//! The ignored tests pin live bugs that later steps of the page pointer router fix: one cursor -//! writer (step 2), one hit test (step 3), and press-time capture with cancel (step 4). +//! The ignored tests pin live bugs that later steps of the page pointer router fix: one hit test +//! (step 3), and press-time capture with cancel (step 4). use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; use pdfcraft_engine::StampKind; use pdfcraft_ui_egui::comments::CommentTool; +use pdfcraft_ui_egui::fill_sign::FillTool; use pdfcraft_ui_egui::{PdfCraftApp, QuickTool, UnlockPrompt}; use crate::support::input::{coalesced_click, double_click, one_frame, touch_drag, touch_tap}; @@ -76,6 +78,18 @@ fn comment_on_text_pdf() -> Vec { ]) } +/// A 300×400 page with two rectangle comments overlapping at x 100..150, y 200..250: "Bottom +/// note" first in `/Annots`, then "Top note" over it. +fn stacked_comments_pdf() -> Vec { + pdf(&[ + CATALOG, + PAGES, + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R] >>", + "<< /Type /Annot /Subtype /Square /Rect [50 150 150 250] /C [1 0 0] /F 4 /T (Ana) /Contents (Bottom note) >>", + "<< /Type /Annot /Subtype /Square /Rect [100 200 200 300] /C [0 0 1] /F 4 /T (Ben) /Contents (Top note) >>", + ]) +} + /// A 300×400 page with one line of text at x 40, y 300. fn text_pdf() -> Vec { let text = stream(LINE); @@ -173,6 +187,12 @@ fn agree(h: &Harness<'_, PdfCraftApp>) -> Vec { s.session.get(s.views[0].id).unwrap().form.iter().find(|f| f.name == "agree").unwrap().value.clone() } +/// Whether the page's hover tip shows and says `text`. +fn tip_says(h: &Harness<'_, PdfCraftApp>, text: &str) -> bool { + let Some(tip) = h.ctx.memory(|m| m.area_rect(egui::Id::unique("canvas-hover"))).filter(|_| tip_shown(h)) else { return false }; + h.query_all_by_label_contains(text).any(|n| tip.contains(n.rect().center())) +} + /// Hovers `p` until `want` accepts the cursor there (text layers and paragraph boxes arrive from /// worker threads), and returns the last cursor seen. fn wait_for_cursor(h: &mut Harness<'_, PdfCraftApp>, p: egui::Pos2, want: impl Fn(egui::CursorIcon) -> bool) -> egui::CursorIcon { @@ -191,7 +211,6 @@ fn wait_for_cursor(h: &mut Harness<'_, PdfCraftApp>, p: egui::Pos2, want: impl F // The cursor promises one thing and a press does another. #[test] -#[ignore = "step 2 of the page pointer router: one cursor writer"] fn stamp_tool_over_a_link_shows_its_crosshair() { let mut h = open(link_pdf(), "150", false); stamp_tool(&mut h); @@ -205,7 +224,6 @@ fn stamp_tool_over_a_link_shows_its_crosshair() { } #[test] -#[ignore = "step 2 of the page pointer router: one cursor writer"] fn rectangle_tool_over_a_link_shows_its_crosshair() { let mut h = open(link_pdf(), "150", false); h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Rectangle); @@ -218,7 +236,6 @@ fn rectangle_tool_over_a_link_shows_its_crosshair() { } #[test] -#[ignore = "step 2 of the page pointer router: one cursor writer"] fn edit_text_over_a_linked_word_offers_the_move_arrow() { let mut h = open(linked_text_pdf(), "150", false); h.state_mut().quick_tool = QuickTool::EditText; @@ -242,7 +259,6 @@ fn edit_text_over_a_linked_word_offers_the_move_arrow() { } #[test] -#[ignore = "step 2 of the page pointer router: one cursor writer"] fn select_tool_over_a_comment_on_text_offers_the_move_arrow() { let mut h = open(comment_on_text_pdf(), "150", false); // Inside the rectangle where there is no text, and inside it over a word. @@ -259,6 +275,71 @@ fn select_tool_over_a_comment_on_text_offers_the_move_arrow() { assert_eq!(over_text, egui::CursorIcon::Move, "so it offers the arrow where text lies under it too"); } +#[test] +fn a_links_tip_shows_only_with_its_hand() { + let mut h = open(link_pdf(), "150", false); + let on_link = at(&h, 150.0, 325.0); + let hover = |h: &mut Harness<'_, PdfCraftApp>, tool: QuickTool, nudge: f32| { + h.state_mut().quick_tool = tool; + h.run_steps(4); + // A new position each time, so the hover is fresh. + let cursor = cursor_at(h, on_link + egui::vec2(nudge, 0.0)); + h.run_steps(2); + (cursor, tip_says(h, "https://example.org/")) + }; + use egui::CursorIcon::{Crosshair, PointingHand}; + assert_eq!(hover(&mut h, QuickTool::Select, 0.0), (PointingHand, true), "the Select tool follows the link, and the tip says where to"); + assert_eq!(hover(&mut h, QuickTool::Link, 1.0), (PointingHand, true), "the Link tool edits it"); + let stamp = QuickTool::Stamp(StampKind::DynApproved); + assert_eq!(hover(&mut h, stamp, 2.0), (Crosshair, false), "a click places a stamp, so neither hand nor tip offers the link"); + let rectangle = QuickTool::Comment(CommentTool::Rectangle); + assert_eq!(hover(&mut h, rectangle, 3.0), (Crosshair, false), "a drag draws a rectangle"); +} + +/// 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. +#[test] +#[ignore = "step 3 of the page pointer router: one hit test"] +fn a_click_on_stacked_comments_selects_the_one_drawn_on_top() { + let mut h = open(stacked_comments_pdf(), "150", false); + let overlap = at(&h, 125.0, 225.0); + assert_eq!(cursor_at(&mut h, overlap), egui::CursorIcon::Move); + h.run_steps(2); + assert!(tip_says(&h, "Top note") && !tip_says(&h, "Bottom note"), "the tip names the comment drawn on top"); + mouse_click(&mut h, overlap); + assert_eq!(h.state().views[0].comments.selected, Some((0, 1)), "a click selects it too"); +} + +#[test] +fn a_comments_tip_shows_with_every_tool_beside_its_cursor() { + let mut h = open(comment_on_text_pdf(), "150", false); + // Inside the rectangle comment by "Reviewer": where there is no text, and over a word. + let (blank, on_text) = (at(&h, 120.0, 315.0), at(&h, 60.0, 303.0)); + let hover = |h: &mut Harness<'_, PdfCraftApp>, tool: QuickTool, p: egui::Pos2, want: egui::CursorIcon| { + h.state_mut().quick_tool = tool; + h.run_steps(4); + let cursor = wait_for_cursor(h, p, |c| c == want); + h.run_steps(2); + (cursor, tip_says(h, "Reviewer")) + }; + use egui::CursorIcon::{Crosshair, Move, Text}; + // A new position each time, so the hover is fresh. + let nudge = |n: f32| blank + egui::vec2(n, 0.0); + assert_eq!(hover(&mut h, QuickTool::Select, blank, Move), (Move, true), "the Select tool picks the comment up"); + let stamp = QuickTool::Stamp(StampKind::DynApproved); + assert_eq!(hover(&mut h, stamp, nudge(1.0), Crosshair), (Crosshair, true), "a stamp keeps its crosshair, and the comment still says whose it is"); + let rectangle = QuickTool::Comment(CommentTool::Rectangle); + assert_eq!(hover(&mut h, rectangle, nudge(2.0), Crosshair), (Crosshair, true), "so does a drawing tool"); + let highlight = QuickTool::Comment(CommentTool::Highlight); + assert_eq!(hover(&mut h, highlight, on_text, Text), (Text, true), "and the highlighter its I-beam"); + // Not while drawing freehand, where it would cover the next stroke (#429), nor with Fill & Sign. + let ink = QuickTool::Comment(CommentTool::Ink); + assert_eq!(hover(&mut h, ink, nudge(3.0), Crosshair), (Crosshair, false), "freehand"); + let fill = QuickTool::Fill(FillTool::Check); + assert_eq!(hover(&mut h, fill, nudge(4.0), Crosshair), (Crosshair, false), "Fill & Sign"); +} + /// Narrows the paragraph by dragging its left edge 100 pt inward, starting `dx` screen points /// left of the edge. Returns the cursor shown at the press point and the line count after. fn narrow_from_left(dx: f32) -> (egui::CursorIcon, usize) { diff --git a/crates/ui-egui/tests/it/signing.rs b/crates/ui-egui/tests/it/signing.rs index 0050f82b19..b37f134a1a 100644 --- a/crates/ui-egui/tests/it/signing.rs +++ b/crates/ui-egui/tests/it/signing.rs @@ -331,13 +331,25 @@ fn clicking_an_empty_signature_field_signs_it() { let dir = dir().join("field"); std::fs::create_dir_all(&dir).unwrap(); let mut h = harness(dir.clone()); + sign_the_empty_field(&mut h); + let s = h.state(); + let sig = s.session.get(s.views[1].id).unwrap().signatures.iter().find(|x| x.field == "Approver").cloned().unwrap(); + assert!(sig.signed && sig.visible); +} + +/// The centre of `WITH_FIELD`'s signature field, opened as the second document. +fn field_centre(h: &Harness<'static, PdfCraftApp>) -> Pos2 { + let r = h.state().views[1].page_screen_rect(0).expect("on screen"); + pos2(r.left() + 150.0 / 300.0 * r.width(), r.top() + (200.0 - 50.0) / 200.0 * r.height()) +} + +/// Opens `WITH_FIELD` as the second document and signs its empty field with the test ID, by +/// clicking the field. +fn sign_the_empty_field(h: &mut Harness<'static, PdfCraftApp>) { h.state_mut().open_bytes("form.pdf", None, WITH_FIELD.to_vec()).unwrap(); h.state_mut().active = Some(1); h.run_steps(6); - let p = { - let r = h.state().views[1].page_screen_rect(0).expect("on screen"); - pos2(r.left() + 150.0 / 300.0 * r.width(), r.top() + (200.0 - 50.0) / 200.0 * r.height()) - }; + let p = field_centre(h); h.hover_at(p); h.run_steps(1); h.drag_at(p); @@ -363,9 +375,39 @@ fn clicking_an_empty_signature_field_signs_it() { h.run_steps(2); h.get_by_label("Sign").click(); h.run_steps(6); - let s = h.state(); - let sig = s.session.get(s.views[1].id).unwrap().signatures.iter().find(|x| x.field == "Approver").cloned().unwrap(); - assert!(sig.signed && sig.visible); +} + +/// Whether the page's hover tip shows and says `text`. +fn tip_says(h: &Harness<'static, PdfCraftApp>, text: &str) -> bool { + let layer = egui::LayerId::new(egui::Order::Tooltip, egui::Id::unique("canvas-hover")); + let Some(tip) = h.ctx.memory(|m| m.area_rect(egui::Id::unique("canvas-hover")).filter(|_| m.areas().is_visible(&layer))) else { return false }; + h.query_all_by_label_contains(text).any(|n| tip.contains(n.rect().center())) +} + +/// A signed field's validation badge is the viewer's own mark, not part of the page: its verdict +/// is the tip with any tool, beside that tool's cursor. +#[test] +fn a_signature_badge_says_its_verdict_with_any_tool() { + let dir = dir().join("badge"); + std::fs::create_dir_all(&dir).unwrap(); + let mut h = harness(dir.clone()); + sign_the_empty_field(&mut h); + let verdict = { + let s = h.state(); + let sig = s.session.get(s.views[1].id).unwrap().signatures.iter().find(|x| x.field == "Approver").cloned().unwrap(); + sig.summary() + }; + let p = field_centre(&h); + let hover = |h: &mut Harness<'static, PdfCraftApp>, tool: QuickTool, at: Pos2| { + h.state_mut().quick_tool = tool; + h.run_steps(4); + h.hover_at(at); + h.run_steps(4); + (h.output().platform_output.cursor_icon, tip_says(h, verdict)) + }; + assert!(hover(&mut h, QuickTool::Select, p).1, "the Select tool shows the verdict"); + let stamp = QuickTool::Stamp(pdfcraft_engine::StampKind::DynApproved); + assert_eq!(hover(&mut h, stamp, p + egui::vec2(1.0, 0.0)), (egui::CursorIcon::Crosshair, true), "so does a stamp, beside its crosshair"); } #[test] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 5e1c4a7e75..45a107ab94 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1202,8 +1202,8 @@ milestone = "M3" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/it/comments.rs::hovering_a_comment_shows_its_author_and_text", "crates/ui-egui/tests/it/comments.rs::multiline_markup_hover_stays_on_marked_lines", "crates/ui-egui/tests/it/comments.rs::neighboring_markup_hover_changes_to_its_own_comment", "crates/ui-egui/tests/it/comments.rs::overlapping_markup_hover_shows_the_topmost_comment", "crates/ui-egui/tests/it/comments.rs::overlapping_markup_hover_uses_annotation_index_with_different_top_edges", "crates/ui-egui/tests/it/comments.rs::markup_hover_without_quads_keeps_the_rectangle_fallback", "crates/ui-egui/tests/it/comments.rs::markup_hover_skips_popup_containers", "crates/ui-egui/tests/it/comments.rs::preference_hides_author_only_highlight_hover_previews"] -notes = "A popup with the author and comment text. Text markup uses selection's per-quad screen bounds, including page and view rotation, so unmarked parts of a multiline Rect do not claim the hover (#3541). The last-painted eligible comment wins where annotations overlap (#3620), using the original /Annots index independently of spatial inspection order. Missing complete quads retain the Rect fallback; author-only highlight hiding and dialog suppression remain. Bounds are rectangular, not exact hit testing of skewed quadrilaterals. Pointer feedback is UI-only, reachable through the opt-in control channel, with no document automation tool." +evidence = ["crates/ui-egui/tests/it/comments.rs::hovering_a_comment_shows_its_author_and_text", "crates/ui-egui/tests/it/pointer.rs::a_comments_tip_shows_with_every_tool_beside_its_cursor", "crates/ui-egui/tests/it/comments.rs::multiline_markup_hover_stays_on_marked_lines", "crates/ui-egui/tests/it/comments.rs::neighboring_markup_hover_changes_to_its_own_comment", "crates/ui-egui/tests/it/comments.rs::overlapping_markup_hover_shows_the_topmost_comment", "crates/ui-egui/tests/it/comments.rs::overlapping_markup_hover_uses_annotation_index_with_different_top_edges", "crates/ui-egui/tests/it/comments.rs::markup_hover_without_quads_keeps_the_rectangle_fallback", "crates/ui-egui/tests/it/comments.rs::markup_hover_skips_popup_containers", "crates/ui-egui/tests/it/comments.rs::preference_hides_author_only_highlight_hover_previews"] +notes = "A popup with the author and comment text, as Acrobat shows it: with every tool, beside that tool's own cursor (Acrobat does the same with Select, Hand, Add comment, Highlight, Rectangle and Stamp); not with Fill & Sign, while drawing freehand (#429) or while a comment is moved, resized or drawn. Text markup uses selection's per-quad screen bounds, including page and view rotation, so unmarked parts of a multiline Rect do not claim the hover (#3541). The last-painted eligible comment wins where annotations overlap (#3620), using the original /Annots index independently of spatial inspection order. Missing complete quads retain the Rect fallback; author-only highlight hiding and dialog suppression remain. Bounds are rectangular, not exact hit testing of skewed quadrilaterals. Still missing: Acrobat's hover outline on a highlight with every tool, and its hit on an unfilled shape's border only. Pointer feedback is UI-only, reachable through the opt-in control channel, with no document automation tool." [[feature]] id = "view.page-size-hover" From 61f92a1d0c9364283a90349ebfa615e55dcb880a Mon Sep 17 00:00:00 2001 From: alehel Date: Mon, 19 Oct 2026 19:34:27 +0200 Subject: [PATCH 2/4] test(ui): page modules claim the cursor and never set it A sibling of page_modules_read_the_pointer_only_through_page_pointer in page_pointer.rs, sharing its scan of the page modules (now `misuses`, which both tests call): it fails on set_cursor_icon, a `.cursor_icon = ` write to egui's output, on_hover_cursor and on_hover_and_drag_cursor in a page module, naming the file and line and pointing to the claims in src/page_cursor.rs. Only the resolver, which is not a page module, sets the pages' cursor. NOT_PAGE_CURSOR allows the three writers that are not the pages' cursor: the signature dialog's Scan tab (fill_sign::scan_tab), the paragraph editor's frame (edit_text_ui::overlay, an Area of its own drawn after the pages' cursor is set) and the Calculation order panel, whose rows are dragged to reorder them (calc_order_ui::panel). An entry that sets no cursor any more fails too. Seen failing with a set_cursor_icon put back into canvas.rs's link loop, an output_mut cursor write in forms_ui, an on_hover_cursor in comments and the Scan tab's write removed: one message each. The pointer guard still fails on a hover_pos() read put into crop.rs. Rebased onto main's newer page modules, it named one write before the panel's entry was added, calc_order_ui.rs's Grabbing: main's other new cursors are claims now. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/page_pointer.rs | 80 ++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/crates/ui-egui/src/page_pointer.rs b/crates/ui-egui/src/page_pointer.rs index a6a318c1fd..c6baf70908 100644 --- a/crates/ui-egui/src/page_pointer.rs +++ b/crates/ui-egui/src/page_pointer.rs @@ -346,22 +346,33 @@ mod tests { rest.find(|c: char| !(c.is_alphanumeric() || c == '_')).map(|end| &rest[..end]) } - /// Whether `line` calls `read`, and not a longer name that ends in it. - fn reads(line: &str, read: &str) -> bool { - line.match_indices(read).any(|(at, _)| !line[..at].ends_with(|c: char| c.is_alphanumeric() || c == '_')) + /// Ways to set egui's cursor. + const CURSOR_WRITES: [&str; 4] = ["set_cursor_icon(", ".cursor_icon = ", "on_hover_cursor(", "on_hover_and_drag_cursor("]; + + /// Functions in the page modules that set a cursor other than the pages': (file, function, + /// why). + const NOT_PAGE_CURSOR: [(&str, &str, &str); 3] = [ + ("fill_sign.rs", "scan_tab", "the signature dialog's Scan tab"), + ("edit_text_ui.rs", "overlay", "the paragraph editor is an Area of its own over the page, drawn after the pages' cursor is set"), + ("calc_order_ui.rs", "panel", "the Calculation order panel, whose rows are dragged to reorder them"), + ]; + + /// Whether `line` uses `name`, and not a longer name that ends in it (a field, after its + /// `.`, is never part of a longer name). + fn uses(line: &str, name: &str) -> bool { + line.match_indices(name).any(|(at, _)| name.starts_with('.') || !line[..at].ends_with(|c: char| c.is_alphanumeric() || c == '_')) } - /// Page modules read the pointer only through `PagePointer`. egui's raw state misses what is - /// drawn over the pages and where a touch lifted, so a raw read in a page module brings back - /// the bugs `PagePointer` fixed: a cursor or a press through a dialog, a tap that does nothing. - #[test] - fn page_modules_read_the_pointer_only_through_page_pointer() { + /// Each use of one of `names` in the page modules' code (their tests left out) outside the + /// functions `allowed` to, as "src/file:line: `name` why"; then each allowed function that + /// uses none of them any more, as "src/file: fn function unused". + fn misuses(names: &[&str], allowed: &[(&str, &str, &str)], why: &str, unused: &str) -> Vec { let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut problems = Vec::new(); - let mut needed = vec![false; NOT_PAGE_INPUT.len()]; + let mut needed = vec![false; allowed.len()]; for file in PAGE_MODULES { let source = std::fs::read_to_string(src.join(file)).expect("page module").replace("\r\n", "\n"); - // Tests drive egui's input themselves. + // Tests drive egui's input and read its output themselves. let code = source.split("#[cfg(test)]\nmod ").next().unwrap_or_default(); // The top-level function, or method of an `impl`, that each line is in. let (mut function, mut in_impl) = (None, false); @@ -376,24 +387,36 @@ mod tests { if line.trim_start().starts_with("//") { continue; } - for read in RAW_READS.into_iter().filter(|read| reads(line, read)) { - match NOT_PAGE_INPUT.iter().position(|(f, name, _)| *f == file && function == Some(*name)) { + for name in names.iter().filter(|name| uses(line, name)) { + match allowed.iter().position(|(f, function_name, _)| *f == file && function == Some(*function_name)) { Some(k) => needed[k] = true, - None => problems.push(format!( - "src/{file}:{}: `{read}` reads egui's pointer state in a page module. Read `PagePointer` \ - (src/page_pointer.rs) instead: it knows what is drawn over the pages and where a touch \ - lifted. Not page input? Add the function to NOT_PAGE_INPUT there, with why.", - n + 1 - )), + None => problems.push(format!("src/{file}:{}: `{name}` {why}", n + 1)), } } } } - for ((file, name, _), needed) in NOT_PAGE_INPUT.iter().zip(needed) { + for ((file, name, _), needed) in allowed.iter().zip(needed) { if !needed { - problems.push(format!("src/{file}: fn {name} reads no pointer state any more: remove it from NOT_PAGE_INPUT.")); + problems.push(format!("src/{file}: fn {name} {unused}")); } } + problems + } + + /// Page modules read the pointer only through `PagePointer`. egui's raw state misses what is + /// drawn over the pages and where a touch lifted, so a raw read in a page module brings back + /// the bugs `PagePointer` fixed: a cursor or a press through a dialog, a tap that does nothing. + #[test] + fn page_modules_read_the_pointer_only_through_page_pointer() { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut problems = misuses( + &RAW_READS, + &NOT_PAGE_INPUT, + "reads egui's pointer state in a page module. Read `PagePointer` (src/page_pointer.rs) instead: it knows \ + what is drawn over the pages and where a touch lifted. Not page input? Add the function to NOT_PAGE_INPUT \ + there, with why.", + "reads no pointer state any more: remove it from NOT_PAGE_INPUT.", + ); // A module that takes the page pointer is a page module, so it is checked too. let mut dirs = vec![src.clone()]; while let Some(dir) = dirs.pop() { @@ -416,4 +439,21 @@ mod tests { } assert!(problems.is_empty(), "{}", problems.join("\n")); } + + /// Page modules claim the cursor and never set it. Where each set it, the last write of a + /// frame won, so the order of the per-page chain decided the cursor and not what a press + /// does: a link's hand over every tool, the I-beam over a comment that a drag moves. + #[test] + fn page_modules_claim_the_cursor_and_never_set_it() { + let problems = misuses( + &CURSOR_WRITES, + &NOT_PAGE_CURSOR, + "sets the cursor in a page module, where the last write of a frame would win. Claim it instead, on the \ + layer of what is under the pointer (`view.cursor.claim`, src/page_cursor.rs): `document_area` sets it \ + once, from the claim on the highest layer the active tool reaches. Not the pages' cursor? Add the \ + function to NOT_PAGE_CURSOR in src/page_pointer.rs, with why.", + "sets no cursor any more: remove it from NOT_PAGE_CURSOR.", + ); + assert!(problems.is_empty(), "{}", problems.join("\n")); + } } From 5815dbfb794bec4dc5bb7dfa449cd36801171eff Mon Sep 17 00:00:00 2001 From: alehel Date: Mon, 19 Oct 2026 19:34:27 +0200 Subject: [PATCH 3/4] docs: page modules claim the cursor, never set it A "Page cursor" non-negotiable in CLAUDE.md, under "Page input": page modules claim the cursor for what is under the pointer (view.cursor.claim, crates/ui-egui/src/page_cursor.rs) and never call set_cursor_icon, write cursor_icon or use on_hover_cursor; document_area sets it once, from the claim on the highest layer the active tool reaches. It names the test that enforces it, page_pointer::tests::page_modules_claim_the_cursor_and_never_set_it. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index f51a96d56e..944479805b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,6 +22,7 @@ PdfCraft is a clean-room, open-source, Rust-native PDF application targeting Ado - **Privacy.** When observing Acrobat, use synthetic fixtures only. Never capture the Home view, recent files or account info. Capture by window id (`plan/acrobat/tools/`). Never commit Acrobat outputs, corpus files or personal data. - **Layering.** Nothing below L7 depends on egui/winit/eframe/rfd (`plan/architecture.md` §3). `cos`/`filters`/`crypt`/`arlington` stay standalone. - **Page input.** Page modules read `PagePointer` (`crates/ui-egui/src/page_pointer.rs`), never egui's raw pointer state (`hover_pos`, `interact_pos`, `latest_pos`, `press_origin`, button reads); `page_pointer::tests::page_modules_read_the_pointer_only_through_page_pointer` enforces it. +- **Page cursor.** Page modules claim the cursor for what is under the pointer (`view.cursor.claim(Layer::…, icon)`, `crates/ui-egui/src/page_cursor.rs`), never set it (`set_cursor_icon`, `cursor_icon =`, `on_hover_cursor`); `document_area` sets it once, from the claim on the highest layer the active tool reaches. `page_pointer::tests::page_modules_claim_the_cursor_and_never_set_it` enforces it. - **Fidelity.** The PDF object graph is the model. Preserve unknown data. Saves are incremental unless a full rewrite is required. Never silently drop or repair data without recording it. - **Never panic.** Every PDF, script, MCP call, settings file and keystroke is untrusted input, and none of it may crash the app or lose the user's work. This outranks feature work. Return `Result` (the crate's error enum) and propagate with `?`, or fall back leniently and record it. In non-test code: - No `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` unless it is provably infallible, with a comment saying why. From c14910b182e9dd9d8bf574475b2b64e3b4ff9973 Mon Sep 17 00:00:00 2001 From: alehel Date: Wed, 28 Oct 2026 11:55:53 +0100 Subject: [PATCH 4/4] docs(ui): a link's tip shows where a press acts on it, with Select or Link Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas/page_cursor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/canvas/page_cursor.rs b/crates/ui-egui/src/canvas/page_cursor.rs index d8ae61bc39..382f5efb63 100644 --- a/crates/ui-egui/src/canvas/page_cursor.rs +++ b/crates/ui-egui/src/canvas/page_cursor.rs @@ -6,10 +6,10 @@ //! showed over every tool though only the Select tool follows links, and the I-beam of the text //! under a comment hid that a drag there moves the comment. `canvas::document_area` sets the //! cursor once, from the claim on the highest layer the active tool reaches. The tip beside it is -//! that claim's own (a link's target, so only where a press follows the link), or else what lies -//! under the pointer says (a comment, or a signature's validation badge), with any tool, as -//! Acrobat shows a comment's tooltip. A test in `page_pointer/tests.rs` keeps page modules from -//! setting the cursor. +//! that claim's own (a link's target, so only where a press acts on the link: the Select tool +//! follows it, the Link tool edits it), or else what lies under the pointer says (a comment, or a +//! signature's validation badge), with any tool, as Acrobat shows a comment's tooltip. A test in +//! `page_pointer/tests.rs` keeps page modules from setting the cursor. use egui::CursorIcon;