diff --git a/CLAUDE.md b/CLAUDE.md index 47a80b86bc..5c7034b7a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,7 @@ PdfCraft is a clean-room, open-source, Rust-native PDF application targeting Ado - **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/canvas/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. -- **Page presses.** A page module decides what of its own is under a point in one hit test, which returns a `Hit` (owner, layer, cursor, tip; `crates/ui-egui/src/canvas/page_router.rs`) and gives its cursor, its tip and its press. It acts on a press only when the router says the press is its own: the page view tells it what the press is to its page (`PagePress`: its own, nobody's, or another page's), and it never tests the press point again or asks another module whether it wants it. Escape cancels any press the pages hold (`cancel_on_escape`), and that press is ignored until the button is up. `page_pointer::tests::page_modules_act_on_a_press_only_through_the_router` enforces it. +- **Page presses.** A page module decides what of its own is under a point in one hit test, which returns a `Hit` (owner, layer, cursor, tip; `crates/ui-egui/src/canvas/page_router.rs`) and gives its cursor, its tip and its press. It acts on a press only when the press is its own or, for the active tool, nobody's: the page view tells it which (`PagePress`: its own, nobody's, or another page's), and it never tests the press point again or asks another module whether it wants it. Escape cancels any press the pages hold (`cancel_on_escape`), and that press is ignored until the button is up. `page_pointer::tests::page_modules_act_on_a_press_only_through_the_router` 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. diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index 4a9ed3a54e..647a39d970 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -201,9 +201,12 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut view.links.context = Some((i, index)); } // Comments, every frame: their own presses, a commenting tool's, and a click elsewhere that - // leaves them unselected. - let on_comment = comments::page_input(ui, resp, &pcx, press, view); - let consumed = on_objects || on_edit_text || on_link || on_content || boxing || on_field || on_comment; + // leaves them unselected. A press nobody holds that another tool took this frame (Prepare's + // selection rectangle, added content, which still decide for themselves) is not also a + // commenting tool's, as before. + let taken = on_objects || on_edit_text || on_link || on_content || boxing || on_field; + let on_comment = comments::page_input(ui, resp, &pcx, press, taken, view); + let consumed = taken || on_comment; let hover = pointer.and_then(|p| claim_under(&hits, view, p, frame.env.unobstructed)); PageInput { consumed, area_tool, hover, released_over } } @@ -324,33 +327,36 @@ fn tool_input( } } -/// A right-click at `p`: it goes to what lies on top there ([`PageHits::context_at`]), which the -/// page's menu then acts on. A comment there is selected, and none elsewhere; a Prepare field is -/// selected; links take theirs from what this returns. +/// A right-click at `p`: the page's menu opens there, and it goes to what lies on top there +/// ([`PageHits::context_at`]), which the menu then acts on. A comment there is selected, and none +/// elsewhere; a Prepare field is selected; links take theirs from what this returns. /// `under`: the hits at `p` before the tools act. `press_held`: the primary button is held, and the /// selections its press acts on are left alone until its release. fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mut DocView, p: Pos2, press_held: bool) -> Option { let context = hits.context_at(view, p, under); - // Add text takes every right-click on a page it can change, Edit text those on anything of its - // own (paragraphs, pictures, added content, the objects selected together and their grips), the - // Link tool those on a link, and a Prepare field tool the whole page, as before: comments see - // none of them. + // Delete page, Paste and Copy Image act where the menu opened, whoever takes the right-click. + comments::anchor(hits.pcx, p, view); + if press_held { + return context; + } + // Add text takes every right-click on a page it can change, Edit text those on its own + // paragraphs and pictures (the objects selected together, and crop mode's picture, among them), + // the Link tool those on a link, and a Prepare field tool the whole page, as before: comments + // see none of them. let tool_takes_it = match hits.tool { - QuickTool::AddText => hits.can_modify || top(under.iter().cloned(), hits.tool).is_some(), - QuickTool::EditText => top(under.iter().cloned(), hits.tool).is_some(), + QuickTool::AddText => hits.can_modify, + QuickTool::EditText => under.iter().any(|h| matches!(h.owner, Owner::Paragraph { .. } | Owner::Picture { .. } | Owner::Objects { .. })), QuickTool::Link => hits.can_modify && matches!(context, Some(Owner::Link { .. })), _ => hits.field_tool.is_some(), }; - if !tool_takes_it && !press_held { + if !tool_takes_it { let comment = match context { Some(Owner::Comment { index, .. }) => Some(index), _ => None, }; - comments::context(hits.pcx, p, comment, view); + comments::context(hits.pcx, comment, view); } - if let Some(Owner::PrepareField { field, widget }) = context - && !press_held - { + if let Some(Owner::PrepareField { field, widget }) = context { crate::prepare::context(hits.form, field, widget, view); } context diff --git a/crates/ui-egui/src/canvas/page_router.rs b/crates/ui-egui/src/canvas/page_router.rs index be83411072..c689f9fde5 100644 --- a/crates/ui-egui/src/canvas/page_router.rs +++ b/crates/ui-egui/src/canvas/page_router.rs @@ -119,7 +119,7 @@ pub(crate) enum PagePress { /// tool's. Also every page's while a press that began off the pages is held, and after the /// press was dropped ([`PageRouter::cancel`]) until the button is up. Free, - /// The press began on another page, or between the pages. + /// The press began on another page, or off the pages (between them, or beside one). Elsewhere, } diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index c0a797fa67..82b340e571 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -2047,3 +2047,14 @@ fn cancelling_pointer_gestures_lets_go_of_a_camera_corner() { v.cancel_pointer_gestures(); assert_eq!(v.camera.map(|q| q.grab), Some(None)); } + +/// A camera corner at the page's edge is hit on the page only: a press off the page is no page's, +/// so its grab cursor does not show there. +#[test] +fn a_camera_corner_at_the_page_edge_is_hit_on_the_page_only() { + let mut v = view(1, PageLayout::Single, true); + v.camera = Some(crate::camera_ui::CameraQuad { page: 0, corners: [[0.0, 0.0], [300.0, 0.0], [300.0, 400.0], [0.0, 400.0]], grab: None }); + let xf = PageXform { rect: Rect::from_min_size(pos2(10.0, 20.0), egui::vec2(300.0, 400.0)), rot: 0, pw: 300.0, ph: 400.0 }; + assert!(crate::camera_ui::hit(&xf, 0, &v, pos2(14.0, 24.0)).is_some(), "on the page"); + assert!(crate::camera_ui::hit(&xf, 0, &v, pos2(6.0, 16.0)).is_none(), "just off it"); +} diff --git a/crates/ui-egui/src/tools/camera_ui.rs b/crates/ui-egui/src/tools/camera_ui.rs index a7e7d6c934..c220f8aea3 100644 --- a/crates/ui-egui/src/tools/camera_ui.rs +++ b/crates/ui-egui/src/tools/camera_ui.rs @@ -61,10 +61,11 @@ fn screen(xf: &PageXform, c: [f32; 2]) -> Pos2 { } /// The corner of the quadrilateral under `p` on `page`, the nearest within reach: the one hit test -/// of the camera tool, for the cursor and the press. `None` once the page is enhanced, or on a page -/// without the quadrilateral. +/// of the camera tool, for the cursor and the press. `None` once the page is enhanced, on a page +/// without the quadrilateral, or off the page (a press there is no page's), though a corner at its +/// edge reaches past it. pub(crate) fn hit(xf: &PageXform, page: usize, view: &DocView, p: Pos2) -> Option { - if view.camera_done.is_some() { + if view.camera_done.is_some() || !xf.rect.contains(p) { return None; } let q = view.camera.filter(|q| q.page == page)?; diff --git a/crates/ui-egui/src/tools/comments.rs b/crates/ui-egui/src/tools/comments.rs index 83ee620ae1..5ba9ca5401 100644 --- a/crates/ui-egui/src/tools/comments.rs +++ b/crates/ui-egui/src/tools/comments.rs @@ -37,7 +37,7 @@ pub(crate) use geometry::{corner_ratio, page_rotation, resized}; pub use geometry::{drawn_text_box_rect, text_box_rect}; #[cfg(test)] use input::hover_tip; -pub(crate) use input::{context, context_hit, describe, hit, keys, nudge_keys, page_after_text, page_input}; +pub(crate) use input::{anchor, context, context_hit, describe, hit, keys, nudge_keys, page_after_text, page_input}; pub use menu::{CanvasAction, fill_picker, recent_custom_colours, remember_custom_colour, status_badge, swatch_grid}; #[cfg(test)] use menu::{RECENT_CUSTOM, same_colour}; diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index 4c026604ea..ccb12c7676 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -11,12 +11,13 @@ use crate::page_router::{CommentPart, Hit, Owner, PagePress}; /// Comment input on one page, before text selection runs. `press` is what this page has to do with /// the press the pages hold: a press on a comment where the tool reaches it ([`hit`]: the Select /// tool, Fill & Sign over its marks, a shape tool over the selected shape's handles) is that -/// comment's, and one nobody holds is a commenting tool's. Returns `true` when the pointer gesture belongs to -/// commenting (text selection must ignore it). -pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, press: PagePress, view: &mut DocView) -> bool { +/// comment's, and one nobody holds is a commenting tool's, unless another tool took it this frame +/// (`taken`: Prepare's selection rectangle, added content), though a click there still unselects. +/// Returns `true` when the pointer gesture belongs to commenting (text selection must ignore it). +pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, press: PagePress, taken: bool, view: &mut DocView) -> bool { let (pointer, origin) = (cx.ptr.pos, cx.ptr.origin); let page_rect = cx.xf.rect; - let free = press == PagePress::Free; + let free = press == PagePress::Free && !taken; let pressed_here = free && origin.is_some_and(|o| page_rect.contains(o)); let over_page = pointer.is_some_and(|p| page_rect.contains(p)); let owned = match press { @@ -98,11 +99,15 @@ pub(crate) fn hit(cx: &PageCx<'_>, view: &DocView, p: Pos2, on_link: bool) -> Op } let markup = is_markup(&a.subtype); let icon = if cx.allowed && !markup { egui::CursorIcon::Move } else { egui::CursorIcon::PointingHand }; - // What a drag moves whole (a note, a shape, a signature, a Fill & Sign mark) is placed on the - // page, over a form field; text markup lies on the text, under one. - let layer = if a.fill_sign || !markup { Layer::Placed } else { Layer::Comment }; // Text markup does not move: a click selects it, a drag across it selects the text. - Some(Hit { declines_drag: markup, ..Hit::new(Owner::Comment { index: a.index, part: CommentPart::Body }, layer, icon) }) + Some(Hit { declines_drag: markup, ..Hit::new(Owner::Comment { index: a.index, part: CommentPart::Body }, layer(a), icon) }) +} + +/// The layer a comment's body lies on: what a drag moves whole (a note, a shape, a signature, a +/// Fill & Sign mark) is placed on the page, over a form field; text markup lies on the text, under +/// one. +fn layer(a: &Annotation) -> Layer { + if a.fill_sign || !is_markup(&a.subtype) { Layer::Placed } else { Layer::Comment } } /// The comment a right-click at `p` is on: the topmost, with every tool, as a comment's menu shows @@ -113,14 +118,18 @@ pub(crate) fn context_hit(cx: &PageCx<'_>, p: Pos2) -> Option { return None; } let a = cx.comment_at(p)?; - let layer = if a.fill_sign || !is_markup(&a.subtype) { Layer::Placed } else { Layer::Comment }; - Some(Hit::new(Owner::Comment { index: a.index, part: CommentPart::Body }, layer, egui::CursorIcon::Default)) + Some(Hit::new(Owner::Comment { index: a.index, part: CommentPart::Body }, layer(a), egui::CursorIcon::Default)) } -/// A right-click at `p` on this page: the page's menu is anchored there (Copy Image), and the -/// comment it is on ([`context_hit`], when it wins there) is selected, or none. -pub(crate) fn context(cx: &PageCx<'_>, p: Pos2, index: Option, view: &mut DocView) { +/// A right-click at `p` on this page: the page's menu is anchored there (Delete page, Paste, Copy +/// Image, a note added here). +pub(crate) fn anchor(cx: &PageCx<'_>, p: Pos2, view: &mut DocView) { view.comments.context_at = Some((cx.page, cx.to_user(p))); +} + +/// A right-click on comment `index` of this page ([`context_hit`], when it wins there) selects it, +/// and one on anything else selects none. +pub(crate) fn context(cx: &PageCx<'_>, index: Option, view: &mut DocView) { view.comments.selected = index.map(|i| (cx.page, i)); } diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index 6685085614..d412496fcf 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1419,7 +1419,8 @@ fn the_link_tools_hand_stops_at_the_pages_edge() { } /// With Edit text a right-click on a paragraph is Edit text's, as before, though a comment lies -/// over it: the comment is not selected. +/// over it: the comment is not selected. The page's menu is anchored there all the same (Paste on +/// the picture menu, Delete page); it used to stay where the last right-click comments saw was. #[test] fn a_right_click_with_edit_text_on_a_paragraph_under_a_comment_selects_no_comment() { let mut h = open(comment_on_text_pdf(), "150", false); @@ -1429,6 +1430,7 @@ fn a_right_click_with_edit_text_on_a_paragraph_under_a_comment_selects_no_commen wait_for_cursor(&mut h, on_text, |c| c != egui::CursorIcon::Default); right_click(&mut h, on_text); assert_eq!(h.state().views[0].comments.selected, None); + assert_eq!(h.state().views[0].comments.context_at.map(|(page, _)| page), Some(0), "the menu is anchored on the page"); } /// With a Prepare field tool a right-click is the tool's, as before: a comment under it is not @@ -1544,3 +1546,46 @@ fn a_long_press_on_a_comment_selects_it_for_its_menu() { one_frame(&mut h, [touch(egui::TouchPhase::End, top), button(top, false), egui::Event::PointerGone]); h.run_steps(2); } + +/// A right-click while the primary button is held anchors the page's menu where it is (Delete page, +/// Paste): the held press leaves only the selections alone. It used to keep the menu on the page +/// right-clicked before. +#[test] +fn a_right_click_with_the_button_held_anchors_the_menu_on_its_page() { + let mut h = open(two_pages_pdf(), "100", false); + h.run_steps(2); + h.state_mut().views[0].comments.context_at = Some((0, [10.0, 10.0])); + let two = h.state().views[0].page_screen_rect(1).expect("page 2 on screen"); + let press = two.min + egui::vec2(50.0, 30.0); + h.hover_at(press); + h.run_steps(1); + h.drag_at(press); + h.run_steps(1); + h.hover_at(press + egui::vec2(40.0, 0.0)); + h.run_steps(1); + let menu = press + egui::vec2(100.0, 60.0); + for pressed in [true, false] { + h.event(egui::Event::PointerButton { pos: menu, button: egui::PointerButton::Secondary, pressed, modifiers: egui::Modifiers::NONE }); + } + h.run_steps(1); + assert_eq!(h.state().views[0].comments.context_at.map(|(page, _)| page), Some(1), "the menu is page 2's"); + h.drop_at(press + egui::vec2(40.0, 0.0)); + h.run_steps(2); +} + +/// With Edit text a right-click on added text reaches comments, as before (Edit text keeps those on +/// its own paragraphs and pictures): the selected comment is unselected. +#[test] +fn a_right_click_with_edit_text_on_added_text_unselects_the_comment() { + let mut h = open(comment_on_text_pdf(), "150", false); + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { + page: 0, + text: pdfcraft_engine::AddedText { rect: [40.0, 195.0, 140.0, 210.0], text: "Added".into(), ..Default::default() }, + })); + h.state_mut().quick_tool = QuickTool::EditText; + h.run_steps(4); + h.state_mut().views[0].comments.selected = Some((0, 0)); + let on_added = at(&h, 90.0, 202.0); + right_click(&mut h, on_added); + assert_eq!(h.state().views[0].comments.selected, None); +} diff --git a/crates/ui-egui/tests/it/prepare.rs b/crates/ui-egui/tests/it/prepare.rs index 281e3d9e18..51999431d9 100644 --- a/crates/ui-egui/tests/it/prepare.rs +++ b/crates/ui-egui/tests/it/prepare.rs @@ -1599,3 +1599,21 @@ fn a_right_click_during_a_field_drag_moves_the_dragged_field() { assert_eq!(field_rect(&h, "city"), city, "the field right-clicked stays where it was"); assert_ne!(field_rect(&h, "name"), name, "the dragged field moved"); } + +/// With Prepare open, a drag with a shape tool on the page's empty part draws Prepare's selection +/// rectangle and no comment, as before: Prepare takes the press nobody holds. It used to draw both. +#[test] +fn with_prepare_open_a_shape_tool_drag_draws_no_comment() { + let mut h = harness(); + h.state_mut().execute("form.prepare"); + h.run_steps(3); + assert!(h.state_mut().execute("comment.square")); + h.run_steps(3); + assert!(h.state().is_preparing(), "picking the tool leaves Prepare open"); + // Below the fields. + let (from, to) = (at(&h, 40.0, 260.0), at(&h, 200.0, 360.0)); + drag(&mut h, from, to); + let s = h.state(); + let squares = s.session.get(s.views[0].id).unwrap().info.annotations.iter().filter(|a| a.subtype == "Square").count(); + assert_eq!(squares, 0, "no rectangle under Prepare's selection rectangle"); +}