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 <lloyd@stackworx.io>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
LloydNicholson
2026-10-09 12:19:10 -04:00
committed by GitHub
co-authored by lloyd Claude Opus 5.5
parent cc17deb634
commit 19625d40a8
3 changed files with 64 additions and 2 deletions
+9
View File
@@ -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());
+53
View File
@@ -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<String, String>);
impl eframe::Storage for Settings {
fn get_string(&self, key: &str) -> Option<String> {
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
+2 -2
View File
@@ -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"