Hold the restored scroll on its line while wrap counts settle (spec §2.4)
On device (Pixel 9 Pro) the relaunch landed further UP than the saved position: while the restore scroll is still armed (scale + size + content can take ~600 ms), the top-of-file window is what gets rendered, and its shaping feedback — real wrap counts for lines ABOVE the restored line — lands before or just after the offset is applied. The counts are correct data, but they change V(line), so the line-derived offset (synthesized for the all-estimate index) maps to a shallower line and the viewport drifts up; the save then persists the drifted line and every subsequent relaunch lands there. Fix: the restore pins the logical line. Until the restored window's own shaping arrives (or a 2 s timeout, or the user scrolls / a search jumps), every accepted wrap correction re-derives the offset as VisualsBefore(line)*lh + sub under the corrected index. The pin refresh does not clamp to MaxScroll: the correction just grew the index, so the pre-layout clamp value is stale and would under-clamp the re-derived offset (the layout of the emitted frame clamps to the fresh value). Also: the apply-time offset is mapped through VisualsBefore under the current index (identical to line*lh while the index is all estimates), so pre-apply corrections are absorbed instead of gated away. Verified on device: saved line 420 -> restored 420 (was 333); saved 573 (near EOF, clamp territory) -> restored 573. New e2e regression TestRestore_LinePinHoldsAcrossLateWrapFeedback replays the late top-window feedback and fails pre-fix (window drifts 200 -> 66).
This commit is contained in:
parent
6968f6a284
commit
0f1b6e6290
|
|
@ -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).
|
||||
|
|
|
|||
10
doc/spec.md
10
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
|
||||
|
|
|
|||
|
|
@ -114,6 +114,18 @@ type Logic struct {
|
|||
// the stat result, and is not a content-arrival signal).
|
||||
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)
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user