Merge track r1-view (fredapps-r1)

crates/ui-egui/src/canvas.rs

@@ -444,6 +444,9 @@ pub struct DocView {
     pub(crate) text_failed: HashSet<usize>,
     pub find: Option<Find>,
     selection: Option<Selection>,
+    /// 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<Selection>,
     /// 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<usize> = if rects.is_empty() {
-        Vec::new()
-    } else {
+    let shown_row = |view: &DocView| -> Vec<usize> {
+        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<usize> = 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 };

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();

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`.

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() {

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]

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(|_| {});

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]

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");
+}

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.

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:?}");
+}