diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 0acab64a0..c74085efe 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -444,6 +444,9 @@ pub struct DocView { pub(crate) text_failed: HashSet, pub find: Option, selection: Option, + /// The selection whose action bar was closed by its Copy button: the bar stays hidden + /// until the selection changes, as it does once a markup button consumes the selection. + actions_dismissed: 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, @@ -724,6 +727,7 @@ impl DocView { text_failed: HashSet::new(), find: None, selection: None, + actions_dismissed: None, selection_drag: false, clicks: 0, last_press_time: 0.0, @@ -1033,6 +1037,7 @@ impl DocView { pub(crate) fn clear_selection(&mut self) { self.selection = None; + self.actions_dismissed = None; } /// Select text on `page` from glyph `from` to glyph `to` (tests and automation). @@ -2150,11 +2155,20 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s if ctx.input_mut(|i| i.consume_shortcut(&KeyboardShortcut::new(Modifiers::COMMAND, Key::F))) { view.open_find(); } + let cmd = |k| KeyboardShortcut::new(Modifiers::COMMAND, k); + let pressed = |s: KeyboardShortcut| ctx.input_mut(|i| i.consume_shortcut(&s)); + // ⌘G / ⇧⌘G step through the matches from the find field too, as in Acrobat and Preview + // (#2669): ahead of the text-input check, but only while there is a search to step. + if view.find.is_some() { + if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::G)) { + view.find_step(false); + } else if pressed(cmd(Key::G)) { + view.find_step(true); + } + } if ctx.egui_wants_keyboard_input() { return; } - let cmd = |k| KeyboardShortcut::new(Modifiers::COMMAND, k); - let pressed = |s: KeyboardShortcut| ctx.input_mut(|i| i.consume_shortcut(&s)); // Rotation first: egui matches ⌘+ loosely with respect to Shift. if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::Plus)) || pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::Equals)) @@ -2195,11 +2209,6 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s view.fit = Fit::Width; view.goto = Some((view.current, 0.0)); } - if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::G)) { - view.find_step(false); - } else if pressed(cmd(Key::G)) { - view.find_step(true); - } if pressed(cmd(Key::OpenBracket)) { view.view_history(false); } @@ -2406,9 +2415,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // When not scrolling continuously, only one row is shown at a time: the current page, or the // current two-page spread (with the cover rule). These indices drive paging, which pages are // drawn, and the content height (#752). Empty for a document with no pages. - let row: Vec = if rects.is_empty() { - Vec::new() - } else { + let shown_row = |view: &DocView| -> Vec { + if rects.is_empty() { + return Vec::new(); + } let last = rects.len() - 1; let cur = view.current.min(last); match view.layout { @@ -2420,6 +2430,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } }; + let row = shown_row(view); let middle_gesture = view.auto_scroll.blocks_input(); // Not scrolling: when the whole page or spread fits, the wheel would do nothing, so it turns // pages instead; zoomed in far enough to pan, it pans, and at the edge it turns the page on @@ -2520,6 +2531,9 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { if view.fling.gliding() { ui.ctx().request_repaint(); } + // The wheel may have turned the page above: draw the row it turned to. Measured from the old + // row, the jump to the new page scrolled both out of view, a blank frame on every turn forward. + let row = shown_row(view); let visible_pages: Vec = if view.continuous { (0..rects.len()).collect() } else { row.clone() }; let (y_shift, content_h) = if view.continuous { (0.0, rects.iter().map(|r| r.bottom()).reduce(f32::max).map(|bottom| bottom + MARGIN).unwrap_or(0.0)) @@ -2559,13 +2573,26 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let r = rects[page.min(rects.len() - 1)]; let point = pos2(r.left() + fx * r.width(), r.top() - y_shift + fy * r.height()); scroll = scroll.scroll_offset(vec2((point.x - rel.x).max(0.0), (point.y - rel.y).max(0.0))); - } else if edge_turned { - // The row above was laid out for the page before the turn: place the new one next frame. - ui.ctx().request_repaint(); } else if let Some((page, frac)) = view.goto.take() { let page = page.min(rects.len() - 1); let r = rects[page]; - scroll = scroll.vertical_scroll_offset((r.top() - y_shift - GAP + frac * r.height()).max(0.0)); + // Clamped here: egui clamps only after drawing, so a page that fits was drawn MARGIN - GAP + // too high for a frame and then hopped into place. (`avail` is the viewport, as the + // scroll bars float.) + let max_y = (content_h - avail.height()).max(0.0); + scroll = scroll.vertical_scroll_offset((r.top() - y_shift - GAP + frac * r.height()).max(0.0).min(max_y)); + // Horizontally too, as Acrobat does (#2665): beside a much wider page the others sit off + // to one side. A page already shown whole, or one filling the window's width, keeps the + // scroll, so paging through a zoomed-in document stays on the same column. Otherwise a + // page that fits is centred, and a wider one scrolled just far enough to fill the window. + // The page counts with its side gutters, which keep its edge clear of the floating + // quick-action bar as the layout does. + let (x, w) = (view.last_scroll.x, avail.width()); + let (left, right) = (r.left() - SIDE, r.right() + SIDE); + let target = if right - left <= w { if left >= x && right <= x + w { x } else { r.center().x - w / 2.0 } } else { x.clamp(left, right - w) }; + if (target - x).abs() > 0.5 { + scroll = scroll.horizontal_scroll_offset(target.clamp(0.0, (content_w - w).max(0.0))); + } } let ppp = ui.ctx().pixels_per_point(); let scale = view.render_scale(ppp); @@ -3191,6 +3218,9 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } if resp.drag_started() && press_here && !over_link { + // A drag replaces the selection without passing through none, so a + // re-drag over text whose bar Copy closed would keep the bar hidden. + view.actions_dismissed = None; view.selection_drag = true; let origin = ui.input(|inp| inp.pointer.press_origin()).unwrap_or(p); let (ox, oy) = xf.screen_to_view(origin); @@ -3275,6 +3305,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { if tool == QuickTool::Select && !consumed { crate::image_pick::page_input(&resp, &xf, i, info, doc, pointer, view); } + crate::image_pick::drop_stale(doc, view); crate::image_pick::paint(painter, &xf, i, info, view); comments::page_after_text(&resp, &pcx, view); if tool == QuickTool::Redact && can_modify { @@ -3607,6 +3638,26 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { view.last_scroll = scroll_offset; let pan_held = now_settle < view.pan_hold_until; let (mut wanted, visible_now) = out.inner; + // Single-page and two-page views draw only the current row, so the next one was first asked + // for on the turn and showed blank paper until it rendered. Prefetch the rows either side, + // as continuous scrolling does for the pages just off screen. + if !view.continuous + && let (Some(&first), Some(&last)) = (visible_pages.first(), visible_pages.last()) + { + let n = if view.layout == PageLayout::Single { 1 } else { 2 }; + let max_side = ui.ctx().input(|inp| inp.max_texture_side) as f32; + for i in (last.saturating_add(1)..=last.saturating_add(n)).chain(first.saturating_sub(n)..first) { + let Some(p) = info.pages.get(i).filter(|_| !view.errors.contains_key(&i)) else { continue }; + // As drawn: the whole page at this scale, or the backdrop of a tiled page. + let side = p.width.max(p.height).max(1.0); + wanted.push(if side * scale > TILE_THRESHOLD.min(max_side) { + let bs = BASE_SIDE.min(max_side) / side; + (i, bs, scale_tag(bs), None) + } else { + (i, scale, tag, None) + }); + } + } let cur = view.current; wanted.sort_by_key(|w| (!visible_now.contains(&w.0), w.0.abs_diff(cur), w.3.is_some())); // Tiles of far-away pages are useless: free them. Tiles of an earlier zoom stay for a page @@ -3893,7 +3944,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { if copy_selected { app.execute("edit.copy"); } - quick_bar(app, avail, ui); + // Read mode hides the tools (#2646). + if app.mode != crate::Mode::Read { + quick_bar(app, avail, ui); + } if let Some((source, file, destination)) = remote_open { app.open_remote_link(source, file, destination); } @@ -3920,7 +3974,13 @@ fn selection_actions( if ui.input(|input| input.pointer.is_decidedly_dragging()) { return false; } - let Some(selection) = view.selection else { return false }; + let Some(selection) = view.selection else { + view.actions_dismissed = None; + return false; + }; + if view.actions_dismissed == Some(selection) { + return false; + } let Some(text) = view.texts.get(&selection.page) else { return false }; let Some(xf) = view.page_xform(selection.page) else { return false }; let Some(last) = selection.rects(text).last().copied() else { return false }; @@ -3957,6 +4017,8 @@ fn selection_actions( }); let Some(action) = chosen else { return false }; if let SelectionAction::Copy = action { + // Copying closes the bar but keeps the words selected, for ⌘C or a second action. + view.actions_dismissed = Some(selection); return true; } let Some((page, quads)) = view.selection_quads(info) else { return false }; diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index be1347d68..6f7819949 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -52,6 +52,12 @@ impl PdfCraftApp { let Some(doc) = self.session.get(id) else { return true }; let info = &doc.info; let view = &mut self.views[i]; + // A picture picked with the Select tool may be the one this edit removed or moved: + // drop its highlight now, not on the next frame (which only comes with the next input). + view.picked_image = None; + if let Some(ctx) = &self.ctx { + ctx.request_repaint(); + } if !matches!(edit, Edit::AddAnnotation(_)) { // Only a new drawing waits on its raster; any other edit may remove or move it. view.comments.settling.clear(); diff --git a/crates/ui-egui/src/image_pick.rs b/crates/ui-egui/src/image_pick.rs index a05aefe2d..53e4bdf15 100644 --- a/crates/ui-egui/src/image_pick.rs +++ b/crates/ui-egui/src/image_pick.rs @@ -9,12 +9,14 @@ use crate::PdfCraftApp; use crate::canvas::{DocView, PageXform}; use pdfcraft_render::DocInfo; -/// The selected picture: page, index in `page_images` and its box in user space. +/// The selected picture: page, index in `page_images` and its box in user space, and the +/// document's edit generation when it was picked (any later edit may have removed or renumbered it). #[derive(Clone, Copy, Debug, PartialEq)] pub struct PickedImage { pub page: usize, pub index: usize, pub rect: [f64; 4], + pub generation: u64, } const BLUE: Color32 = Color32::from_rgb(0x3A, 0x7B, 0xF0); @@ -43,8 +45,18 @@ pub(crate) fn page_input( let (vx, vy) = xf.screen_to_view(p); let u = info_page.view_to_user(vx, vy); let at = [f64::from(u[0]), f64::from(u[1])]; + let generation = doc.edit_generation(); view.picked_image = - doc.page_image_at(page, at).and_then(|index| doc.page_images(page).get(index).map(|i| PickedImage { page, index, rect: i.rect })); + doc.page_image_at(page, at).and_then(|index| doc.page_images(page).get(index).map(|i| PickedImage { page, index, rect: i.rect, generation })); +} + +/// Forget the selected picture once the document has been edited since it was picked: Edit text & +/// images (or Undo) may have deleted or moved it, and its highlight would stay behind on an empty +/// spot of the page. +pub(crate) fn drop_stale(doc: &pdfcraft_engine::Document, view: &mut DocView) { + if view.picked_image.is_some_and(|p| p.generation != doc.edit_generation()) { + view.picked_image = None; + } } /// The selected picture's highlight on page `page`. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 79464624c..bfe5179a6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2152,6 +2152,11 @@ impl PdfCraftApp { } else if matches!(self.quick_tool, QuickTool::EditText | QuickTool::AddText) { self.put_down_edit_text(); } + // Read mode hides the quick toolbar, so no other tool may stay active there. Opening a + // document with Read as the default mode comes here without `switch_mode`. + if mode == Mode::Read { + self.quick_tool = QuickTool::Select; + } } /// Back to the Select tool from an editing tool. A paragraph still open in the editor is @@ -2169,6 +2174,7 @@ impl PdfCraftApp { /// Switches workspace because the user chose to (mode bar, Read mode command). Entering Read /// mode closes the right panel, as Acrobat's Read mode hides its panes, and leaving it reopens /// that panel. The rail stays in Read mode, and its buttons open panels there too (#932). + /// Read mode hides the quick toolbar, so entering it also puts the current tool down. pub(crate) fn switch_mode(&mut self, mode: Mode, select_tools: bool) { let was_read = self.mode == Mode::Read; if select_tools { @@ -2178,6 +2184,7 @@ impl PdfCraftApp { } if mode == Mode::Read && !was_read { self.right_before_read = self.right.take(); + self.quick_tool = QuickTool::Select; } else if mode != Mode::Read && was_read { let before = self.right_before_read.take(); if self.right.is_none() { diff --git a/crates/ui-egui/src/marks_ui.rs b/crates/ui-egui/src/marks_ui.rs index 4a60c40a6..d91a69f93 100644 --- a/crates/ui-egui/src/marks_ui.rs +++ b/crates/ui-egui/src/marks_ui.rs @@ -28,8 +28,10 @@ pub struct PageRange { } impl Default for PageRange { + /// From the first page to the last: `to` starts past any page count, and the range field + /// shows (and clamps it to) the document's last page. fn default() -> Self { - Self { all: true, from: 1, to: 1, subset: Subset::All } + Self { all: true, from: 1, to: usize::MAX, subset: Subset::All } } } @@ -53,9 +55,6 @@ impl PageRange { widgets::radio_value(ui, &mut self.all, false, tl!("Pages from")); ui.add_enabled(!self.all, egui::DragValue::new(&mut self.from).range(1..=count.max(1))); ui.label(tl!("to")); - if self.to == 1 && self.from == 1 { - self.to = count.max(1); - } ui.add_enabled(!self.all, egui::DragValue::new(&mut self.to).range(1..=count.max(1))); ui.label(tl!("Subset")); let label = match self.subset { @@ -519,6 +518,11 @@ mod tests { assert_eq!(r.pages(6), [1, 3, 5]); let r = PageRange { all: true, subset: Subset::Odd, ..PageRange::default() }; assert_eq!(r.pages(5), [0, 2, 4]); + // Not all pages: by default up to the last one; a single page is a range too. + let r = PageRange { all: false, ..PageRange::default() }; + assert_eq!(r.pages(4), [0, 1, 2, 3]); + let r = PageRange { all: false, from: 1, to: 1, subset: Subset::All }; + assert_eq!(r.pages(38), [0]); } #[test] diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 914bc281b..c127e2c99 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/crates/ui-egui/tests/comments.rs @@ -364,6 +364,40 @@ fn selected_text_shows_actions_and_underlines_without_changing_tools() { assert_eq!(h.state().quick_tool, QuickTool::Select); } +#[test] +fn selected_text_copy_action_closes_the_bar_and_keeps_the_selection() { + let mut h = harness(|_| {}); + h.state_mut().views[0].select_text(0, 4, 8); + h.run_steps(3); + h.get_by_label("Copy").click(); + h.run_steps(3); + assert!(h.query_by_label("Add comment").is_none(), "the action bar closes after Copy"); + assert!(h.state().views[0].selected_text().is_some_and(|t| !t.is_empty()), "the words stay selected"); + // A new selection brings the bar back. + h.state_mut().views[0].select_text(0, 2, 6); + h.run_steps(3); + h.get_by_label("Add comment"); +} + +#[test] +fn dragging_over_copied_text_again_brings_the_action_bar_back() { + let mut h = harness(|_| {}); + let (a, b) = { + let v = &h.state().views[0]; + (v.glyph_screen_pos(0, 4).expect("text layer"), v.glyph_screen_pos(0, 8).expect("glyph")) + }; + drag(&mut h, a, b); + let copied = h.state().views[0].selected_text(); + assert!(copied.is_some(), "the drag selects text"); + h.get_by_label("Copy").click(); + h.run_steps(3); + assert!(h.query_by_label("Add comment").is_none(), "the action bar closes after Copy"); + // The same drag again selects the same words, which is a new selection: the bar is back. + drag(&mut h, a, b); + assert_eq!(h.state().views[0].selected_text(), copied, "the drag selects the same words"); + assert!(h.query_by_label("Add comment").is_some(), "the action bar is back after a new drag"); +} + #[test] fn selected_text_replace_action_opens_replacement_composer() { let mut h = harness(|_| {}); diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index 1b6276d7c..7fedd2bcc 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -246,6 +246,39 @@ fn read_mode_closes_the_panes_and_leaving_it_brings_them_back() { assert_eq!(h.state().right, Some(RightPanel::Pages)); } +/// Read mode hides the quick toolbar too, and puts down the tool that was in use (#2646). +#[test] +fn read_mode_hides_the_quick_toolbar() { + use pdfcraft_ui_egui::{Mode, QuickTool}; + let mut h = harness(|app| app.open_bytes("fixture.pdf", None, FIXTURE.to_vec()).expect("fixture opens")); + h.get_by_label("Hand (H)").click(); + h.run_steps(2); + assert_eq!(h.state().quick_tool, QuickTool::Hand); + h.get_by_label("Read").click(); + h.run_steps(3); + assert!(h.query_by_label("Select (V)").is_none(), "no quick toolbar in Read mode"); + assert_eq!(h.state().quick_tool, QuickTool::Select); + h.state_mut().run_command("view.read_mode"); + h.run_steps(3); + assert_eq!(h.state().mode, Mode::AllTools); + h.get_by_label("Select (V)"); +} + +/// Opening a document with Read as the default mode puts down the tool in use, as entering Read +/// mode from the mode bar does: the hidden toolbar would leave no way to see or change it. +#[test] +fn opening_into_read_mode_puts_the_tool_down() { + use pdfcraft_ui_egui::{Mode, QuickTool}; + let mut h = harness(|app| app.open_bytes("fixture.pdf", None, FIXTURE.to_vec()).expect("fixture opens")); + assert_eq!(h.state().mode, Mode::AllTools); + h.state_mut().set_option("quick", "hand").unwrap(); + h.state_mut().set_option("default-mode", "read").unwrap(); + h.state_mut().open_bytes("again.pdf", None, FIXTURE.to_vec()).unwrap(); + h.run_steps(3); + assert_eq!(h.state().mode, Mode::Read, "the new document opens in the default mode"); + assert_eq!(h.state().quick_tool, QuickTool::Select, "no tool stays active behind the hidden toolbar"); +} + /// A pane closed in Read mode stays closed on the way out: leaving Read mode doesn't bring back the /// pane saved on the way in, and closing Comments keeps its close preference. #[test] diff --git a/crates/ui-egui/tests/ui_group/marks.rs b/crates/ui-egui/tests/ui_group/marks.rs index 0a0e45b15..1f1515ad6 100644 --- a/crates/ui-egui/tests/ui_group/marks.rs +++ b/crates/ui-egui/tests/ui_group/marks.rs @@ -96,3 +96,37 @@ fn backgrounds_and_watermarks_from_a_file() { let s = h.state(); assert_eq!(s.session.get(s.views[0].id).unwrap().can_undo(), Some("Add background")); } + +/// Pages from 1 to 1 stays one page: the dialog used to turn "to" back into the last page on +/// every frame while both were 1, so only page 1 could not be chosen alone. The field starts on +/// the last page. +#[test] +fn page_range_from_one_to_one_selects_only_the_first_page() { + let mut h = harness(); + h.state_mut().execute("edit.background"); + h.run_steps(2); + assert_eq!(h.state().marks_draft.range.pages(3), [0, 1, 2]); + h.get_by_label("Pages from").click(); + h.run_steps(2); + let range = h.state().marks_draft.range; + assert_eq!((range.from, range.to), (1, 3), "from the first page to the last"); + h.state_mut().marks_draft.range.to = 1; + h.run_steps(3); + let range = h.state().marks_draft.range; + assert_eq!((range.from, range.to), (1, 1), "the range stays as typed"); + h.get_by_label("Applies to 1 page"); + h.get_by_label("OK").click(); + h.run_steps(3); + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + assert_eq!(doc.marks(), [MarkKind::Background]); + // Rendered: page 1 has the (default cream) background, page 2 is still blank. + let mut r = pdfcraft_render::PageRenderer::new(doc.bytes.clone(), pdfcraft_render::RenderConfig::default()); + let corner = |r: &mut pdfcraft_render::PageRenderer, page: usize| { + let p = r.render(pdfcraft_render::RenderRequest { page, scale: 1.0, ..Default::default() }); + assert!(p.error.is_none(), "{:?}", p.error); + p.rgba[..4].to_vec() + }; + assert_eq!(corner(&mut r, 0), vec![255, 255, 217, 255], "page 1 has the background"); + assert_eq!(corner(&mut r, 1), vec![255, 255, 255, 255], "page 2 does not"); +} diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 3f4420d3b..95761ab04 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -185,6 +185,53 @@ fn single_page_layout_shows_one_page_at_a_time() { assert!(rect(&h, 0).is_none() && rect(&h, 1).is_some(), "only page 2"); } +#[test] +fn single_page_view_prefetches_the_next_page() { + // Only the shown page is drawn, but the next one renders ahead of the turn: otherwise + // turning forward shows blank paper until it arrives. + let mut h = harness(&[("layout", "single"), ("zoom", "50")]); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while h.state().render_pending() { + assert!(std::time::Instant::now() < deadline, "renders settle"); + h.run_steps(2); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(rect(&h, 1).is_none(), "page 2 is not shown"); + assert!(h.state().views[0].raster_memory().page_count >= 2, "page 2 is rendered ahead of the turn"); +} + +#[test] +fn single_page_wheel_turn_draws_the_new_page_at_once() { + // The frame that turns the page draws the new one. Its jump used to be measured from the + // old page's place, which scrolled both out of view: a blank frame on every turn forward. + let mut h = harness_stepping(1.0 / 60.0, &[("layout", "single")]); + h.key_press_modifiers(Modifiers::COMMAND, Key::Num0); // fit page + h.run_steps(4); + let vp = h.state().views[0].viewport_rect(); + h.hover_at(vp.center()); + h.run_steps(2); + h.event(wheel(MouseWheelUnit::Line, -1.0, TouchPhase::Move)); + h.step(); + assert_eq!(h.state().views[0].current, 1, "one notch turns one page"); + let r = rect(&h, 1).expect("page 2 drawn in the frame that turns to it"); + assert!(r.min.y >= vp.min.y && r.max.y <= vp.max.y, "page 2 fits on screen: {r:?} in {vp:?}"); +} + +#[test] +fn single_page_jump_draws_a_fitting_page_where_it_stays() { + // A page that fits has nothing to scroll, but the jump to it asked for the gap above it: + // drawn that much too high for a frame, it then hopped into place. + let mut h = harness_stepping(1.0 / 60.0, &[("layout", "single")]); + h.key_press_modifiers(Modifiers::COMMAND, Key::Num0); // fit page + h.run_steps(4); + h.state_mut().set_option("page", "2").unwrap(); + h.step(); + let first = rect(&h, 1).expect("page 2 drawn in the frame after the jump"); + h.run_steps(4); + let settled = rect(&h, 1).expect("page 2"); + assert!((first.min.y - settled.min.y).abs() < 0.5, "page 2 drawn where it settles: {first:?} then {settled:?}"); +} + #[test] fn single_page_wheel_pans_instead_when_zoomed_in() { // The zoomed-in guard: with room to pan, the wheel pans and must not turn pages. diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 474360865..1f0c8b09b 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -175,6 +175,36 @@ fn find_shortcuts_advance_and_retreat_matches() { assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some((start + n - 1) % n)); } +/// #2669: ⌘G and ⇧⌘G step through the matches while the find field still has the focus. +#[test] +fn find_shortcuts_work_from_the_find_field() { + let mut h = harness(); + h.state_mut().active = Some(0); + h.run_steps(2); + h.state_mut().views[0].open_find(); + h.state_mut().views[0].find.as_mut().unwrap().query = "page".into(); + h.state_mut().views[0].rerun_find(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + while std::time::Instant::now() < deadline { + h.run_steps(2); + std::thread::sleep(std::time::Duration::from_millis(5)); + if h.state().views[0].find.as_ref().unwrap().matches.len() >= 15 { + break; + } + } + let n = h.state().views[0].find.as_ref().unwrap().matches.len(); + assert_eq!(n, 15); + assert!(h.ctx.egui_wants_keyboard_input(), "the find field has the focus"); + let start = h.state().views[0].find.as_ref().unwrap().current.unwrap(); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::G); + h.run_steps(2); + assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some((start + 1) % n)); + h.key_press_modifiers(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, egui::Key::G); + h.run_steps(2); + assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some(start)); + assert!(h.ctx.egui_wants_keyboard_input(), "and keeps it"); +} + #[test] fn marking_while_searching_does_not_jitter_or_reset_search_state() { let mut h = harness(); @@ -1384,6 +1414,55 @@ fn a_clicked_picture_is_selected_and_copied_in_its_original_format() { assert_eq!(h.state().views[i].picked_image, None); } +/// A picture picked with the Select tool, then deleted in Edit text & images, leaves no highlight +/// behind on the empty spot where it was. +#[test] +fn a_picked_picture_that_is_deleted_in_edit_mode_loses_its_highlight() { + let (_, mut h) = picture_harness(); + let i = h.state().active.unwrap(); + let id = h.state().views[i].id; + let at = h.state().views[i].page_screen_rect(0).expect("on screen").center(); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + assert!(h.state().views[i].picked_image.is_some(), "picked with the Select tool"); + // Edit text & images: click the picture, delete it with the Delete key. + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + h.hover_at(at); + h.run_steps(1); + h.drag_at(at); + h.run_steps(1); + h.drop_at(at); + h.run_steps(2); + h.key_press(egui::Key::Delete); + // One frame, no more input: the highlight goes at once, it does not wait for a click. + h.run_steps(1); + assert!(h.state().session.get(id).unwrap().page_images(0).is_empty(), "the picture is gone"); + assert_eq!(h.state().views[i].picked_image, None, "and so is its highlight"); +} + +/// The highlight goes in the very edit that removes the picture, not on a later frame: the window +/// only redraws on the next input, so waiting for one leaves the highlight on screen until a click. +#[test] +fn an_edit_clears_the_picked_picture_at_once() { + let (_, mut h) = picture_harness(); + let i = h.state().active.unwrap(); + let at = h.state().views[i].page_screen_rect(0).expect("on screen").center(); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + assert!(h.state().views[i].picked_image.is_some(), "picked with the Select tool"); + let delete = pdfcraft_engine::Edit::EditPageImage { page: 0, index: 0, change: pdfcraft_engine::ImageEdit::Delete }; + assert!(h.state_mut().apply_edit(delete)); + // No frame has run since the edit. + assert_eq!(h.state().views[i].picked_image, None); +} + #[test] fn right_click_on_a_picture_copies_it_at_full_size() { let (px, mut h) = picture_harness(); @@ -1781,3 +1860,27 @@ fn arrow_keys_follow_the_scroll_speed_preference() { let (slow, normal, fast) = (lines_to_page_2("slow"), lines_to_page_2("normal"), lines_to_page_2("fast")); assert!(fast * 3 < normal * 2 && normal * 3 < slow * 2, "presses to page 2: slow {slow}, normal {normal}, fast {fast}"); } + +/// #2665: next to a very wide page the normal pages sit far to its right, outside the window. +/// Going to one changed the page counter but kept the horizontal scroll, so it stayed out of +/// sight; as in Acrobat, it is brought into view. +#[test] +fn go_to_page_brings_the_page_into_view_horizontally() { + use pdfcraft_ui_egui::canvas::Fit; + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("wide.pdf", None, sized_fixture(&[(595, 842), (595, 842), (595, 842), (6000, 2000)])).unwrap(); + app.views[0].fit = Fit::None; + app.views[0].zoom = 1.0; + app + }); + h.run_steps(4); + assert!(h.state().views[0].page_screen_rect(0).is_none(), "page 1 starts off-screen, right of the window"); + h.state_mut().views[0].go_to_page(2); + h.run_steps(4); + let v = &h.state().views[0]; + assert_eq!(v.current, 2); + let (r, vp) = (v.page_screen_rect(2).expect("page 3 on screen"), v.viewport_rect()); + assert!(r.left() >= vp.left() && r.right() <= vp.right(), "page 3 at {r:?}, viewport {vp:?}"); +}