diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 82c07235..363c3c83 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -354,6 +354,9 @@ pub struct DocView { pub(crate) text_failed: HashSet, pub find: Option, selection: Option, + /// The drag in progress started a text selection: the wheel scrolls the page during it + /// (#721), which egui's scroll area refuses while anything is dragged. + selection_drag: bool, /// Clicks in the current run on the text layer: two select a word, three the line, four the page. clicks: u32, last_press_time: f64, @@ -613,6 +616,7 @@ impl DocView { text_failed: HashSet::new(), find: None, selection: None, + selection_drag: false, clicks: 0, last_press_time: 0.0, last_press_pos: None, @@ -2382,6 +2386,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { ui.scroll_with_delta_animation(delta, egui::style::ScrollAnimation::none()); } let (resp_rect, resp) = ui.allocate_exact_size(vec2(content_w, content_h), Sense::click_and_drag()); + // A drag lasts only while egui reports it, so the mark can't outlive one. + if !resp.dragged_by(egui::PointerButton::Primary) { + view.selection_drag = false; + } // The Hand tool pans: the content widget takes every drag, so scroll by its delta. if hand { if resp.dragged() { @@ -2798,6 +2806,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } if resp.drag_started() && press_here && !over_link { + view.selection_drag = true; let origin = ui.input(|inp| inp.pointer.press_origin()).unwrap_or(p); let (ox, oy) = xf.screen_to_view(origin); if view.clicks < 2 { @@ -2988,6 +2997,23 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } } + // 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 + // document area, like #759. A Line/Page event clears precise_scroll_until; a Point + // gesture's eased tail after its suppression deadline is not picked up. + if view.selection_drag + && navigation_enabled + && unobstructed + && !single_paging + && view.precise_scroll_until == 0.0 + && ui.rect_contains_pointer(avail) + { + let delta = ui.input_mut(|input| std::mem::take(&mut input.smooth_scroll_delta)); + if delta != Vec2::ZERO { + ui.scroll_with_delta_animation(delta, egui::style::ScrollAnimation::none()); + } + } // The page navigated to stays current while no page shows more of itself, so the next // page step starts from it rather than from a page further down the screen (#188). // It also stays current while it is wholly on screen: a short page (a cheque) gone to diff --git a/crates/ui-egui/tests/ui_group/text_select.rs b/crates/ui-egui/tests/ui_group/text_select.rs index d6f970de..a98b2f8e 100644 --- a/crates/ui-egui/tests/ui_group/text_select.rs +++ b/crates/ui-egui/tests/ui_group/text_select.rs @@ -1,7 +1,7 @@ //! Issue #295: quick clicks on text widen the selection, as in Acrobat: two clicks select a //! word, three the line, four all the text on the page. -use egui::{Event, Modifiers, PointerButton, Pos2}; +use egui::{Event, Modifiers, MouseWheelUnit, PointerButton, Pos2, TouchPhase}; use egui_kittest::Harness; use pdfcraft_ui_egui::PdfCraftApp; @@ -27,10 +27,15 @@ trailer << /Root 1 0 R >> /// The page on screen and the pointer over the "u" of "quick". Frames are 1/60 s: a click takes /// two frames (press, release), and egui counts a double-click only within 0.3 s. fn harness(setup: impl FnOnce(&mut PdfCraftApp) + 'static) -> (Harness<'static, PdfCraftApp>, Pos2) { + harness_with(two_lines(), setup) +} + +/// `harness` with another document; the pointer ends over glyph 5 of its first page. +fn harness_with(pdf: Vec, setup: impl FnOnce(&mut PdfCraftApp) + 'static) -> (Harness<'static, PdfCraftApp>, Pos2) { let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 60.0).build_eframe(move |_cc| { let mut app = PdfCraftApp::new(); app.set_option("language", "en").unwrap(); - app.open_bytes("lines.pdf", None, two_lines()).expect("opens"); + app.open_bytes("lines.pdf", None, pdf).expect("opens"); app.set_option("left", "closed").unwrap(); app.set_option("author", "Tester").unwrap(); setup(&mut app); @@ -184,3 +189,81 @@ fn multiclick_selects_on_pointer_press_before_release() { h.run_steps(1); assert_eq!(h.state().views[0].selected_text().as_deref(), Some("The quick brown fox"), "line stays selected on release"); } + +/// `n` pages of 300×200, each ending with one line of Helvetica text near its bottom edge. +fn bottom_lines(n: usize) -> Vec { + let mut objects = String::new(); + let mut kids = Vec::new(); + for k in 0..n { + let (page, contents) = (4 + 2 * k, 5 + 2 * k); + kids.push(format!("{page} 0 R")); + let content = format!("BT /F1 14 Tf 20 20 Td (Page {} ends with this line) Tj ET", k + 1); + objects.push_str(&format!( + "{page} 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Contents {contents} 0 R /Resources << /Font << /F1 3 0 R >> >> >> endobj\n\ + {contents} 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n", + content.len() + )); + } + format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [{}] /Count {n} >> endobj\n\ + 3 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> endobj\n{objects}trailer << /Root 1 0 R >>\n%%EOF", + kids.join(" ") + ) + .into_bytes() +} + +/// Press at `from` and move to `to`, keeping the button down. +fn press_and_move(h: &mut Harness<'static, PdfCraftApp>, from: Pos2, to: Pos2) { + h.event(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(from + (to - from) * (k as f32 / 4.0)); + h.run_steps(1); + } +} + +/// One wheel event, with time to scroll (egui spreads a wheel line over a few frames). +fn wheel(h: &mut Harness<'static, PdfCraftApp>, unit: MouseWheelUnit, dy: f32) { + h.event(Event::MouseWheel { unit, delta: egui::vec2(0.0, dy), phase: TouchPhase::Move, modifiers: Modifiers::NONE }); + h.run_steps(10); +} + +/// The top edge of `page` on screen. +fn page_top(h: &Harness<'static, PdfCraftApp>, page: usize) -> f32 { + h.state().views[0].page_screen_rect(page).expect("page on screen").min.y +} + +/// Issue #721: selecting near the bottom of a page with the button held, the wheel keeps +/// scrolling while the pointer crosses the gap onto the next page. The selection stays on its +/// own page (a selection can't span pages yet). +#[test] +fn a_selection_drag_keeps_scrolling_over_the_gap_and_the_next_page() { + let (mut h, _) = harness_with(bottom_lines(3), |app| { + app.set_option("layout", "continuous").unwrap(); + app.set_option("zoom", "200").unwrap(); + }); + let glyph = |h: &Harness<'static, PdfCraftApp>, g: usize| h.state().views[0].glyph_screen_pos(0, g).expect("glyph on screen"); + // "ends" (glyphs 7..=10) in "Page 1 ends with this line". + let (from, to) = (glyph(&h, 7), glyph(&h, 10)); + press_and_move(&mut h, from, to); + assert_eq!(h.state().views[0].selected_text().as_deref(), Some("ends")); + let (mut over_gap, mut over_next) = (false, false); + for _ in 0..100 { + let before = page_top(&h, 0); + wheel(&mut h, MouseWheelUnit::Line, -0.1); + assert!(page_top(&h, 0) < before - 0.5, "the wheel scrolls during the drag (over the gap yet: {over_gap})"); + let view = &h.state().views[0]; + let on = |page| view.page_screen_rect(page).is_some_and(|r| r.contains(to)); + over_gap |= !on(0) && !on(1); + over_next = on(1); + if over_next { + break; + } + } + assert!(over_gap && over_next, "the pointer crossed the gap ({over_gap}) onto the next page ({over_next})"); + let held = h.state().views[0].selected_text().expect("still selected"); + assert!(held.starts_with("ends") && "Page 1 ends with this line".contains(held.as_str()), "the selection stays on page 1: {held:?}"); + h.event(Event::PointerButton { pos: to, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); + assert_eq!(h.state().views[0].selected_text(), Some(held), "releasing keeps the selection"); +} diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 35a82c59..b5b1668b 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -1044,3 +1044,177 @@ fn a_picture_inside_form_artwork_under_text_is_selected_and_copied() { let png = image::load_from_memory_with_format(&bytes, image::ImageFormat::Png).unwrap().into_rgba8(); assert_eq!(png.into_raw(), vec![255, 0, 0, 255, 0, 0, 255, 255]); } +#[test] +fn mouse_wheel_scrolls_while_selecting_text() { + let mut h = harness(); + h.state_mut().set_option("zoom", "200").unwrap(); + h.state_mut().set_option("left", "closed").unwrap(); + h.state_mut().set_option("panel", "none").unwrap(); + h.run_steps(8); + let view = h.state().active.unwrap(); + let page = h.state().views[view].page_screen_rect(0).unwrap(); + let start = egui::pos2(page.left() + page.width() * 0.11, page.top() + page.height() * 0.48); + let end = start + egui::vec2(100.0, 0.0); + h.hover_at(start); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: start, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + h.hover_at(end); + h.run_steps(2); + assert!(h.state().views[view].selected_text().is_some_and(|s| !s.is_empty()), "a text selection is active"); + assert!(h.ctx.dragged_id().is_some(), "the primary drag is still held"); + let before = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::vec2(0.0, -3.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(20); + let after = h.state().views[view].page_screen_rect(0).unwrap().top(); + assert!(after < before - 1.0, "wheel should move the page during selection: {before} -> {after}"); + assert!(h.state().views[view].selected_text().is_some_and(|s| !s.is_empty())); + h.event(egui::Event::PointerButton { pos: end, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(2); +} +fn selection_wheel_harness() -> (Harness<'static, PdfCraftApp>, usize, egui::Pos2) { + let mut h = harness(); + h.state_mut().set_option("zoom", "200").unwrap(); + h.state_mut().set_option("left", "closed").unwrap(); + h.state_mut().set_option("panel", "none").unwrap(); + h.run_steps(8); + let view = h.state().active.unwrap(); + let page = h.state().views[view].page_screen_rect(0).unwrap(); + let start = egui::pos2(page.left() + page.width() * 0.11, page.top() + page.height() * 0.48); + let end = start + egui::vec2(100.0, 0.0); + h.hover_at(start); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: start, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + h.hover_at(end); + h.run_steps(2); + assert!(h.state().views[view].selected_text().is_some_and(|s| !s.is_empty()), "a text selection is active"); + assert!(h.ctx.dragged_id().is_some(), "the primary drag is still held"); + + (h, view, end) +} + +fn selection_wheel_delta(unit: egui::MouseWheelUnit, amount: f32, held: bool, animated: bool) -> f32 { + let (mut h, view, end) = selection_wheel_harness(); + h.state_mut().animation_effects = + if animated { pdfcraft_ui_egui::motion::AnimationEffects::On } else { pdfcraft_ui_egui::motion::AnimationEffects::Off }; + if !held { + h.event(egui::Event::PointerButton { pos: end, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + } + h.run_steps(2); + let before = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { unit, delta: egui::vec2(0.0, -amount), phase: egui::TouchPhase::Move, modifiers: egui::Modifiers::NONE }); + h.run_steps(40); + before - h.state().views[view].page_screen_rect(0).unwrap().top() +} + +#[test] +fn selection_wheel_preserves_normal_distance_and_reduced_motion() { + for animated in [true, false] { + let normal = selection_wheel_delta(egui::MouseWheelUnit::Line, 1.0, false, animated); + let held = selection_wheel_delta(egui::MouseWheelUnit::Line, 1.0, true, animated); + assert!(normal > 1.0); + assert!((normal - held).abs() < 1.0, "animated={animated}: released={normal}, held={held}"); + } + let on = selection_wheel_delta(egui::MouseWheelUnit::Line, 1.0, true, true); + let off = selection_wheel_delta(egui::MouseWheelUnit::Line, 1.0, true, false); + assert!((on - off).abs() < 1.0, "on={on}, off={off}"); +} + +#[test] +fn selection_point_scroll_is_not_applied_twice() { + for animated in [true, false] { + let normal = selection_wheel_delta(egui::MouseWheelUnit::Point, 30.0, false, animated); + let held = selection_wheel_delta(egui::MouseWheelUnit::Point, 30.0, true, animated); + eprintln!("Point animated={animated}: released={normal}, held={held}"); + assert!(normal > 1.0, "point wheel must scroll: {normal}"); + assert!((held - 30.0).abs() < 1.0, "animated={animated}: held precision motion={held}, normal={normal}"); + } +} + +#[test] +fn selection_control_wheel_still_zooms() { + for modifiers in [egui::Modifiers::CTRL, egui::Modifiers::COMMAND] { + let (mut h, view, _) = selection_wheel_harness(); + let before = h.state().views[view].page_screen_rect(0).unwrap().width(); + h.event(egui::Event::MouseWheel { unit: egui::MouseWheelUnit::Line, delta: egui::vec2(0.0, 1.0), phase: egui::TouchPhase::Move, modifiers }); + h.run_steps(40); + let after = h.state().views[view].page_screen_rect(0).unwrap().width(); + assert!(after > before + 1.0, "control wheel should zoom: {before} -> {after}"); + } +} + +#[test] +fn selection_wheel_outside_document_does_not_scroll() { + let (mut h, view, _) = selection_wheel_harness(); + h.hover_at(egui::pos2(1398.0, 2.0)); + h.run_steps(2); + let before = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::vec2(0.0, -1.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(40); + let after = h.state().views[view].page_screen_rect(0).unwrap().top(); + assert!((after - before).abs() < 1.0, "outside wheel moved document: {before} -> {after}"); +} + +#[test] +fn selection_page_wheel_preserves_normal_distance() { + for animated in [true, false] { + let normal = selection_wheel_delta(egui::MouseWheelUnit::Page, 0.1, false, animated); + let held = selection_wheel_delta(egui::MouseWheelUnit::Page, 0.1, true, animated); + assert!(normal > 1.0); + assert!((normal - held).abs() < 1.0, "animated={animated}: released={normal}, held={held}"); + } +} + +#[test] +fn selection_wheel_does_not_scroll_behind_preferences() { + let (mut h, view, _) = selection_wheel_harness(); + h.state_mut().set_option("dialog", "preferences").unwrap(); + h.run_steps(3); + let before = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::vec2(0.0, -1.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(40); + let after = h.state().views[view].page_screen_rect(0).unwrap().top(); + assert!((after - before).abs() < 1.0, "modal wheel moved document: {before} -> {after}"); +} + +#[test] +fn selection_mouse_wheel_resumes_after_point_scrolling() { + for delay in [1, 16, 40] { + let (mut h, view, _) = selection_wheel_harness(); + let start = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, -30.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(delay); + let before = h.state().views[view].page_screen_rect(0).unwrap().top(); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::vec2(0.0, -1.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(40); + let after = h.state().views[view].page_screen_rect(0).unwrap().top(); + eprintln!("Point-to-Line delay={delay}: point={}, line={}", start - before, before - after); + assert!(before - after > 50.0, "Line input must resume after Point: {before} -> {after}"); + } +}