From 19625d40a83d5be9837a63b7075eefa456aba332 Mon Sep 17 00:00:00 2001 From: LloydNicholson <34131158+LloydNicholson@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:19:10 +0200 Subject: [PATCH] M3: a newly opened document starts at the top, not where the last session scrolled (#554) Opening a PDF after a restart showed it part-way down, at the page some other document had been left on in the previous session. Since #259 (#189) the page view's ScrollArea is salted with the document's DocId, so each tab keeps its own offset. eframe persists egui's memory (persist_egui_memory defaults to true) to app.ron every 30 s and on quit, scroll offsets included, and restores it at the next launch. DocIds start again at 1 every launch, so the first document opened inherited the offset stored under id 1 last time, the second the one under id 2, and so on. A DocView now notes when its page view is first shown, and that frame sets the scroll offset to zero. Navigation queued before the first frame still wins: a zoom anchor or `goto` (--page, the PDF's open action, reopening the last session's files at their pages, #462) is applied on top. Tabs that were already shown keep their own position, as #189 needs. egui's other persisted memory (panel sizes and the like) is untouched. - canvas: `DocView::shown`, reset of the offset on the first frame - tests/viewer.rs: a_new_document_opens_at_the_top_after_a_relaunch saves egui's memory through eframe's own RON storage path after scrolling a.pdf to page 4, restores it in a new app, opens c.pdf (DocId 1) and checks it starts on page 1 (it opened on page 4 before the fix) and then keeps its own position - parity: the test is evidence on view.document-tabs Refs #189, #259, #462. Co-authored-by: lloyd Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 9 ++++++ crates/ui-egui/tests/viewer.rs | 53 ++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +-- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index a9b8faf4d..2be170187 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -240,6 +240,8 @@ pub struct DocView { viewport_screen: Rect, /// Pending zoom anchor: page, position within it (0..1), and offset from the viewport corner. zoom_anchor: Option<(usize, f32, f32, Vec2)>, + /// The page view has been shown, so its scroll offset in egui's memory is this document's. + shown: bool, /// Turns wheel input into page turns in single-page view. wheel: crate::wheel_pager::WheelPager, pub(crate) auto_scroll: crate::autoscroll::AutoScroll, @@ -379,6 +381,7 @@ impl DocView { screen_xforms: Vec::new(), viewport_screen: Rect::NOTHING, zoom_anchor: None, + shown: false, wheel: Default::default(), auto_scroll: Default::default(), selected: BTreeSet::new(), @@ -1326,6 +1329,12 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { }); // Each document keeps its own scroll position when switching tabs (#189). scroll = scroll.id_salt(("page-view", view.id)); + // A newly opened document starts at the top. eframe saves egui's memory with the settings + // and document ids start again at 1 every launch, so the offset stored under this id can be + // where an earlier session left another document. + if !std::mem::replace(&mut view.shown, true) { + scroll = scroll.scroll_offset(Vec2::ZERO); + } if let Some((page, fx, fy, rel)) = view.zoom_anchor.take() { let r = rects[page.min(rects.len() - 1)]; let point = pos2(r.left() + fx * r.width(), r.top() - y_shift + fy * r.height()); diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index ffbabc10c..312f6b356 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -427,6 +427,59 @@ fn each_document_keeps_its_own_scroll_position() { assert_eq!(show(&mut h, 1), 1); } +#[test] +fn a_new_document_opens_at_the_top_after_a_relaunch() { + // eframe saves egui's memory, scroll offsets included, with the settings and restores it at + // the next launch, where document ids start again at 1: the first document opened then + // started at the offset the previous session's first document was left at. + use pdfcraft_ui_egui::canvas::Fit; + let mut h = harness(); + for v in &mut h.state_mut().views { + v.fit = Fit::Width; + } + h.state_mut().active = Some(0); + h.run_steps(4); + h.state_mut().views[0].goto = Some((3, 0.0)); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 3, "a.pdf scrolled to page 4"); + // Quit: eframe writes egui's memory to app.ron, as below, and reads it back at the next launch. + #[derive(Default)] + struct Settings(std::collections::HashMap); + impl eframe::Storage for Settings { + fn get_string(&self, key: &str) -> Option { + self.0.get(key).cloned() + } + fn set_string(&mut self, key: &str, value: String) { + self.0.insert(key.into(), value); + } + fn remove_string(&mut self, key: &str) { + self.0.remove(key); + } + fn flush(&mut self) {} + } + let mut settings = Settings::default(); + h.ctx.memory(|m| eframe::set_value(&mut settings, "egui", m)); + let memory: egui::Memory = eframe::get_value(&settings, "egui").expect("egui's memory round-trips"); + // Next launch: a document never seen before, which gets a.pdf's old id. + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|cc| { + cc.egui_ctx.memory_mut(|m| *m = memory); + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("c.pdf", None, fixture(5)).unwrap(); + app.views[0].fit = Fit::Width; + app + }); + h.run_steps(4); + assert_eq!(h.state().views[0].id, pdfcraft_engine::DocId(1)); + assert_eq!(h.state().views[0].current, 0, "c.pdf opens on page 1"); + // It is still a scroll position of its own from then on. + h.state_mut().views[0].goto = Some((2, 0.0)); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 2); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 2, "c.pdf stays on page 3"); +} + #[test] fn arrow_and_page_keys_move_through_a_scrolling_document() { // #185: in the continuous (default) and two-page views the arrow keys and Page Down / Up did diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 26371f1bf..eec71e9e3 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1178,8 +1178,8 @@ milestone = "M3" status = "shipped" commands = [] tools = ["doc_list"] -evidence = ["crates/ui-egui/tests/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/control.rs::state_and_view_options", "crates/ui-egui/tests/viewer.rs::each_document_keeps_its_own_scroll_position"] -notes = "each tab keeps its own scroll position (#189)" +evidence = ["crates/ui-egui/tests/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/control.rs::state_and_view_options", "crates/ui-egui/tests/viewer.rs::each_document_keeps_its_own_scroll_position", "crates/ui-egui/tests/viewer.rs::a_new_document_opens_at_the_top_after_a_relaunch"] +notes = "each tab keeps its own scroll position (#189), and a newly opened one starts at the top, not where an earlier session left the scroll" [[feature]] id = "view.multiple-windows"