diff --git a/cmd/pad/main.go b/cmd/pad/main.go index 7583f32..a26c305 100644 --- a/cmd/pad/main.go +++ b/cmd/pad/main.go @@ -366,12 +366,24 @@ func run(w *app.Window) error { if sendFind { logic.FindQueryChan() <- newFind } - logic.LayoutChan() <- ui.LayoutFeedback{ - GlyphLayout: glyphLayout, - WindowText: frame.WindowText, - WindowStartByte: frame.WindowStartByte, - WindowStartLine: frame.WindowStartLine, - EditSeq: frame.EditSeq, + // Skip layout feedback for frames built before the window size + // was known (frame.ViewportDegenerate, set at frame-build time — + // feedback delivery lags shaping by a frame, so checking the + // current size here would miss them): a zero-width shape wraps + // every line into many visual lines, and feeding those counts + // back would poison the WrapIndex for the window's lines + // (applied once, corrected only if those lines are re-shaped at a + // real width — a restored scroll that moves the viewport away + // never re-shapes them, and the poisoned counts then map a + // legitimate scroll offset to the wrong line). + if !frame.ViewportDegenerate { + logic.LayoutChan() <- ui.LayoutFeedback{ + GlyphLayout: glyphLayout, + WindowText: frame.WindowText, + WindowStartByte: frame.WindowStartByte, + WindowStartLine: frame.WindowStartLine, + EditSeq: frame.EditSeq, + } } default: handleEvent(e) diff --git a/internal/editor/frame.go b/internal/editor/frame.go index d895844..0327780 100644 --- a/internal/editor/frame.go +++ b/internal/editor/frame.go @@ -45,23 +45,31 @@ type Frame struct { WindowStartLine int // -1 when the frame has no editor window WindowText string // the editor window this frame's text element holds EditSeq uint64 + // ViewportDegenerate is set when this frame was built before the + // window's pixel size was known (0x0). Its editor window, if shaped at + // all, was shaped at zero width: the shaper wraps every line into many + // visual lines, and feeding those counts back (LayoutFeedback) would + // poison the WrapIndex for the window's lines. The main goroutine + // drops the feedback for such frames. + ViewportDegenerate bool } // frameOf wraps a computed element tree with the current view-state // snapshot. Must be called on the logic goroutine. func (l *Logic) frameOf(elems []ui.Element) Frame { return Frame{ - Elems: elems, - Scale: l.state.scale, - FontScale: l.state.fontScale, - FocusedElementID: l.state.FocusedElementID, - Query: l.state.Browser.Query, - FindQuery: l.state.Editor.Find.Query, - FindClearSeq: l.state.Editor.Find.ClearSeq, - WindowStartByte: l.state.Editor.IMEWindowStartByte, - WindowStartLine: l.state.WindowStartLine, - WindowText: l.state.Editor.IMEWindowText, - EditSeq: l.state.Editor.EditSeq, + Elems: elems, + Scale: l.state.scale, + FontScale: l.state.fontScale, + FocusedElementID: l.state.FocusedElementID, + Query: l.state.Browser.Query, + FindQuery: l.state.Editor.Find.Query, + FindClearSeq: l.state.Editor.Find.ClearSeq, + WindowStartByte: l.state.Editor.IMEWindowStartByte, + WindowStartLine: l.state.WindowStartLine, + WindowText: l.state.Editor.IMEWindowText, + EditSeq: l.state.Editor.EditSeq, + ViewportDegenerate: l.state.PixelWidth <= 0 || l.state.PixelHeight <= 0, } } diff --git a/internal/editor/logic.go b/internal/editor/logic.go index 96c9533..38a64da 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -101,16 +101,20 @@ type Logic struct { session SessionState restoreFile string // restoreScroll/restoreScrollArmed hold the snapshot's scroll offset - // until it is safe to apply it: only once the scale is known (the first - // ScaleEvent — the size ConfigEvent arrives before it, and a layout in - // between computes the viewport in the wrong unit, which would clamp - // the offset, and the clamp is one-way). scaleSeen tracks that event. - restoreScroll ui.Dp - restoreScrollArmed bool - scaleSeen bool - sessionSaver func(SessionState) - lastSession SessionState - lastSessionSave time.Time + // until it is safe to apply it: only on an emitFrame whose layout pass + // saw a trustworthy viewport (scale known, size known, restored content + // present — the layout computes MaxScroll and one-way-clamps the offset, + // and on device the first ScaleEvent can precede the size ConfigEvent). + // 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 + scaleSeen bool + restoreContentLanded bool + sessionSaver func(SessionState) + lastSession SessionState + lastSessionSave time.Time } // NewLogic creates a new Logic instance, accepting an optional mockFS. @@ -254,14 +258,6 @@ func (l *Logic) Run() { l.scaleSeen = true } l.emitFrame() - // The restored scroll lands only AFTER a layout at the current - // scale: its clamp uses MaxScroll, which must reflect the same - // viewport units (the size ConfigEvent precedes the first - // ScaleEvent, so a layout between them is in the wrong unit). - if l.scaleSeen && l.restoreScrollArmed { - l.maybeApplyRestoreScroll() - l.emitFrame() - } case fb := <-l.layoutChan: // Store the full GlyphLayout on editor state. // Derive LastLineY from it for scroll clamping. @@ -364,6 +360,18 @@ func (l *Logic) emitFrame() { // the rate limit elapsed (tiny JSON file, see session.go). l.saveSessionIfChanged() elems := l.state.layout(l.browserManager) + // Relaunch restore (spec §7): land the armed restore scroll AFTER the + // layout pass above (it refreshed MaxScroll for the current viewport and + // one-way-clamps any offset set against an earlier, smaller one). The + // guard defers the application until the viewport is trustworthy — on + // device the first ScaleEvent can precede the size ConfigEvent. The + // re-layout makes this frame carry the restored viewport. The restore is + // complete once the scroll lands; drop restoreFile so a later open of + // this same file is treated as a fresh one. + if l.restoreScrollArmed && l.maybeApplyRestoreScroll() { + l.restoreFile = "" + elems = l.state.layout(l.browserManager) + } now := time.Now() if PerfRecord != nil { var delta time.Duration @@ -579,8 +587,12 @@ func (l *Logic) handleWorkerResult(res pool.Result) { // the snapshot to the replacement's buffer. if res.FilePath == l.state.Editor.Filename && l.restoreFile == res.FilePath { l.applyRestorePositions(len(content)) - l.maybeApplyRestoreScroll() - l.abortRestore() + l.restoreContentLanded = true + // The snapshot has landed: resume session saving. restoreFile + // and the armed scroll stay until the scroll itself lands in + // the emitFrame hook (which needs this content for a real + // MaxScroll), so a different open in between still aborts via + // openFile's restoreFile guard. // Re-scan the restored query only when the bar was open: // with the bar closed, the result would be dropped (the apply // gate requires Visible) and Scanning would stay stuck true; @@ -588,6 +600,12 @@ func (l *Logic) handleWorkerResult(res pool.Result) { if f := &l.state.Editor.Find; f.Visible && f.Query != "" && !f.Scanning { l.state.Editor.findDispatchScan() } + // No armed scroll: the snapshot has fully landed with the + // content; lift the save suppression now (the emitFrame + // hook does this when a scroll does land). + if !l.restoreScrollArmed { + l.session = SessionState{} + } } } } else if res.FilePath == l.restoreFile { diff --git a/internal/editor/session.go b/internal/editor/session.go index c49ed83..32f3991 100644 --- a/internal/editor/session.go +++ b/internal/editor/session.go @@ -136,8 +136,9 @@ func (l *Logic) BeginRestore(s SessionState) { // Fresh per-file find state; the snapshot's query and visibility land // with the stat (the file must exist first). e.findReset() - // The scroll offset is armed, not applied: it must not be clamped by a - // layout that runs before the scale is known (see the Logic struct). + // The scroll offset is armed, not applied: it lands in the emitFrame + // hook, once the layout pass has a trustworthy viewport (see the Logic + // struct). l.restoreScroll = ui.Dp(s.Scroll) l.restoreScrollArmed = s.Scroll > 0 l.state.page = EditorPage @@ -153,25 +154,46 @@ func (l *Logic) BeginRestore(s SessionState) { func (l *Logic) abortRestore() { l.restoreFile = "" l.restoreScrollArmed = false + l.restoreContentLanded = false l.session = SessionState{} } // maybeApplyRestoreScroll lands the armed restore scroll offset once the -// viewport unit is trustworthy (the first ScaleEvent has arrived), clamped -// to the current MaxScroll (the layout clamps again if the line index -// shrinks it later). Must be called on the logic goroutine. -func (l *Logic) maybeApplyRestoreScroll() { +// viewport is trustworthy, clamped to the current MaxScroll (the layout +// clamps again if the line index shrinks it later). It is called from +// emitFrame AFTER the layout pass, because that pass is where MaxScroll is +// computed for the current viewport — applying the offset before it (or in a +// pass with an unknown viewport) would hit the one-way clamp and lose the +// offset. On device the first ScaleEvent can precede the size ConfigEvent +// (and both can precede the restored content), so the offset stays armed +// until scale, size and editor content are all known; it is applied on the +// first such emitFrame. restoreContentLanded (not FileLen, which the stat +// result sets before the content arrives) marks that the buffer holds the +// restored file; clearing restoreFile any earlier would make the read +// result's path guard miss the just-loaded file. Returns true if it +// applied. Must be called on the logic goroutine. +func (l *Logic) maybeApplyRestoreScroll() bool { if !l.restoreScrollArmed { - return + return false } if !l.scaleSeen { - return + return false + } + if l.state.PixelWidth <= 0 || l.state.scale <= 0 { + return false + } + if l.state.Page() != EditorPage || !l.restoreContentLanded { + return false } if l.restoreScroll > l.state.MaxScroll { l.restoreScroll = l.state.MaxScroll } l.state.ScrollOffset = l.restoreScroll l.restoreScrollArmed = false + // The snapshot has fully landed (cursor/selection/find with the + // content, scroll now): lift the save suppression. + l.session = SessionState{} + return true } // applyRestorePositions clamps the restored snapshot's cursor and selection diff --git a/internal/test/e2e/restore_test.go b/internal/test/e2e/restore_test.go index b59d4d6..d43700a 100644 --- a/internal/test/e2e/restore_test.go +++ b/internal/test/e2e/restore_test.go @@ -119,6 +119,46 @@ func TestRestore_FileCursorScrollSelection(t *testing.T) { } } +// TestRestore_ScrollLandsWhenScalePrecedesSize reproduces the on-device +// startup ordering, where the first FrameEvent (and hence the ScaleEvent) +// arrives before the size ConfigEvent. The scroll must still land, clamped +// by the layout at the real scale: an earlier revision applied it at the +// ScaleEvent against a not-yet-sized viewport and lost it. +func TestRestore_ScrollLandsWhenScalePrecedesSize(t *testing.T) { + content := strings.Repeat("hello world line\n", 100) + dir := t.TempDir() + diskPath := filepath.Join(dir, "late.txt") + if err := os.WriteFile(diskPath, []byte(content), 0644); err != nil { + t.Fatal(err) + } + h := e2e.NewHarness(e2e.WithFileSystem(real.NewRealFileSystem(dir), "/")) + h.Logic().BeginRestore(editor.SessionState{File: "/late.txt", Cursor: 123, Scroll: 100}) + h.Run() + h.SendScale(2.0) // scale first + h.SendConfig(780, 1688) // size second (on-device ordering) + defer h.Cleanup() + + 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 false + } + full, err := cb.FullContent() + return err == nil && int64(len(full)) == cb.FileLen() && + cb.LineIndex.Size == cb.FileLen() && st.ScrollOffset == ui.Dp(100) + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + if v.(bool) { + return + } + time.Sleep(50 * time.Millisecond) + } + t.Fatal("timed out: scroll did not land when the scale event preceded the size") +} + // TestRestore_ClampsToShorterFile: the file was truncated since the last // session; the cursor and selection must clamp to the shorter content, not // restore past EOF.