diff --git a/doc/architecture.md b/doc/architecture.md index 6f2ea7b..4cb426e 100644 --- a/doc/architecture.md +++ b/doc/architecture.md @@ -302,8 +302,24 @@ Only the visible byte range is shaped and drawn each frame: 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. + the restore re-derives the offset as `line·lh + sub` — mapped through + `VisualsBefore(line)` under the index as it stands at apply time. The + raw offset is kept as the fallback for pre-line-coordinate session + files. The pin is still needed after apply: on device the first frames + after relaunch render the TOP-OF-FILE window (the scroll is armed until + scale+size+content are all known, which can be ~600 ms), and that + window's shaping feedback — real wrap counts for the lines ABOVE the + restored line — lands before or just after the offset is applied. + Those counts are correct data, but they change `V(line)`: the offset + synthesized for the estimate index then maps to a shallower line (the + on-device "lands further up" report). So while the restore settles, + every accepted correction re-derives the offset as + `VisualsBefore(line)·lh + sub` (no MaxScroll clamp — the correction + just grew the index, so the pre-layout clamp value is stale; the + layout of the emitted frame clamps to the fresh one), and the pin + stands down when the restored window's own feedback arrives, after a + 2 s timeout, or when the user scrolls or a search takes over the + viewport. - **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). diff --git a/doc/spec.md b/doc/spec.md index 99c1aab..e9a0781 100644 --- a/doc/spec.md +++ b/doc/spec.md @@ -123,7 +123,15 @@ elsewhere. 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. + deeper by every wrapped continuation above it. The inverse drift also + happens in the first frames after relaunch: the pre-restore (top-of-file) + window is what gets shaped first, and its real wrap counts landing below + the restored line would drag the line-derived offset to a shallower line. + The restore therefore pins the logical line: while the restore settles + (until the restored window itself has shaped, a 2 s timeout, or the user + scrolls / a search jumps), every wrap-count correction re-derives the + offset as `V(line)·lh + sub` under the current index, so the viewport + stays on the restored line regardless of which counts have landed. - 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 diff --git a/internal/editor/logic.go b/internal/editor/logic.go index 88656fd..2a89c44 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -76,8 +76,8 @@ type Logic struct { // flushSession: one-shot request to persist the session snapshot now // (the OS activity onStop hook, see FlushSession). Buffered 1 so the // requester never blocks, even if an earlier flush is still queued. - flushSession chan struct{} - inspectChan chan *inspectReq + flushSession chan struct{} + inspectChan chan *inspectReq // Per-file write protocol (see requestSave). Workers are a shared pool and // the on-disk staging file is per-file, so two concurrent writes for the @@ -112,8 +112,20 @@ type Logic struct { // scaleSeen tracks the first ScaleEvent; restoreContentLanded marks the // read result that filled the buffer (FileLen alone is set earlier, by // the stat result, and is not a content-arrival signal). - restoreScroll ui.Dp - restoreScrollArmed bool + restoreScroll ui.Dp + restoreScrollArmed bool + // restoreScrollLine/restoreScrollSub/restorePinDeadline implement the + // restore line-pin (see refreshRestorePin in session.go): the + // line-derived scroll offset is only consistent with the all-estimate + // WrapIndex, so while relaunch restore is settling, every wrap-count + // correction landing below the pinned line shifts the offset-to-line + // mapping and would drag the viewport off the restored line. Until the + // restored window itself has shaped (or a timeout, or the user or a + // search takes over), the offset is re-derived from the pinned line + // after each correction. + restoreScrollLine int + restoreScrollSub float64 + restorePinDeadline time.Time scaleSeen bool restoreContentLanded bool sessionSaver func(SessionState) @@ -260,7 +272,7 @@ func (l *Logic) Run() { case <-l.flushSession: // Persist now, bypassing the rate limit: the activity is going // away (recents-wipe or app switch) and the process may die - // shortly after this returns. + // shortly after this returns. l.flushSessionSave() case update := <-l.configChan: update.apply(l.state) @@ -283,6 +295,18 @@ func (l *Logic) Run() { // lines since shaping (fb.EditSeq correlates with the content). if fb.EditSeq == l.state.Editor.EditSeq { l.state.applyWrapCounts(fb) + // Restore line-pin (see refreshRestorePin): a correction + // landing below the pinned line shifted the offset-to-line + // mapping, so re-derive the offset from the pinned line under + // the corrected index. The restored window's own shaping means + // its neighborhood is real and the pin can stand down. + if l.restoreScrollLine >= 0 { + if fb.WindowStartLine == l.restoreScrollLine { + l.restoreScrollLine = -1 + } else { + l.refreshRestorePin() + } + } // Search settle (see EditorState.findSettle): the shaping above // may have corrected the wrap counts around a find-jumped // viewport; re-scroll while the correction still matters. diff --git a/internal/editor/search.go b/internal/editor/search.go index 75dfba7..e83fe05 100644 --- a/internal/editor/search.go +++ b/internal/editor/search.go @@ -355,6 +355,9 @@ func scrollToFindMatch(absByte int) { e.Find.SettleByte = -1 return } + if TheLogic != nil { + TheLogic.releaseRestorePin() // search takes over the viewport + } TheState.ScrollOffset = target if e.Find.Visible { e.Find.SettleByte = absByte diff --git a/internal/editor/session.go b/internal/editor/session.go index 3ab0bf2..d1e1095 100644 --- a/internal/editor/session.go +++ b/internal/editor/session.go @@ -243,6 +243,7 @@ func (l *Logic) abortRestore() { l.restoreFile = "" l.restoreScrollArmed = false l.restoreContentLanded = false + l.restoreScrollLine = -1 l.session = SessionState{} } @@ -281,12 +282,30 @@ func (l *Logic) maybeApplyRestoreScroll() bool { // 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 { + s := l.session + if 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) + // The offset for a logical line is V(L)*lh + sub under the CURRENT + // WrapIndex (V(L) = L while the index is all estimates). Wrapping + // below the line was already corrected before the offset could be + // applied (pre-apply frames shaped the top-of-file window), so + // mapping through V(L) lands on the line under either state. + base := float64(s.ScrollLine) + if cb := l.state.Editor.ChunkedBuffer; cb != nil && cb.WrapIndex != nil && s.ScrollLine < cb.WrapIndex.Len() { + base = float64(cb.WrapIndex.VisualsBefore(s.ScrollLine)) + } + l.restoreScroll = ui.Dp(base*float64(lh) + s.ScrollSub) + // Arm the line-pin (see refreshRestorePin): wrap-count corrections + // landing below this line would shift the mapping and drag the + // viewport off the restored line while the index settles. + l.restoreScrollLine = s.ScrollLine + l.restoreScrollSub = float64(s.ScrollSub) + l.restorePinDeadline = time.Now().Add(restorePinTimeout) + } else { + l.restoreScrollLine = -1 } if l.restoreScroll > l.state.MaxScroll { l.restoreScroll = l.state.MaxScroll @@ -299,6 +318,53 @@ func (l *Logic) maybeApplyRestoreScroll() bool { return true } +// restorePinTimeout bounds the line-pin: after this long the index around +// the restored window has either settled or the user has moved on. +const restorePinTimeout = 2 * time.Second + +// releaseRestorePin clears the restore line-pin (user or search took over +// the viewport). Must be called on the logic goroutine. +func (l *Logic) releaseRestorePin() { + l.restoreScrollLine = -1 +} + +// refreshRestorePin re-derives the scroll offset from the pinned line under +// the CURRENT WrapIndex (V(L)*lh + sub, the same mapping the layout uses), +// so wrap-count corrections landing below the pinned line cannot drag the +// viewport off it. Releases the pin when the deadline passes. Must be +// called on the logic goroutine, after a wrap correction has been applied. +func (l *Logic) refreshRestorePin() { + if l.restoreScrollLine < 0 { + return + } + if time.Now().After(l.restorePinDeadline) { + l.restoreScrollLine = -1 + return + } + cb := l.state.Editor.ChunkedBuffer + if cb == nil || cb.WrapIndex == nil { + return + } + w := cb.WrapIndex + if l.restoreScrollLine >= w.Len() { + return + } + lh := EffectiveLineHeight() + if gl := l.state.Editor.GlyphLayout; gl.LineHeight > 0 { + lh = gl.LineHeight + } + // No MaxScroll clamp here: the correction that triggered this refresh + // just grew the index, so the layout's MaxScroll (computed before it) is + // stale and would under-clamp the re-derived offset; the layout pass of + // the emitted frame clamps to the fresh value. An edit shrinking the + // file mid-pin is covered the same way. + off := ui.Dp(float64(w.VisualsBefore(l.restoreScrollLine))*float64(lh) + l.restoreScrollSub) + if off != l.state.ScrollOffset { + l.state.ScrollOffset = off + l.emitFrame() + } +} + // applyRestorePositions clamps the restored snapshot's cursor and selection // to a file of n bytes and applies them. Must be called on the logic // goroutine. diff --git a/internal/editor/state.go b/internal/editor/state.go index 843eb9b..0a0305f 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -350,6 +350,9 @@ func HandleScroll(data any) { return } delta := data.(int) // pixels + if TheLogic != nil { + TheLogic.releaseRestorePin() // the user takes over the viewport + } TheState.ScrollOffset += ui.ToDp(ui.Px(delta), TheState.scale) if TheState.ScrollOffset < 0 { TheState.ScrollOffset = 0 diff --git a/internal/test/e2e/restore_test.go b/internal/test/e2e/restore_test.go index ff51ac1..5aaff8d 100644 --- a/internal/test/e2e/restore_test.go +++ b/internal/test/e2e/restore_test.go @@ -616,6 +616,143 @@ got: } } +// TestRestore_LinePinHoldsAcrossLateWrapFeedback reproduces the on-device +// report "relaunch lands further UP than where I left off" (Pixel 9 Pro): +// while the restore scroll is still armed, the top-of-file window is what's +// rendered, and its shaping feedback (real wrap counts for the lines ABOVE +// the restored line) can land before or just after the offset is applied. +// Those counts are correct data, but they change the offset-to-line mapping: +// the line-derived offset was synthesized for the all-estimate index, so +// once counts land below the pinned line the same offset maps to a shallower +// line and the viewport drifts up. The restore therefore pins the logical +// line: every wrap correction re-derives the offset as V(L)*lh + sub until +// the restored window itself has shaped (or a timeout, or the user scrolls). +func TestRestore_LinePinHoldsAcrossLateWrapFeedback(t *testing.T) { + const lines = 300 + const lineLen = 17 // "line %03d content\n" + 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) + } + + // The harness runs at the default font scale, so the stateless variant + // matches what the owner will compute. + lh := float64(editor.EffectiveLineHeightAt(1)) + snap := editor.SessionState{ + File: "/wrap.txt", + Cursor: 0, + Scroll: float64(ui.Dp(500 * lh)), // the saving session's visual-space offset + ScrollLine: 200, // the logical line at the viewport top + ScrollSub: 0, + } + + h := e2e.NewHarness(e2e.WithFileSystem(real.NewRealFileSystem(dir), "/")) + h.Logic().BeginRestore(snap) + h.Run() + h.SendConfig(780, 1688) + h.SendScale(2.0) + defer h.Cleanup() + + // Wait for the restore to land: with the fresh all-estimate index the + // offset maps to exactly line 200. + var winLine int + for i := 0; i < 100; i++ { + v, err := h.Inspect(func(st *editor.State) any { + cb := st.Editor.ChunkedBuffer + if cb == nil || cb.FileLen() == 0 || cb.LineIndex == nil { + return -1 + } + return st.WindowStartLine + }) + if err == nil { + winLine = v.(int) + if winLine == 200 { + break + } + } + time.Sleep(20 * time.Millisecond) + } + if winLine != 200 { + t.Fatalf("restore did not land on line 200 (window at %d)", winLine) + } + + // Replay the on-device interleaving: the top-of-file window (lines + // 0-49) was shaped while the scroll was still armed, and its feedback — + // real wrap counts (3 visual lines each for those lines) — arrives now, + // after the offset was applied. + editSeq, err := h.Inspect(func(st *editor.State) any { + return st.Editor.EditSeq + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + var wt strings.Builder + for i := 0; i < 50; i++ { + fmt.Fprintf(&wt, "line %03d content\n", i) + } + windowText := wt.String() + var starts []int + for i := 0; i < 50; i++ { + base := i * lineLen + starts = append(starts, base, base+5, base+10) // 3 visual lines per line + } + h.Logic().LayoutChan() <- ui.LayoutFeedback{ + GlyphLayout: ui.GlyphLayout{ + VisualLineStarts: starts, + }, + WindowText: windowText, + WindowStartByte: 0, + WindowStartLine: 0, + EditSeq: editSeq.(uint64), + } + + // The correction must NOT drag the viewport: the pin re-derives the + // offset as V(200)*lh, where V(200) = 50*3 + 150 = 300 under the + // corrected index, keeping line 200 at the top. Pre-fix, the applied + // offset (200*lh) mapped to line ~66 under the corrected index. + for i := 0; i < 100; i++ { + v, err := h.Inspect(func(st *editor.State) any { + return struct { + Scroll float64 + WinLine int + }{float64(st.ScrollOffset), st.WindowStartLine} + }) + if err == nil { + got := v.(struct { + Scroll float64 + WinLine int + }) + if got.WinLine == 200 && got.Scroll > 299*lh && got.Scroll < 301*lh { + return // held + } + } + time.Sleep(20 * time.Millisecond) + } + v, err := h.Inspect(func(st *editor.State) any { + return struct { + Scroll float64 + WinLine int + }{float64(st.ScrollOffset), st.WindowStartLine} + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + got := v.(struct { + Scroll float64 + WinLine int + }) + if got.WinLine != 200 { + t.Errorf("window at line %d after late wrap feedback, want 200: the line-pin must hold the viewport on the restored line (a shallower line is the pre-fix drift)", got.WinLine) + } + if got.Scroll < 499*lh || got.Scroll > 501*lh { + t.Errorf("scroll = %v after late wrap feedback, want ~300*lh (%v): V(200) under the corrected index", got.Scroll, 300*lh) + } +} + // 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