Restore scroll by logical line, not pixel offset (wrap-aware)
On device: scroll far down a wrapped file, relaunch, and the app lands further DOWN than where the user left off — the deeper the scroll, the further off. Root cause: the persisted Scroll is a pixel offset in VISUAL-line space. Restoring maps it through the WrapIndex (scrollDecompose -> LineForVisual), but on relaunch every count is the estimate (1) until the line is shaped, and shaping covers the visible window only — the lines ABOVE the restored viewport are never shaped. With all-ones counts LineForVisual maps the offset 1:1, landing a logical line deeper by every wrapped continuation above the viewport, and the state is stable (the under-counted lines never re-enter the window), so it never self-corrects. The snapshot now persists wrap-independent coordinates: the logical line at the viewport top (derived with the same mapping the layout uses, against the current index, so it is exactly the shown line) plus the sub-line remainder. The restore re-derives the offset as line*lh + sub, which maps to the saved line under any wrap state (all-ones or populated). The raw Dp offset is kept for pre-line-coordinate session files (loadSession defaults the missing key to -1; BeginRestore rejects the ambiguous zero value: a genuine line-0 snapshot always has Scroll < lh). TestRestore_ScrollSurvivesWrapState reproduces it: 150 lines recorded as wrapped x3, viewport at logical line 200 (visual 500); a relaunch with a fresh WrapIndex must land the window on line 200. Fails pre-fix (window at line 254, i.e. deeper) and passes with the fix. Docs: spec §2.4 (line-based scroll persist + current save policy), architecture §6.7 (why the offset is unrestorable by re-mapping).
This commit is contained in:
parent
fdbffc9f5c
commit
6968f6a284
|
|
@ -423,6 +423,7 @@ func loadSession(path string) (editor.SessionState, bool) {
|
|||
return editor.SessionState{}, false
|
||||
}
|
||||
var s editor.SessionState
|
||||
s.ScrollLine = -1 // pre-line session files lack the key; 0 would restore to the top
|
||||
if err := json.Unmarshal(b, &s); err != nil {
|
||||
return editor.SessionState{}, false
|
||||
}
|
||||
|
|
@ -439,6 +440,12 @@ func loadSession(path string) (editor.SessionState, bool) {
|
|||
if s.FindCurByte < -1 {
|
||||
s.FindCurByte = -1
|
||||
}
|
||||
if s.ScrollLine < -1 {
|
||||
s.ScrollLine = -1
|
||||
}
|
||||
if s.ScrollSub < 0 {
|
||||
s.ScrollSub = 0
|
||||
}
|
||||
if s.File == "" {
|
||||
return editor.SessionState{}, false
|
||||
}
|
||||
|
|
|
|||
|
|
@ -292,6 +292,18 @@ Only the visible byte range is shaped and drawn each frame:
|
|||
- Max scroll is `TotalVisuals()·lh − regionH + lh/2` with the
|
||||
font-scale-effective line height; it grows incrementally as shaped
|
||||
counts arrive (pre-shaping it equals the no-wrap estimate).
|
||||
- **Relaunch snapshot (spec §2.4):** the counts are process-local (rebuilt
|
||||
from the all-ones estimate on relaunch; only the visible window is ever
|
||||
shaped), so a persisted pixel scroll offset cannot be restored by
|
||||
re-mapping — with every count above the restored viewport at the
|
||||
estimate, `LineForVisual` maps the offset 1:1 to a logical line deeper
|
||||
by all the wrapped continuations above it, with no self-correction
|
||||
(those lines never enter the window). The snapshot therefore persists
|
||||
the wrap-independent coordinates: the logical line at the viewport top
|
||||
(derived with the same scrollDecompose + LineForVisual mapping the
|
||||
layout uses, against the CURRENT index) plus the sub-line remainder;
|
||||
the restore re-derives the offset as `line·lh + sub`. The raw offset is
|
||||
kept as the fallback for pre-line-coordinate session files.
|
||||
- **Font-scale axis.** The shaper draws baselines in sp, so on Android the
|
||||
rendered line pitch in density-dp is `EditorLineHeight()*fontScale`
|
||||
(`fontScale = Metric.PxPerSp/PxPerDp`, the user font-size setting).
|
||||
|
|
|
|||
18
doc/spec.md
18
doc/spec.md
|
|
@ -114,18 +114,28 @@ elsewhere.
|
|||
### 2.4 State restoration on relaunch
|
||||
|
||||
- On launch, Pad re-opens the file from the last session and lands straight
|
||||
on the editor page, restoring the cursor, scroll offset, live selection,
|
||||
on the editor page, restoring the cursor, scroll position, live selection,
|
||||
and the find bar (query, open/closed, current match). A relaunch therefore
|
||||
never requires re-browsing to the last file.
|
||||
- The scroll position is persisted as the LOGICAL line at the viewport top
|
||||
plus the sub-line remainder (the raw pixel offset is kept too, for
|
||||
pre-line-coordinate session files). The pixel offset alone is not
|
||||
restorable: it lives in visual-line space, and the wrap counts that map
|
||||
it to a line are rebuilt from estimates on relaunch — lines above the
|
||||
restored viewport are never shaped, so the offset would land a line
|
||||
deeper by every wrapped continuation above it.
|
||||
- The snapshot is a tiny JSON file (a few hundred bytes) written by the
|
||||
cmd layer: `$HOME/.pad/session.json` off-Android, and
|
||||
`/storage/emulated/0/Pad/session.json` on Android (the dir that already
|
||||
carries the browser's `.pad` index caches). Search results are NOT stored:
|
||||
they are regenerated by re-scanning the restored query, and the current
|
||||
match is re-selected by byte offset.
|
||||
- The snapshot is written rate-limited (≤ 1/s, only on change) while the app
|
||||
runs and unconditionally at shutdown, so a kill shortly after a change
|
||||
loses at most ~1 s of state.
|
||||
- The snapshot is written rate-limited (250 ms, only on change) while the
|
||||
app runs; a file change or a selection appearing/vanishing saves
|
||||
immediately; on Android the activity's onStop (recents-wipe, app switch)
|
||||
flushes it one last time; and it is written unconditionally at shutdown.
|
||||
A kill shortly after a change therefore loses at most a fraction of a
|
||||
second of state.
|
||||
- If the restored file no longer exists (deleted/moved, e.g. by Syncthing),
|
||||
the app falls back to the browser page; positions that exceed a shrunk
|
||||
file are clamped to its new end.
|
||||
|
|
|
|||
|
|
@ -40,10 +40,23 @@ const sessionSaveInterval = 250 * time.Millisecond
|
|||
// SessionState is the relaunch snapshot (see the file doc above). All
|
||||
// positions are absolute file byte offsets; Scroll is in Dp. It is a plain
|
||||
// comparable value (used with == for change detection).
|
||||
//
|
||||
// Scroll is backed by ScrollLine: the pixel offset lives in VISUAL-line
|
||||
// space (a wrapped line occupies several visual lines, and the scroll-to-
|
||||
// line mapping runs through the WrapIndex), but on relaunch the counts of
|
||||
// every line above the restored viewport are still the estimate (1) until
|
||||
// the line is shaped, and lines above the visible window are never shaped.
|
||||
// Mapping the saved offset through that fresh index would land a logical
|
||||
// line DEEPER by all the wrapped continuations above the viewport, with no
|
||||
// self-correction. The logical line at the viewport top and the sub-line
|
||||
// remainder are wrap-independent, so they are the restorable unit; Scroll
|
||||
// is kept as the pre-line-coordinate value (legacy session files).
|
||||
type SessionState struct {
|
||||
File string // last opened file ("" = nothing to restore)
|
||||
Cursor int // cursor byte offset
|
||||
Scroll float64 // editor scroll offset, Dp
|
||||
ScrollLine int // logical line at the viewport top (-1 = unknown)
|
||||
ScrollSub float64 // Scroll's sub-line remainder, Dp (0 <= r < lineHeight)
|
||||
SelStart int // selection start byte (-1 = no selection)
|
||||
SelEnd int // selection end byte, exclusive
|
||||
FindQuery string // find bar query ("" = none)
|
||||
|
|
@ -77,10 +90,30 @@ func (l *Logic) SnapshotSession() SessionState {
|
|||
if f.Cur >= 0 && f.Cur < len(f.Matches) {
|
||||
curByte = f.Matches[f.Cur][0]
|
||||
}
|
||||
// Persist the scroll in wrap-independent coordinates (see the
|
||||
// SessionState doc). The derivation mirrors visibleByteRangePrecise
|
||||
// (scrollDecompose, then LineForVisual against the CURRENT index), so
|
||||
// the line is exactly the one the layout is showing at this offset.
|
||||
scrollLine := -1
|
||||
var scrollSub float64
|
||||
if cb := e.ChunkedBuffer; cb != nil && !e.TooLarge {
|
||||
lh := EffectiveLineHeight()
|
||||
if gl := e.GlyphLayout; gl.LineHeight > 0 {
|
||||
lh = gl.LineHeight
|
||||
}
|
||||
v0, r0 := scrollDecompose(s.ScrollOffset, lh)
|
||||
scrollLine = int(v0)
|
||||
if w := cb.WrapIndex; w != nil {
|
||||
scrollLine = w.LineForVisual(int32(v0))
|
||||
}
|
||||
scrollSub = r0
|
||||
}
|
||||
return SessionState{
|
||||
File: e.Filename,
|
||||
Cursor: cur,
|
||||
Scroll: float64(s.ScrollOffset),
|
||||
ScrollLine: scrollLine,
|
||||
ScrollSub: scrollSub,
|
||||
SelStart: e.SelectionStart,
|
||||
SelEnd: e.SelectionEnd,
|
||||
FindQuery: f.Query,
|
||||
|
|
@ -171,6 +204,14 @@ func (l *Logic) BeginRestore(s SessionState) {
|
|||
if s.File == "" {
|
||||
return
|
||||
}
|
||||
// The zero-value ScrollLine (0) is ambiguous: a genuine top-of-file
|
||||
// snapshot always has Scroll < lineHeight (no line above line 0
|
||||
// contributes visual lines), so line 0 with a deep offset is an unset
|
||||
// field (an in-process caller or test built the snapshot without it) —
|
||||
// fall back to the pixel offset rather than snapping to the top.
|
||||
if s.ScrollLine == 0 && s.Scroll >= 2*float64(EffectiveLineHeight()) {
|
||||
s.ScrollLine = -1
|
||||
}
|
||||
l.session = s
|
||||
e := &l.state.Editor
|
||||
e.Filename = s.File
|
||||
|
|
@ -232,6 +273,21 @@ func (l *Logic) maybeApplyRestoreScroll() bool {
|
|||
if l.state.Page() != EditorPage || !l.restoreContentLanded {
|
||||
return false
|
||||
}
|
||||
// Re-derive the offset from the persisted logical line (see the
|
||||
// SessionState doc): the snapshot's pixel offset mapped to its position
|
||||
// only while the saving session's WrapIndex was valid. On relaunch the
|
||||
// counts above the restored viewport are all the estimate until shaped,
|
||||
// and those lines never are (shaping covers the visible window only),
|
||||
// so the raw offset would land a line deeper by every wrapped
|
||||
// continuation above the viewport. The line-based offset lands on the
|
||||
// saved line regardless of wrap state.
|
||||
if s := l.session; s.ScrollLine >= 0 {
|
||||
lh := EffectiveLineHeight()
|
||||
if gl := l.state.Editor.GlyphLayout; gl.LineHeight > 0 {
|
||||
lh = gl.LineHeight
|
||||
}
|
||||
l.restoreScroll = ui.Dp(float64(s.ScrollLine)*float64(lh) + s.ScrollSub)
|
||||
}
|
||||
if l.restoreScroll > l.state.MaxScroll {
|
||||
l.restoreScroll = l.state.MaxScroll
|
||||
}
|
||||
|
|
|
|||
|
|
@ -472,6 +472,150 @@ func TestRestore_ShutdownSavesFinalSnapshot(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestRestore_ScrollSurvivesWrapState reproduces the on-device report
|
||||
// "relaunch lands further DOWN than where I left off": the snapshot's pixel
|
||||
// scroll lives in visual-line space (wrapped lines occupy several visual
|
||||
// lines, and the scroll-to-line mapping runs through the WrapIndex), but on
|
||||
// relaunch the counts of every line above the restored viewport are the
|
||||
// estimate (1) until shaped, and lines above the visible window are never
|
||||
// shaped. Mapping the saved offset through that fresh index lands a logical
|
||||
// line deeper by all the wrapped continuations above it. The snapshot
|
||||
// therefore persists the logical line at the viewport top (and the sub-line
|
||||
// remainder), and the restore derives the offset from the line.
|
||||
func TestRestore_ScrollSurvivesWrapState(t *testing.T) {
|
||||
const lines = 300
|
||||
var b strings.Builder
|
||||
for i := 0; i < lines; i++ {
|
||||
fmt.Fprintf(&b, "line %03d content\n", i)
|
||||
}
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "wrap.txt"), []byte(b.String()), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// First session: open the file, record the first 150 lines as wrapped
|
||||
// into 3 visual lines each (as a shaping pass would), and scroll so the
|
||||
// viewport top sits on logical line 200. In visual space that is line
|
||||
// 150*3 + 50 = 500, so the pixel offset is 500 line-heights.
|
||||
saves := make(chan editor.SessionState, 16)
|
||||
h1 := e2e.NewHarness(e2e.WithFileSystem(real.NewRealFileSystem(dir), "/"))
|
||||
h1.Logic().SetSessionSaver(func(s editor.SessionState) { saves <- s })
|
||||
h1.Run()
|
||||
h1.SendConfig(780, 1688)
|
||||
h1.SendScale(2.0)
|
||||
defer h1.Cleanup()
|
||||
|
||||
if err := h1.WithState(func(st *editor.State) { editor.OpenFile("/wrap.txt") }); err != nil {
|
||||
t.Fatalf("OpenFile: %v", err)
|
||||
}
|
||||
for i := 0; i < 100; i++ {
|
||||
v, err := h1.Inspect(func(st *editor.State) any {
|
||||
cb := st.Editor.ChunkedBuffer
|
||||
return cb != nil && cb.FileLen() > 0 && cb.LineIndex != nil
|
||||
})
|
||||
if err == nil && v.(bool) {
|
||||
break
|
||||
}
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
}
|
||||
// Set the wrapped counts and the deep scroll on the owner; the line
|
||||
// height comes from the owner too (EffectiveLineHeight reads state).
|
||||
lhAny, err := h1.Inspect(func(st *editor.State) any {
|
||||
w := st.Editor.ChunkedBuffer.WrapIndex
|
||||
for i := 0; i < 150; i++ {
|
||||
w.Set(i, 3)
|
||||
}
|
||||
st.ScrollOffset = ui.Dp(500 * float64(editor.EffectiveLineHeight()))
|
||||
return float64(editor.EffectiveLineHeight())
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Inspect: %v", err)
|
||||
}
|
||||
lh := lhAny.(float64)
|
||||
// Wait past the rate limit and force a frame: the saver must capture
|
||||
// the deep scroll together with its logical line (200). Match on the
|
||||
// line, not the exact Dp: ui.Dp is float32-based, so ui.Dp(500*lh) and
|
||||
// 500*float64(lh) round differently.
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
h1.SendConfig(780, 1688)
|
||||
snap := editor.SessionState{}
|
||||
deadline := time.After(5 * time.Second)
|
||||
for {
|
||||
select {
|
||||
case s := <-saves:
|
||||
if s.File == "/wrap.txt" && s.ScrollLine == 200 && s.Scroll > 0 {
|
||||
snap = s
|
||||
goto got
|
||||
}
|
||||
case <-deadline:
|
||||
t.Fatal("timed out: saver did not capture the deep-scroll snapshot")
|
||||
}
|
||||
}
|
||||
got:
|
||||
if snap.Scroll < 499*lh || snap.Scroll > 501*lh {
|
||||
t.Fatalf("snapshot Scroll = %v, want ~500*lh (%v): the pixel offset must stay the visual-space value",
|
||||
snap.Scroll, 500*lh)
|
||||
}
|
||||
if snap.ScrollLine != 200 {
|
||||
t.Fatalf("snapshot ScrollLine = %d, want 200 (the logical line at the viewport top under wrap)", snap.ScrollLine)
|
||||
}
|
||||
if snap.ScrollSub >= 0.01 { // 0 apart from ui.Dp float32 rounding
|
||||
t.Errorf("snapshot ScrollSub = %v, want ~0", snap.ScrollSub)
|
||||
}
|
||||
|
||||
// Second session (the relaunch): the WrapIndex is fresh, every count the
|
||||
// estimate (1). The restore must land on logical line 200, not on visual
|
||||
// line 500 mapped 1:1 (which would be line 500, far deeper).
|
||||
h2 := e2e.NewHarness(e2e.WithFileSystem(real.NewRealFileSystem(dir), "/"))
|
||||
h2.Logic().BeginRestore(snap)
|
||||
h2.Run()
|
||||
h2.SendConfig(780, 1688)
|
||||
h2.SendScale(2.0)
|
||||
defer h2.Cleanup()
|
||||
|
||||
// The restore must land the scroll within one line of 200*lh (the ui.Dp
|
||||
// float32 rounding keeps an exact match out of reach); the exact
|
||||
// assertion is the window start line below.
|
||||
for i := 0; i < 100; i++ {
|
||||
v, err := h2.Inspect(func(st *editor.State) any {
|
||||
cb := st.Editor.ChunkedBuffer
|
||||
if cb == nil || cb.FileLen() == 0 || cb.LineIndex == nil {
|
||||
return false
|
||||
}
|
||||
s := float64(st.ScrollOffset)
|
||||
return s > 199*lh && s < 201*lh
|
||||
})
|
||||
if err == nil && v.(bool) {
|
||||
break
|
||||
}
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
}
|
||||
// One more frame so the layout pass records the window start line for
|
||||
// the landed scroll.
|
||||
h2.SendConfig(780, 1688)
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
v, err := h2.Inspect(func(st *editor.State) any {
|
||||
return struct {
|
||||
Scroll ui.Dp
|
||||
WinLine int
|
||||
}{st.ScrollOffset, st.WindowStartLine}
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Inspect: %v", err)
|
||||
}
|
||||
got := v.(struct {
|
||||
Scroll ui.Dp
|
||||
WinLine int
|
||||
})
|
||||
if s := float64(got.Scroll); s < 199*lh || s > 201*lh {
|
||||
t.Errorf("restored scroll = %v, want ~200*lh (%v): the saved logical line's offset",
|
||||
got.Scroll, 200*lh)
|
||||
}
|
||||
if got.WinLine != 200 {
|
||||
t.Errorf("window starts at line %d, want 200 (the saved line); a deeper line is the pre-fix visual-space mapping", got.WinLine)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRestore_UrgentSaveOnSelection verifies the kill-race fix: a selection
|
||||
// appearing (the user just highlighted text) saves immediately, even inside
|
||||
// the rate-limit window that the file-open save opened. Without the urgent
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user