diff --git a/internal/editor/find_scroll_test.go b/internal/editor/find_scroll_test.go new file mode 100644 index 0000000..2bac6f7 --- /dev/null +++ b/internal/editor/find_scroll_test.go @@ -0,0 +1,181 @@ +package editor + +import ( + "math" + "math/rand" + "testing" + + "pad/internal/ui" +) + +// TestFindScrollTarget_PlacesMatchAtViewportTop is the regression test for +// "in a long file the viewport is not positioned properly when I search." +// +// The visual line at which logical line li starts is exactly V(li) = +// VisualsBefore(li) — the visual lines of all lines BEFORE it. The old code +// computed li + VisualsBefore(li), double-counting li: with the all-ones +// pre-shape estimates that is 2*li, so the viewport landed at twice the +// intended depth. On small files the MaxScroll clamp masked it (everything +// still fit / clamped to the bottom); on long files it was wildly wrong. +func TestFindScrollTarget_PlacesMatchAtViewportTop(t *testing.T) { + const n = 2000 // a "long" file + rng := rand.New(rand.NewSource(3)) + counts := make([]int32, n) + for i := range counts { + counts[i] = int32(1 + rng.Intn(4)) // each line wraps into 1..4 visual lines + } + cb := newTestBufferForWrap(n, counts) + + TheState = NewState() + TheState.Editor.ChunkedBuffer = cb + // Large MaxScroll so the clamp never interferes with the mapping itself. + totalVisuals := float64(cb.WrapIndex.TotalVisuals()) + lh := float64(EffectiveLineHeight()) + TheState.MaxScroll = ui.Dp(totalVisuals * lh) + + w := cb.WrapIndex + // Every line i is "x\n" (2 bytes) except the last ("x"), so line i starts + // at byte i*2. + for _, li := range []int{0, 1, 137, 1500, n - 2, n - 1} { + absByte := li * 2 + target, ok := findScrollTarget(absByte) + if !ok { + t.Fatalf("line %d: findScrollTarget ok=false", li) + } + want := ui.Dp(math.Ceil(float64(w.VisualsBefore(li)) * lh)) + if target != want { + t.Fatalf("line %d: target = %v, want ceil(V(%d)*lh) = %v (the visual line at which the line starts)", + li, target, li, want) + } + // The inverse mapping must put the viewport top back on logical line li. + TheState.ScrollOffset = target + k, _ := scrollVisualDecompose() + if k != li { + t.Fatalf("line %d: decompose(target) = %d, want %d (round-trip failed)", li, k, li) + } + // Guard against the old double-count: the buggy value was + // (li + V(li))*lh, which for li > 0 is never the correct target. + if li > 0 { + if buggy := ui.Dp(math.Ceil(float64(int64(li)+int64(w.VisualsBefore(li))) * lh)); target == buggy { + t.Fatalf("line %d: target %v equals the old double-count value", li, target) + } + } + } +} + +// TestFindScrollTarget_NoWrapIsIdentity pins the all-ones (pre-shape / +// word-wrap-off) case: the target is exactly li*lineHeight, matching the +// legacy mapping and the nil-WrapIndex fallback. +func TestFindScrollTarget_NoWrapIsIdentity(t *testing.T) { + const n = 500 + counts := make([]int32, n) + for i := range counts { + counts[i] = 1 + } + cb := newTestBufferForWrap(n, counts) + TheState = NewState() + TheState.Editor.ChunkedBuffer = cb + lh := float64(EffectiveLineHeight()) + TheState.MaxScroll = ui.Dp(float64(n) * lh) + + for _, li := range []int{0, 1, 42, n - 1} { + target, ok := findScrollTarget(li * 2) + if !ok { + t.Fatalf("line %d: ok=false", li) + } + if want := ui.Dp(math.Ceil(float64(li) * lh)); target != want { + t.Fatalf("line %d: target = %v, want ceil(li*lh) = %v", li, target, want) + } + } +} + +// TestFindSettle_CorrectsAfterWrapCounts verifies the post-jump correction: +// the search scroll arms a settle with the estimate-based offset; when the +// (simulated) shaping pass corrects the wrap counts, findSettle re-scrolls +// to the accurate position and then converges. +func TestFindSettle_CorrectsAfterWrapCounts(t *testing.T) { + const n = 1000 + counts := make([]int32, n) + for i := range counts { + counts[i] = 1 // pre-shape estimates + } + cb := newTestBufferForWrap(n, counts) + TheState = NewState() + TheState.Editor.ChunkedBuffer = cb + lh := float64(EffectiveLineHeight()) + TheState.MaxScroll = ui.Dp(float64(4*n) * lh) + + const li = 500 + absByte := li * 2 + e := &TheState.Editor + e.Find = FindState{Visible: true} + target, ok := findScrollTarget(absByte) + if !ok { + t.Fatal("findScrollTarget ok=false") + } + TheState.ScrollOffset = target + e.Find.SettleByte = absByte + e.Find.SettleScroll = target + e.Find.SettlePasses = 4 + + // Simulate the shaping pass: every line above the target wraps into 3 + // visual lines. The estimate-based offset is now far too small. + for i := 0; i < li; i++ { + cb.WrapIndex.Set(i, 3) + } + + if !e.findSettle() { + t.Fatal("findSettle did not re-scroll after the wrap-count correction") + } + want := ui.Dp(math.Ceil(float64(cb.WrapIndex.VisualsBefore(li)) * lh)) + if TheState.ScrollOffset != want { + t.Fatalf("settled offset %v, want %v", TheState.ScrollOffset, want) + } + if k, _ := scrollVisualDecompose(); k != li { + t.Fatalf("decompose(settled) = %d, want %d", k, li) + } + if e.Find.SettlePasses != 3 { + t.Fatalf("passes %d, want 3", e.Find.SettlePasses) + } + + // No further correction: the settle converges and disarms. + if e.findSettle() { + t.Fatal("findSettle did not converge on a stable offset") + } + if e.Find.SettleByte != -1 { + t.Fatalf("SettleByte %d, want -1 after convergence", e.Find.SettleByte) + } +} + +// TestFindSettle_CancelsOnUserScroll: if anything other than the settle +// moves the viewport, the settle gives up (no yanking the view back). +func TestFindSettle_CancelsOnUserScroll(t *testing.T) { + const n = 200 + counts := make([]int32, n) + for i := range counts { + counts[i] = 1 + } + cb := newTestBufferForWrap(n, counts) + TheState = NewState() + TheState.Editor.ChunkedBuffer = cb + lh := float64(EffectiveLineHeight()) + TheState.MaxScroll = ui.Dp(float64(n) * lh) + + const li = 100 + e := &TheState.Editor + e.Find = FindState{Visible: true} + target, _ := findScrollTarget(li * 2) + TheState.ScrollOffset = target + e.Find.SettleByte = li * 2 + e.Find.SettleScroll = target + e.Find.SettlePasses = 4 + + // The user scrolls away. + TheState.ScrollOffset = target + ui.Dp(5*lh) + if e.findSettle() { + t.Fatal("findSettle re-scrolled after a user scroll") + } + if e.Find.SettleByte != -1 { + t.Fatal("settle not cancelled by a user scroll") + } +} diff --git a/internal/editor/logic.go b/internal/editor/logic.go index 3317aec..ad9b3e3 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -238,6 +238,12 @@ func (l *Logic) Run() { // lines since shaping (fb.EditSeq correlates with the content). if fb.EditSeq == l.state.Editor.EditSeq { l.state.applyWrapCounts(fb) + // 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. + if l.state.Editor.findSettle() { + l.emitFrame() + } } if derivedLastLineY != l.state.LastLineY { l.state.LastLineY = derivedLastLineY diff --git a/internal/editor/search.go b/internal/editor/search.go index 1b08a82..8cbab15 100644 --- a/internal/editor/search.go +++ b/internal/editor/search.go @@ -22,6 +22,7 @@ package editor // edits" — re-typing the query re-scans. import ( + "math" "sort" "pad/internal/io/pool" @@ -36,6 +37,15 @@ type FindState struct { Cur int // index into Matches; -1 when none is selected Gen uint64 // generation of the newest dispatched scan Scanning bool // a scan for Gen is in flight + // Settle* drive the post-jump viewport correction (findSettle). On long + // wrapped files the estimate-based search scroll can land short: lines + // above the target are counted as one visual line until the renderer + // shapes them. SettleByte is the target byte (-1 = inactive), + // SettleScroll the last offset the settle itself set, SettlePasses the + // remaining re-scrolls. + SettleByte int + SettleScroll ui.Dp + SettlePasses int } // ToggleFind opens the find bar (or closes it when open). It is the tap @@ -80,6 +90,7 @@ func (e *EditorState) findClose() { e.Find.Matches = nil e.Find.Cur = -1 e.Find.Scanning = false + e.Find.SettleByte = -1 if TheState.FocusedElementID == "find_bar" { TheState.FocusedElementID = "editor_text" } @@ -93,7 +104,7 @@ func (e *EditorState) findReset() { // Monotonic generation bump (do not zero the counter): a stale // in-flight scan carrying a HIGH generation must never equal a fresh // one, or its result would pass the gen gate in applySearchResult. - e.Find = FindState{Query: e.Find.Query, Gen: e.Find.Gen + 1} + e.Find = FindState{Query: e.Find.Query, Gen: e.Find.Gen + 1, SettleByte: -1} } // findSetQuery processes a query forwarded from the main-owned "find_bar" @@ -104,6 +115,7 @@ func (e *EditorState) findSetQuery(q string) { f.Query = q f.Matches = nil f.Cur = -1 + f.SettleByte = -1 // a new query invalidates any in-flight settle if q == "" || e.TooLarge { f.Scanning = false return @@ -230,27 +242,97 @@ func findStep(dir int) { scrollToFindMatch(m[0]) } -// scrollToFindMatch scrolls so the visual line containing absByte sits at -// the top of the editor viewport (clamped to [0, MaxScroll]). -func scrollToFindMatch(absByte int) { +// findScrollTarget returns the scroll offset that puts the visual line +// containing absByte at the top of the editor viewport (clamped to +// [0, MaxScroll]). ok=false when there is no line index to map through. +func findScrollTarget(absByte int) (ui.Dp, bool) { cb := TheState.Editor.ChunkedBuffer if cb == nil || cb.LineIndex == nil { - return + return 0, false } li := cb.LineIndex.FindLogicalLineForByteOffset(absByte) + // The visual line at which logical line li STARTS is exactly V(li) = + // VisualsBefore(li) (the visual lines of all lines before it). Do not + // add li on top: with the all-ones pre-shape estimates that would put + // the viewport at 2x the intended depth (masked by the MaxScroll clamp + // on small files, wildly wrong on long ones). vl := int64(li) if w := cb.WrapIndex; w != nil { - vl += int64(w.VisualsBefore(li)) + vl = int64(w.VisualsBefore(li)) } - lh := float64(EffectiveLineHeight()) - target := ui.Dp(float64(vl) * lh) + // Same line-height preference as scrollVisualDecompose/MaxScroll, so the + // target sits in the same visual-line space the window decomposition + // uses. + lh := EffectiveLineHeight() + if g := TheState.Editor.GlyphLayout.LineHeight; g > 0 { + lh = g + } + // The scroll offset is integer Dp but the line height is not (14 * 1.2 = + // 16.8): rounding DOWN would put the offset just above the target line's + // top, so scrollDecompose floors to the line ABOVE it. Round UP: for any + // integer s in [V*lh, (V+1)*lh) the decomposed top line is exactly V, and + // ceil(V*lh) is always in that interval (lh > 1). + target := ui.Dp(math.Ceil(float64(vl) * float64(lh))) if target < 0 { target = 0 } if target > TheState.MaxScroll { target = TheState.MaxScroll } + return target, true +} + +// scrollToFindMatch scrolls so the visual line containing absByte sits at +// the top of the editor viewport (clamped to [0, MaxScroll]), and arms the +// settle (findSettle) that corrects the estimate-based landing once the +// jumped-to window has been shaped. +func scrollToFindMatch(absByte int) { + e := &TheState.Editor + target, ok := findScrollTarget(absByte) + if !ok { + e.Find.SettleByte = -1 + return + } TheState.ScrollOffset = target + if e.Find.Visible { + e.Find.SettleByte = absByte + e.Find.SettleScroll = target + e.Find.SettlePasses = 4 // a few frames of re-correction, then stop + } +} + +// findSettle re-runs the search scroll after applyWrapCounts: shaping the +// jumped-to window corrected the visual-line counts around it, so the +// target may now be more accurate than the estimate-based landing. Bounded +// by SettlePasses and cancelled if anything else (user scroll, the +// MaxScroll clamp) moved the viewport in the meantime. Must be called on +// the logic goroutine; returns true when it re-scrolled (caller emits a +// frame). +func (e *EditorState) findSettle() bool { + f := &e.Find + if !f.Visible || f.SettleByte < 0 { + return false + } + if TheState.ScrollOffset != f.SettleScroll { + f.SettleByte = -1 + return false + } + target, ok := findScrollTarget(f.SettleByte) + if !ok { + f.SettleByte = -1 + return false + } + if target == TheState.ScrollOffset { + f.SettleByte = -1 // converged + return false + } + TheState.ScrollOffset = target + f.SettleScroll = target + f.SettlePasses-- + if f.SettlePasses < 0 { + f.SettleByte = -1 + } + return true } // findEdit reports a length-changing edit to the find state: the byte range @@ -261,6 +343,7 @@ func scrollToFindMatch(absByte int) { // re-scan, which happens when the query is re-typed). func (e *EditorState) findEdit(start, end, newLen int) { f := &e.Find + f.SettleByte = -1 // the edit rebuilds the wrap index: the settled offset is stale if len(f.Matches) == 0 { return }