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"