diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index a933c7acc5..8a1aa266e9 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2792,7 +2792,8 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { None => {} } // No dialog, close prompt or palette over the page: only then does page input count. - let unobstructed = !app.modal_open() && !app.palette_open; + let modal = app.modal_open(); + let unobstructed = !modal && !app.palette_open; let organize_zoom = app.organize_zoom; let view = &mut app.views[index]; if view.organize { @@ -3250,9 +3251,9 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let mut best_overlap = -1.0f32; let mut current_overlap = -1.0f32; let mut current_height = f32::INFINITY; - // The pointer as the pages see it: nothing over them, or a press they hold. The page - // modules read it rather than egui's raw pointer state. - let ptr = PagePointer::read(ui, &resp); + // The pointer as the pages see it: nothing over them (a dialog from the frame it opens), + // or a press they hold. The page modules read it rather than egui's raw pointer state. + let ptr = PagePointer::read(ui, &resp, modal); let pointer = ptr.pos; // A press on a page makes it the current one (as in Acrobat), so a page command with // no selection (Rotate, Delete) acts on the page the reader pointed at, not on a page diff --git a/crates/ui-egui/src/page_pointer.rs b/crates/ui-egui/src/page_pointer.rs index df373c1547..baf0e29613 100644 --- a/crates/ui-egui/src/page_pointer.rs +++ b/crates/ui-egui/src/page_pointer.rs @@ -6,6 +6,9 @@ //! a touch or a Windows pen lifts: egui-winit sends `PointerGone` with every touch release, and //! winit reports Windows pen input as touch. So page modules read [`PagePointer`], never egui's //! pointer positions or button states; a test below keeps it that way. +//! +//! Dialogs are drawn after the pages, so on the frame one opens egui has not met its layer yet: +//! the reading also takes whether one is open. use egui::{Event, PointerButton, Pos2, Rect}; @@ -24,8 +27,9 @@ const MAX_EDGES: usize = 8; #[derive(Clone, Debug, Default, PartialEq)] pub(crate) struct PagePointer { /// Where the pointer is, while the pages are what egui has under it, or while they hold a - /// press (until the frame of its release, past a page's edge included). It is egui's - /// `interact_pos`, which still has the position on the frame a touch lifts. + /// press (until the frame of its release, past a page's edge included). With a dialog open + /// only a drag the pages already have keeps it. It is egui's `interact_pos`, which still has + /// the position on the frame a touch lifts. pub pos: Option, /// Where the press the pages hold began, also on the frame of its release when the press /// came in that same frame (egui has forgotten it by then). `None` for a press that began on @@ -40,18 +44,22 @@ pub(crate) struct PagePointer { } impl PagePointer { - /// The pointer for `resp`, the content widget that holds every page, allocated in `ui`. - pub(crate) fn read(ui: &egui::Ui, resp: &egui::Response) -> Self { + /// The pointer for `resp`, the content widget that holds every page, allocated in `ui`, while + /// a dialog is open over the window or not (`modal`). + pub(crate) fn read(ui: &egui::Ui, resp: &egui::Response, modal: bool) -> Self { let (at, origin, edges, primary_down, any_down) = ui.input(|i| (i.pointer.interact_pos(), i.pointer.press_origin(), edges(&i.events), i.pointer.primary_down(), i.pointer.any_down())); // egui's own answer to "do the pages hold a press?": from the press on until the frame // of its click, release or drag stop. A drag can outlive egui's press while the - // pointer is outside the window. - let held = resp.dragged() || resp.interact_pointer_pos().is_some(); + // pointer is outside the window. With a dialog open only a drag counts, up to the frame + // it stops: on the frame one opens egui still gives the pages a press that lands under + // it, and its click. + let held = resp.dragged() || resp.drag_stopped() || (!modal && resp.interact_pointer_pos().is_some()); // `contains_pointer` alone also holds a few points under the edge of an Area over the - // pages (egui's interact radius), so the top layer there must be the pages' own. + // pages (egui's interact radius), so the top layer there must be the pages' own. A + // dialog covers them from the frame it opens, before egui knows its layer. let layer = ui.layer_id(); - let on_top = |p: &Pos2| resp.contains_pointer() && ui.ctx().layer_id_at(*p).is_none_or(|l| l == layer); + let on_top = |p: &Pos2| !modal && resp.contains_pointer() && ui.ctx().layer_id_at(*p).is_none_or(|l| l == layer); let pos = at.filter(|p| held || on_top(p)); if !held { return Self { pos, origin: None, edges: Vec::new(), primary_down, any_down }; @@ -112,11 +120,16 @@ mod tests { /// Runs one frame of the pages' widget (and the Area over it, with `area`) and returns what /// the pages see. fn frame(ctx: &Context, events: Vec, area: bool) -> PagePointer { + frame_with(ctx, events, area, false) + } + + /// [`frame`], with a dialog open (`modal`) whose layer egui has not met. + fn frame_with(ctx: &Context, events: Vec, area: bool, modal: bool) -> PagePointer { let mut seen = PagePointer::default(); let input = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(400.0, 400.0))), events, ..Default::default() }; let mut output = ctx.run_ui(input, |ui| { let (_, resp) = ui.allocate_exact_size(PAGES.size(), Sense::click_and_drag()); - seen = PagePointer::read(ui, &resp); + seen = PagePointer::read(ui, &resp, modal); if area { egui::Area::new(egui::Id::new("over")).fixed_pos(OVER.min).show(ui.ctx(), |ui| { ui.allocate_exact_size(OVER.size(), Sense::hover()); @@ -194,6 +207,25 @@ mod tests { assert_eq!(frame(&ctx, vec![], true).pos, None, "then off the pages, the pointer is not theirs"); } + #[test] + fn a_dialog_hides_the_pointer_and_its_clicks_but_not_a_drag_the_pages_have() { + let ctx = Context::default(); + let p = pos2(50.0, 50.0); + frame(&ctx, vec![Event::PointerMoved(p)], false); + assert_eq!(frame(&ctx, vec![], false).pos, Some(p)); + assert_eq!(frame_with(&ctx, vec![], false, true).pos, None, "no pointer under a dialog, before egui knows its layer too"); + let click = frame_with(&ctx, vec![button(p, PointerButton::Primary, true), button(p, PointerButton::Primary, false)], false, true); + assert_eq!((click.pos, click.origin, click.edges.as_slice()), (None, None, [].as_slice()), "nor a click egui gives the pages then"); + // A drag the pages already have when a dialog opens goes on until its release. + frame(&ctx, vec![button(p, PointerButton::Primary, true)], false); + frame(&ctx, vec![Event::PointerMoved(pos2(80.0, 50.0))], false); + let q = pos2(110.0, 50.0); + let dragging = frame_with(&ctx, vec![Event::PointerMoved(q)], false, true); + assert_eq!((dragging.pos, dragging.origin), (Some(q), Some(p)), "a drag the pages have keeps its pointer under a dialog"); + let up = frame_with(&ctx, vec![button(q, PointerButton::Primary, false)], false, true); + assert_eq!(up.edges.as_slice(), [Edge::Release(q)].as_slice(), "and its release"); + } + #[test] fn a_secondary_press_is_some_button_down_but_no_primary_edge() { let ctx = Context::default(); diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index 9562353ac4..282f53dc02 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -11,7 +11,7 @@ use pdfcraft_engine::StampKind; use pdfcraft_ui_egui::comments::CommentTool; use pdfcraft_ui_egui::{PdfCraftApp, QuickTool, UnlockPrompt}; -use crate::support::input::{coalesced_click, double_click, touch_drag, touch_tap}; +use crate::support::input::{coalesced_click, double_click, one_frame, touch_drag, touch_tap}; /// A PDF of `objects`, numbered from 1 with the catalog first. fn pdf(objects: &[&str]) -> Vec { @@ -358,12 +358,20 @@ fn stamp_cursor_stays_off_the_chrome_above_a_page_scrolled_under_it() { assert_eq!(cursor_at(&mut h, egui::pos2(x, top + 40.0)), egui::CursorIcon::Crosshair); } +/// Whether the page's hover tip was drawn in the last frame. +fn tip_shown(h: &Harness<'_, PdfCraftApp>) -> bool { + let tip = egui::LayerId::new(egui::Order::Tooltip, egui::Id::new("canvas-hover")); + h.ctx.memory(|m| m.areas().is_visible(&tip)) +} + +/// Opens the Unlock prompt, a dialog with a backdrop over the whole window, from the next frame. +fn open_unlock_prompt(h: &mut Harness<'_, PdfCraftApp>) { + let doc = h.state().views[0].id; + h.state_mut().unlock_prompt = Some(UnlockPrompt { doc, name: "test.pdf".into(), input: String::new(), error: None, then: None }); +} + #[test] fn link_tip_stays_under_the_unlock_prompt() { - let tip_shown = |h: &Harness<'_, PdfCraftApp>| { - let tip = egui::LayerId::new(egui::Order::Tooltip, egui::Id::new("canvas-hover")); - h.ctx.memory(|m| m.areas().is_visible(&tip)) - }; let mut h = open(link_pdf(), "300", true); let page = page(&h); let k = page.width() / 300.0; @@ -373,8 +381,7 @@ fn link_tip_stays_under_the_unlock_prompt() { h.run_steps(2); assert!(tip_shown(&h), "the link's tip shows over an uncovered page"); - let doc = h.state().views[0].id; - h.state_mut().unlock_prompt = Some(UnlockPrompt { doc, name: "test.pdf".into(), input: String::new(), error: None, then: None }); + open_unlock_prompt(&mut h); h.hover_at(on_link + egui::vec2(1.0, 0.0)); for _ in 0..4 { h.run_steps(2); @@ -399,6 +406,34 @@ fn the_link_tip_lets_the_pointer_through_to_the_link() { assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::PointingHand, "the tip must not take the pointer from the link"); } +// Dialogs are drawn after the pages, so on the frame one opens egui does not know its layer yet +// and still gives the pages the pointer, and a press there. + +#[test] +fn the_link_tip_stays_under_a_dialog_on_the_frame_it_opens() { + let mut h = open(link_pdf(), "150", false); + let p = at(&h, 100.0, 340.0); + assert_eq!(cursor_at(&mut h, p), egui::CursorIcon::PointingHand); + h.run_steps(2); + assert!(tip_shown(&h), "the link's tip shows over an uncovered page"); + open_unlock_prompt(&mut h); + h.run_steps(1); + assert!(!tip_shown(&h), "the link's tip must not show over the Unlock prompt on the frame it opens"); +} + +#[test] +fn a_click_on_the_frame_a_dialog_opens_places_no_stamp() { + let mut h = open(link_pdf(), "150", false); + stamp_tool(&mut h); + let p = at(&h, 150.0, 150.0); + assert_eq!(cursor_at(&mut h, p), egui::CursorIcon::Crosshair); + open_unlock_prompt(&mut h); + let button = |pressed| egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + one_frame(&mut h, [button(true), button(false)]); + h.run_steps(4); + assert_eq!(stamps(&h), 0, "a click in the frame the Unlock prompt opens places no stamp under it"); +} + #[test] fn a_double_click_on_a_dialog_backdrop_leaves_page_text_alone() { let mut h = open(text_pdf(), "150", false);