From 866de03fd80a984417f51a9f79a7215e737feccb Mon Sep 17 00:00:00 2001 From: alehel Date: Thu, 22 Oct 2026 14:02:17 +0200 Subject: [PATCH] fix(ui): a dialog covers the pages from the frame it opens The page pointer has the pointer only where egui has the pages on top, and egui knows what is drawn over them from the layers it met in the frame before. Dialogs are drawn after the pages, so on the frame one opens egui has not met its layer yet. For that frame the pages kept the pointer, and a press that landed then was theirs: a link's tip showed over the dialog, and a click placed a stamp or followed a link under it. main's page_covered had kept the link's tip and click off the page while a dialog was open, from a hand-kept list. PagePointer::read now also takes whether a dialog is open: PdfCraftApp::modal_open, the list the page press and the edge scroll already check, the Unlock prompt included. With one open the pages see no pointer and no press, except a drag they already have, which goes on to its release. The palette is not a dialog and the page beside it keeps its tools, so it is left to egui's layers, from the frame after it opens. Tests: tests/it/pointer.rs hovers a link and opens the Unlock prompt (no tip in that frame), and clicks with the Stamp tool in the frame the prompt opens (no stamp). A page_pointer unit test hides the pointer and a click under a dialog, and keeps a drag the pages have until its release. With the fix reverted, all three fail. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 9 +++--- crates/ui-egui/src/page_pointer.rs | 50 ++++++++++++++++++++++++------ crates/ui-egui/tests/it/pointer.rs | 49 ++++++++++++++++++++++++----- 3 files changed, 88 insertions(+), 20 deletions(-) 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);