Fix find scroll position on long files
Two bugs made the search jump land in the wrong place: 1. Double-counted visual lines. The visual line at which logical line li starts is exactly VisualsBefore(li); the code computed li + VisualsBefore(li), i.e. 2x the intended depth with the all-ones pre-shape estimates. The MaxScroll clamp masked it on small files; long files landed far off. Now uses VisualsBefore(li) (fallback li). 2. Truncation vs non-integer line height. Line height is 16.8dp, so floor(V*lh) sits just above the target line's top and the window decomposition floors to the line above it. The target now rounds UP: ceil(V*lh) is always in [V*lh, (V+1)*lh), so the viewport top decomposes to exactly V. The line-height source now also prefers the shaped GlyphLayout.LineHeight like scrollVisualDecompose/MaxScroll. 3. Estimate settle. On long wrapped files the lines above the target are still estimated at 1 visual line when the jump happens, so the landing can be short. The scroll now arms a bounded settle (SettleByte / SettleScroll / SettlePasses on FindState); after each layout-feedback wrap-count correction the logic goroutine re-runs the target and re-scrolls until it converges, is exhausted (4 passes), or the user / the MaxScroll clamp moves the viewport (which cancels it). Edits, query changes, close, and file open disarm the settle. Tests: round-trip scroll-target on a 2000-line wrapped file, the no-wrap identity, settle correction/convergence, and settle cancellation on user scroll.
This commit is contained in:
parent
5716d208b8
commit
587c1c5aad
181
internal/editor/find_scroll_test.go
Normal file
181
internal/editor/find_scroll_test.go
Normal file
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user