diff --git a/ROADMAP.md b/ROADMAP.md index 0d5870d5c6..8827e2da53 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -899,6 +899,7 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-21 (M4, #3865):** A new bookmark records where the reader is: the top-left of the selected text, or the top of the window, as `/XYZ left top null` (zoom kept), instead of the page top (`Edit::AddBookmark` takes `left` and `top`; `bookmark_add` takes `top` and `left`; `PageInfo::xyz_for_view`). - **2026-10-18 (renderer, hayro patches 19/35):** A mesh shading (types 4–7) is sampled over its own triangles instead of the whole path: a BricsCAD plan with 310 one-triangle `sh` fills took 2 s per 1024 px tile at every zoom (and hit the 20 s watchdog on a busy machine); now about 70 ms, so zooming into dense topology drawings stays fluid. Estimates unchanged. +- **2026-10-22 (M3, #3530, builds on PR #3547):** A find step keeps its match in the middle of the window after zooming: a zoom pending in the same frame no longer puts the page-top jump off to the next frame, and the rest of an eased ⌘/Ctrl-wheel zoom anchors on the match instead of the pointer. Synthetic UI regressions step through matches on pages larger and smaller than the window at 10–3200% and in every page display. Estimates unchanged. - **2026-10-22 (PR #3227 integration):** M11: integrate upstream `80b7d215`; retain partial target-version compatibility and upstream content-boundary and image-size regression coverage. Scope and estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 5568d07743..983af1d993 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -639,6 +639,11 @@ pub struct DocView { viewport_screen: Rect, /// Pending zoom anchor: page, fractional position (also in gutters), and viewport offset. zoom_anchor: Option<(usize, f32, f32, Vec2)>, + /// The match a find step just went to: its page and its centre as fractions of the displayed + /// page. A zoom before the step has been laid out, or the rest of an eased ⌘-wheel zoom + /// still arriving, keeps it at the window's centre instead of the point under the pointer + /// (#3530). Let go in the first frame without a zoom gesture once the step is laid out. + find_hold: Option<(usize, f32, f32)>, /// The page view has been shown, so its scroll offset in egui's memory is this document's. shown: bool, /// Pages that Edit already tried to turn into editable text (once each, so Undo sticks). @@ -1075,6 +1080,7 @@ impl DocView { screen_ppp: 1.0, viewport_screen: Rect::NOTHING, zoom_anchor: None, + find_hold: None, shown: false, auto_ocr_tried: Default::default(), auto_ocr_skipped: Default::default(), @@ -1174,6 +1180,7 @@ impl DocView { self.goto = None; self.goto_point = None; self.zoom_anchor = None; + self.find_hold = None; self.flash = None; // Thumbnail drags hold page indexes from before the change. self.panel_drag = None; @@ -2160,8 +2167,10 @@ fn draw_pages( (wanted, visible_now) } -/// A zoom the fit mode changed keeps the page at the top of the view in place. -fn anchor_zoom_change(view: &mut DocView, avail_width: f32, old_zoom: f32) { +/// A zoom the fit mode changed keeps the page at the top of the view in place, or, just after a +/// find step, the match in the middle of the view (`avail` is the view's size). +fn anchor_zoom_change(view: &mut DocView, avail: Vec2, old_zoom: f32) { + let avail_width = avail.x; if view.shown && view.zoom_anchor.is_none() && view.goto.is_none() && view.goto_point.is_none() && (view.zoom - old_zoom).abs() > 1e-4 { let prev_viewport = view.viewport_screen; let top_y = prev_viewport.min.y; @@ -2175,7 +2184,10 @@ fn anchor_zoom_change(view: &mut DocView, avail_width: f32, old_zoom: f32) { }) .or_else(|| view.screen_rects.iter().find(|(_, r)| r.min.y >= top_y).map(|(p, r)| (*p, 0.0f32, (r.min.y - top_y).max(0.0)))) .or_else(|| view.screen_rects.last().map(|(p, r)| (*p, 1.0f32, (r.max.y - top_y).max(0.0)))); - if let Some((page, fy, rel_y)) = anchor { + if let Some((page, fx, fy)) = view.find_hold { + // A fit zoom that changes just after a find step keeps the match in the middle too. + view.zoom_anchor = Some((page, fx, fy, avail / 2.0)); + } else if let Some((page, fy, rel_y)) = anchor { view.zoom_anchor = Some((page, 0.5, fy, vec2(avail_width / 2.0, rel_y))); } else { view.zoom_anchor = Some((view.current, 0.5, 0.0, vec2(avail_width / 2.0, MARGIN))); @@ -2885,6 +2897,12 @@ fn pinch_zoom(ui: &egui::Ui, view: &mut DocView, zoom_delta: f32, navigation_rec Some(p) => view.zoom_at(z, p), None => view.set_zoom(z), } + } else if view.goto_point.is_none() && zoom_delta == 1.0 { + // The find step has been laid out and no zoom is easing on: zoom at the pointer again. + // egui spreads a ⌘/Ctrl-wheel notch over about a quarter of a second; while the pointer + // is off the page view that zoom waits, and the hold must outlast it, or the rest of it + // anchors on the pointer once it is back and the match drifts off the middle. + view.find_hold = None; } } @@ -3124,7 +3142,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { view.viewport_h = avail.height(); let old_zoom = view.zoom; view.fit_zoom(info); - anchor_zoom_change(view, avail.width(), old_zoom); + anchor_zoom_change(view, avail.size(), old_zoom); // Pinch / ctrl+scroll zoom, anchored on the current page. let zoom_delta = crate::motion::zoom_delta(ui.ctx()); diff --git a/crates/ui-egui/src/canvas/find.rs b/crates/ui-egui/src/canvas/find.rs index 9d750cfdb2..73b76eb3a6 100644 --- a/crates/ui-egui/src/canvas/find.rs +++ b/crates/ui-egui/src/canvas/find.rs @@ -208,11 +208,19 @@ impl DocView { .map(|(y, h)| ((y - 60.0) / h).clamp(0.0, 1.0)) .unwrap_or(0.0); self.goto = Some((page, frac)); + self.find_hold = None; // Then put the match's centre at the window's centre, as Acrobat does (#3530) and as a // /FitR destination's rectangle is: a page wider or taller than the window no longer // leaves it off to one side or near an edge. - if let Some([fx, fy]) = self.match_centre() { + // Finite only: a hostile page's glyphs can span -inf..inf, and NaN survives `clamp`. + if let Some([fx, fy]) = self.match_centre().filter(|c| c.iter().all(|v| v.is_finite())) { self.goto_point = Some((page, Some(fx), Some(fy), vec2(self.viewport_w, self.viewport_h) / 2.0)); + // The point alone, both axes in one frame. A pending zoom anchor would hold the old + // place for that frame and put `goto` off to the next, where its page-top offset + // undid the centring; a zoom after the step anchors on the match instead. + self.goto = None; + self.zoom_anchor = None; + self.find_hold = Some((page, fx, fy)); } self.current = page; self.reset_coarse_state(); diff --git a/crates/ui-egui/src/canvas/navigation.rs b/crates/ui-egui/src/canvas/navigation.rs index 4e491d514c..39be98bd97 100644 --- a/crates/ui-egui/src/canvas/navigation.rs +++ b/crates/ui-egui/src/canvas/navigation.rs @@ -102,8 +102,10 @@ impl DocView { } self.forward.clear(); } - // Partial wheel motion belongs to the old place. + // Partial wheel motion belongs to the old place, and so does a find step's match. self.reset_coarse_state(); + self.find_hold = None; + self.goto_point = None; self.goto = Some((page, 0.0)); self.current = page; } @@ -276,6 +278,8 @@ impl DocView { self.goto = Some((page, 0.0)); self.current = page; self.reset_coarse_state(); + // A find step's match belongs to the place left, as for any other jump. + self.find_hold = None; true } diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index d6cb1f7f33..1e8add9b2d 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -504,7 +504,7 @@ fn explicit_search_jumps_record_history_and_keep_match_destination() { v.go_to_match(0); } assert_eq!(v.back, vec![0], "explicit search jump must remember its origin"); - assert_eq!(v.goto, Some((3, 0.5)), "keep the match offset instead of the page top"); + assert_eq!(v.goto, None, "the match's point alone: a zoom that frame cannot put a page-top offset off to the next"); assert_eq!(v.current, 3); assert_eq!(v.current_page_label(None), "4"); // Chrome refreshes the idle page box; navigation must not overwrite a draft @@ -537,6 +537,56 @@ fn search_centres_the_match_on_a_rotated_view() { } } +#[test] +fn a_zoom_just_after_a_find_step_keeps_the_match_in_the_middle() { + let mut v = history_search_view(); + v.screen_rects = vec![(0, Rect::from_min_size(pos2(100.0, 20.0), vec2(300.0, 400.0)))]; + // A zoom anchored at the pointer was pending when the step came: the step drops it. + v.zoom_at(2.0, pos2(150.0, 50.0)); + assert!(v.zoom_anchor.is_some()); + v.find_step(true); + assert_eq!((v.goto, v.zoom_anchor), (None, None), "the step alone decides where the view goes"); + let middle = vec2(v.viewport_w, v.viewport_h) / 2.0; + let target = Some((3, Some(40.0 / 300.0), Some(0.675), middle)); + assert_eq!(v.goto_point, target); + // The rest of an eased wheel zoom, or a zoom in the same frame, anchors on the match + // at the window's centre, not on the pointer. + v.zoom_at(3.0, pos2(150.0, 50.0)); + assert_eq!(v.zoom_anchor, Some((3, 40.0 / 300.0, 0.675, middle))); + // Another jump lets go of the match: the next zoom is at the pointer again. + v.go_to_page(1); + v.zoom_at(4.0, pos2(150.0, 50.0)); + assert_eq!(v.zoom_anchor.map(|a| a.0), Some(0), "anchored on the page under the pointer"); +} + +#[test] +fn previous_view_after_a_find_step_lets_go_of_the_match() { + let mut v = history_search_view(); + v.screen_rects = vec![(0, Rect::from_min_size(pos2(100.0, 20.0), vec2(300.0, 400.0)))]; + v.find_step(true); + // Previous view while a wheel zoom is still easing: the zoom must not hold the old match + // and put the history jump off until it stops. + assert!(v.view_history(false)); + assert_eq!(v.goto, Some((0, 0.0))); + v.zoom_at(2.0, pos2(150.0, 50.0)); + assert_eq!(v.zoom_anchor.map(|a| a.0), Some(0), "anchored on the page under the pointer"); +} + +#[test] +fn a_match_with_unbounded_glyphs_falls_back_to_its_page() { + let mut v = history_search_view(); + // A hostile page: a glyph from -inf to inf, whose centre is NaN. + if let Some(text) = v.texts.get_mut(&3).and_then(Arc::get_mut) { + text.glyphs[0].rect = [f32::NEG_INFINITY, 260.0, f32::INFINITY, 280.0]; + } + v.zoom_at(2.0, pos2(150.0, 50.0)); + v.find_step(true); + assert!(v.goto_point.is_none(), "no NaN scroll target"); + assert_eq!(v.goto.map(|g| g.0), Some(3), "the match's page instead"); + v.zoom_at(3.0, pos2(150.0, 50.0)); + assert!(v.zoom_anchor.is_none_or(|a| a.1.is_finite() && a.2.is_finite()), "no NaN zoom anchor"); +} + #[test] fn automatic_search_updates_do_not_record_history_or_discard_forward() { let mut v = history_search_view(); @@ -544,7 +594,7 @@ fn automatic_search_updates_do_not_record_history_or_discard_forward() { v.find = Some(Find { query: "match".into(), ..Default::default() }); v.rerun_find(); assert_eq!(v.current, 3); - assert_eq!(v.goto, Some((3, 0.5))); + assert_eq!((v.goto, v.goto_point.map(|g| g.0)), (None, Some(3)), "the match's point on its page"); assert!(v.back.is_empty()); assert_eq!(v.forward, vec![4]); v.find.as_mut().unwrap().current = None; diff --git a/crates/ui-egui/src/canvas/zoom.rs b/crates/ui-egui/src/canvas/zoom.rs index 91bc4feac8..ef1239110a 100644 --- a/crates/ui-egui/src/canvas/zoom.rs +++ b/crates/ui-egui/src/canvas/zoom.rs @@ -215,15 +215,18 @@ impl DocView { if zoom == self.zoom && self.fit == Fit::None { return; } + // Just after a find step, around the match at the window's centre (#3530). + let held = self.find_hold.map(|(p, fx, fy)| (p, fx, fy, vec2(self.viewport_w, self.viewport_h) / 2.0)); // In gutters/gaps, anchor relative to the nearest page rather than jumping to its top. // Fractions outside 0..1 preserve the pointer's position outside the page as well. - let anchor = + let anchor = held.or_else(|| { self.screen_rects.iter().min_by(|(_, a), (_, b)| a.distance_sq_to_pos(screen_pos).total_cmp(&b.distance_sq_to_pos(screen_pos))).map( |(p, r)| { let f = (screen_pos - r.min) / r.size(); (*p, f.x, f.y, screen_pos - self.viewport_screen.min) }, - ); + ) + }); self.zoom = zoom; self.fit = Fit::None; match anchor { diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 1d848fc8e6..11b93089c4 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -75,6 +75,7 @@ mod reload; mod scan_clean; mod script_noop; mod scroll_prefetch; +mod search_position; mod side_panels; mod signing; mod split_view; diff --git a/crates/ui-egui/tests/it/search_position.rs b/crates/ui-egui/tests/it/search_position.rs new file mode 100644 index 0000000000..9215d997e5 --- /dev/null +++ b/crates/ui-egui/tests/it/search_position.rs @@ -0,0 +1,348 @@ +//! Find goes to where the match is (#3530): after zooming in or out, every step to a match (⌘G, +//! ⇧⌘G, a click in the Search panel's results) leaves the highlighted match on screen, near the +//! middle of the window, on pages larger and smaller than the window, and also when a zoom and a +//! step land in the same frame. + +use egui_kittest::Harness; +use pdfcraft_ui_egui::PdfCraftApp; + +/// The word searched for, placed (x, y in PDF user space) on each page. +const NEEDLE: &str = "needle"; + +/// A page size, or a point on a page, in points. +type Size = (u32, u32); +type Point = (u32, u32); + +/// Page sizes (points) and where on each page the needle is: the corners, the middle and the +/// edges of large pages, as well as a small page that fits the window. +fn pages() -> Vec<(Size, Vec)> { + vec![ + ((612, 792), vec![(60, 720), (480, 60)]), + ((1700, 2400), vec![(40, 2340), (1560, 2340), (820, 1200), (40, 40), (1560, 40)]), + ((2400, 1200), vec![(2300, 1140), (60, 600), (1200, 60)]), + ((612, 792), vec![(300, 400)]), + ] +} + +/// A PDF of [`pages`]: lines of filler text down every page, and the needles. +fn fixture() -> Vec { + let pages = pages(); + let n = pages.len(); + let font = 3 + 2 * n; + let kids: Vec = (0..n).map(|i| format!("{} 0 R", 3 + 2 * i)).collect(); + let mut objs = vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string(), format!("<< /Type /Pages /Kids [{}] /Count {n} >>", kids.join(" "))]; + for (i, ((w, h), needles)) in pages.iter().enumerate() { + let mut body = String::from("BT /F1 9 Tf 11 TL"); + // Filler text a needle search must step over; it never contains the needle. + body.push_str(&format!(" 20 {} Td", h.saturating_sub(100))); + for line in 0..(h.saturating_sub(200) / 11) { + body.push_str(&format!(" (filler line {line} of the quick brown fox jumps over the lazy dog) '")); + } + body.push_str(" ET"); + for (x, y) in needles { + body.push_str(&format!(" BT /F1 14 Tf {x} {y} Td ({NEEDLE}) Tj ET")); + } + objs.push(format!( + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 {w} {h}] /Contents {} 0 R /Resources << /Font << /F1 {font} 0 R >> >> >>", + 4 + 2 * i + )); + objs.push(format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len())); + } + objs.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_string()); + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +fn needles() -> usize { + pages().iter().map(|(_, n)| n.len()).sum() +} + +/// The app with the fixture open and a search for the needle that has found every match. +fn harness() -> Harness<'static, PdfCraftApp> { + harness_dt(1.0 / 4.0) +} + +/// [`harness`] with frames `dt` seconds apart. +fn harness_dt(dt: f32) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).with_step_dt(dt).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("search.pdf", None, fixture()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app + }); + h.run_steps(4); + settle(&mut h); + h.state_mut().views[0].open_find(); + h.state_mut().views[0].find.as_mut().unwrap().query = NEEDLE.into(); + h.state_mut().views[0].rerun_find(); + // Find runs in the background: wait by a deadline, not a frame count. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while std::time::Instant::now() < deadline && h.state().views[0].find.as_ref().unwrap().matches.len() < needles() { + h.run_steps(2); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!(h.state().views[0].find.as_ref().unwrap().matches.len(), needles(), "every needle is found"); + settle(&mut h); + h +} + +/// Run frames until rendering has finished and the layout holds still. +fn settle(h: &mut Harness<'static, PdfCraftApp>) { + for _ in 0..200 { + h.run_steps(2); + if !h.state().render_pending() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } +} + +/// Fail unless the current match is drawn inside the window, and, where the page lets the view +/// scroll that far, near its middle (#3530 asks for the middle, as Acrobat does). +fn assert_match_shown(h: &Harness<'static, PdfCraftApp>, what: &str) { + let v = &h.state().views[0]; + let f = v.find.as_ref().unwrap(); + let i = f.current.unwrap_or_else(|| panic!("{what}: no current match")); + let (page, range) = f.matches[i].clone(); + let window = v.viewport_rect(); + let at = |g: usize| v.glyph_screen_pos(page, g).unwrap_or_else(|| panic!("{what}: match {i} (page {}) is not on screen", page + 1)); + let (first, last) = (at(range.start), at(range.end - 1)); + // At the highest zooms one word is wider than the window: then its middle is what shows. + let ends = if last.x - first.x < window.width() * 0.8 { vec![first, last] } else { vec![first.lerp(last, 0.5)] }; + for p in ends { + assert!(window.contains(p), "{what}: match {i} (page {}) at {p:?} is outside the window {window:?} (zoom {})", page + 1, v.zoom); + } + let Some(page_rect) = v.page_screen_rect(page) else { return }; + let centre = first.lerp(last, 0.5); + // Glyph centres, not the match's box: their middle is off the box's by up to half a glyph. + let slack = 4.0 + 0.25 * (last.x - first.x).abs(); + // Off the middle only as far as the content's ends stop the scroll. + let room_x = (page_rect.left() < window.left() - 1.0 || centre.x > window.center().x) + && (page_rect.right() > window.right() + 1.0 || centre.x < window.center().x); + let room_y = (page_rect.top() < window.top() - 1.0 || centre.y > window.center().y) + && (page_rect.bottom() > window.bottom() + 1.0 || centre.y < window.center().y); + if room_x { + assert!((centre.x - window.center().x).abs() < slack, "{what}: match {i} at x {} is not centred in {window:?} (zoom {})", centre.x, v.zoom); + } + if room_y { + assert!((centre.y - window.center().y).abs() < slack, "{what}: match {i} at y {} is not centred in {window:?} (zoom {})", centre.y, v.zoom); + } +} + +/// Step through every match forward and back at each zoom, as ⌘G and ⇧⌘G do. +#[test] +fn every_match_is_shown_after_zooming() { + let mut h = harness(); + let n = needles(); + for zoom in [0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 16.0] { + h.state_mut().views[0].set_zoom(zoom); + settle(&mut h); + for forward in [true, false] { + for _ in 0..n { + h.state_mut().views[0].find_step(forward); + settle(&mut h); + assert_match_shown(&h, &format!("zoom {zoom}, step {}", if forward { "forward" } else { "back" })); + } + } + } +} + +/// A click on a result in the Search panel goes to it at any zoom. +#[test] +fn search_panel_results_are_shown_after_zooming() { + let mut h = harness(); + for zoom in [0.33, 3.0, 12.0] { + h.state_mut().views[0].set_zoom(zoom); + settle(&mut h); + for i in (0..needles()).rev() { + h.state_mut().views[0].go_to_match(i); + settle(&mut h); + assert_match_shown(&h, &format!("zoom {zoom}, result {i}")); + } + } +} + +/// Zooms and steps mixed in a fixed pseudo-random order, as a reader zooms in and out between +/// searches; some steps land in the same frame as a zoom, before the zoom has been laid out. +#[test] +fn zooming_and_stepping_in_any_order_shows_the_match() { + let mut h = harness(); + let mut seed: u32 = 0x5eed_3530; + let mut next = |m: u32| { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (seed >> 8) % m + }; + for round in 0..80 { + let same_frame = next(3) == 0; + match next(4) { + 0 => h.state_mut().views[0].zoom_step(true), + 1 => h.state_mut().views[0].zoom_step(false), + 2 => { + let z = [0.2, 0.75, 1.5, 6.0, 10.0][next(5) as usize]; + h.state_mut().views[0].set_zoom(z); + } + _ => {} + } + if !same_frame { + settle(&mut h); + } + if next(2) == 0 { + let i = next(needles() as u32) as usize; + h.state_mut().views[0].go_to_match(i); + } else { + let forward = next(2) == 0; + h.state_mut().views[0].find_step(forward); + } + settle(&mut h); + assert_match_shown(&h, &format!("round {round}{}", if same_frame { " (zoom in the same frame)" } else { "" })); + } +} + +/// ⌘/Ctrl with the wheel zooms over several frames (egui eases a notch over about a quarter of a +/// second). A step to a match while that zoom is still easing goes to the match, and the rest of +/// the zoom keeps it where it went instead of holding the point under the pointer still. +#[test] +fn a_step_during_a_wheel_zoom_keeps_the_match_on_screen() { + let mut h = harness(); + h.state_mut().set_option("animation_effects", "on").unwrap(); + h.state_mut().views[0].set_zoom(3.0); + settle(&mut h); + let n = needles(); + for (i, dy) in (0..n).zip([60.0, -60.0].into_iter().cycle()) { + // The pointer well away from the middle, where the zoom would otherwise be anchored. + let w = h.state().views[0].viewport_rect(); + h.hover_at(w.min + egui::vec2(80.0, 80.0)); + h.run_steps(1); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, dy), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::COMMAND, + source: egui::MouseWheelSource::Unknown, + }); + h.run_steps(2); + let before = h.state().views[0].zoom; + h.state_mut().views[0].find_step(true); + // Let the zoom finish easing, then the rendering. + h.run_steps(60); + settle(&mut h); + assert!((h.state().views[0].zoom - before).abs() > 1e-3, "step {i}: the zoom was still easing when the step came"); + assert_match_shown(&h, &format!("step {i} during a wheel zoom ({dy})")); + } +} + +/// CodeRabbit on #4139: egui spreads a ⌘/Ctrl-wheel notch over about a quarter of a second. A +/// step to a match while that zoom is easing, the pointer then leaving the page view for a few +/// frames and coming back before the zoom has ended: the rest of the zoom still keeps the match in +/// the middle, rather than anchoring on the pointer once it is back. Frames a 60th of a second +/// apart, as on screen; the default quarter-second frames end the zoom before the pointer leaves. +#[test] +fn a_step_during_a_wheel_zoom_keeps_the_match_when_the_pointer_leaves_and_returns() { + for (dy, out) in [(120.0, 2), (240.0, 4), (480.0, 3)] { + let mut h = harness_dt(1.0 / 60.0); + h.state_mut().set_option("animation_effects", "on").unwrap(); + h.state_mut().views[0].set_zoom(3.0); + settle(&mut h); + let w = h.state().views[0].viewport_rect(); + // The pointer well away from the middle, where the zoom would otherwise be anchored. + let corner = w.min + egui::vec2(80.0, 80.0); + h.hover_at(corner); + h.run_steps(1); + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, dy), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::COMMAND, + source: egui::MouseWheelSource::Unknown, + }); + h.run_steps(2); + h.state_mut().views[0].find_step(true); + h.run_steps(1); + let during = h.state().views[0].zoom; + // Off the page view (over the toolbar above it) while the zoom eases, then back. + h.hover_at(egui::pos2(corner.x, (w.min.y - 20.0).max(1.0))); + h.run_steps(out); + h.hover_at(corner); + h.run_steps(30); + settle(&mut h); + assert!((h.state().views[0].zoom - during).abs() > 1e-3, "{dy}: the zoom was still easing when the pointer left"); + assert_match_shown(&h, &format!("pointer out for {out} frames during a wheel zoom ({dy})")); + } +} + +/// Single page and two-up, scrolling or a page (spread) at a time: each step shows its match. +#[test] +fn every_page_display_shows_the_match_after_zooming() { + use pdfcraft_ui_egui::canvas::PageLayout; + let mut h = harness(); + for (layout, continuous) in [(PageLayout::Single, false), (PageLayout::TwoUp, true), (PageLayout::TwoUp, false)] { + h.state_mut().views[0].set_display(layout, continuous); + for zoom in [0.5, 4.0] { + h.state_mut().views[0].set_zoom(zoom); + settle(&mut h); + for _ in 0..needles() { + h.state_mut().views[0].find_step(true); + settle(&mut h); + assert_match_shown(&h, &format!("{layout:?}, continuous {continuous}, zoom {zoom}")); + } + } + } +} + +/// FredApps on #4139: the match a find step went to is let go as soon as the reader moves the +/// view by hand. After a step, a scroll by the wheel and then a ⌘-wheel zoom: the zoom keeps the +/// point under the pointer, and the view does not jump back to the old match. +#[test] +fn a_scroll_by_hand_after_a_step_lets_the_match_go() { + let mut h = harness(); + h.state_mut().views[0].set_zoom(3.0); + settle(&mut h); + h.state_mut().views[0].find_step(true); + settle(&mut h); + assert_match_shown(&h, "after the step"); + let w = h.state().views[0].viewport_rect(); + let pointer = w.min + egui::vec2(80.0, 80.0); + h.hover_at(pointer); + h.run_steps(1); + // A plain wheel scroll: the reader moves on. + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, -300.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::NONE, + source: egui::MouseWheelSource::Unknown, + }); + h.run_steps(4); + settle(&mut h); + // Then a ⌘-wheel zoom with the pointer in the corner. + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: egui::vec2(0.0, 60.0), + phase: egui::TouchPhase::Move, + modifiers: egui::Modifiers::COMMAND, + source: egui::MouseWheelSource::Unknown, + }); + h.run_steps(20); + settle(&mut h); + let v = &h.state().views[0]; + let f = v.find.as_ref().unwrap(); + let (page, range) = f.matches[f.current.unwrap()].clone(); + let centre = v.glyph_screen_pos(page, range.start).map(|a| a.lerp(v.glyph_screen_pos(page, range.end - 1).unwrap_or(a), 0.5)); + assert!( + centre.is_none_or(|c| c.distance(v.viewport_rect().center()) > 100.0), + "the zoom went back to the old match at {centre:?} (window centre {:?})", + v.viewport_rect().center() + ); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 577104746d..ebd7519ff8 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1430,8 +1430,8 @@ milestone = "M3" status = "shipped" commands = ["edit.find"] tools = [] -evidence = ["crates/ui-egui/tests/it/ui.rs::find_counts_matches_across_pages"] -notes = "" +evidence = ["crates/ui-egui/tests/it/ui.rs::find_counts_matches_across_pages", "crates/ui-egui/tests/it/search_position.rs::every_match_is_shown_after_zooming", "crates/ui-egui/tests/it/search_position.rs::zooming_and_stepping_in_any_order_shows_the_match", "crates/ui-egui/tests/it/search_position.rs::a_step_during_a_wheel_zoom_keeps_the_match_on_screen", "crates/ui-egui/tests/it/search_position.rs::every_page_display_shows_the_match_after_zooming"] +notes = "Each step (⌘G, ⇧⌘G, Enter, a Search panel result) centres the match in the window on both axes at any zoom, view rotation and page display, as far as the content's ends allow (#3530); a zoom still easing on, or one in the same frame, anchors on the match instead of the pointer. A word wider than the window shows its middle." [[feature]] id = "view.find-options"