Fix cursor movement bug in scrolled editor and add regression test

This commit is contained in:
Greg Pomerantz 2026-06-04 13:53:42 -04:00
parent ca828569f7
commit 672dcbe6b3
2 changed files with 138 additions and 14 deletions

View File

@ -514,8 +514,11 @@ func SetCursorFromPoint(x, y float64) {
// 1. Identify the intended line index based on y // 1. Identify the intended line index based on y
// layout.Y values are relative to the text region origin. // layout.Y values are relative to the text region origin.
// We need to account for scroll offset. // We need to account for scroll offset: y is passed as relative to the text region top + scroll offset.
visualLine := int((y + float64(TheState.ScrollOffset)) / lineHeight) // So y is the position in the *content*.
visualLine := int(y / lineHeight)
log.Printf("SetCursorFromPoint: y=%f, scroll=%f, visualLine=%d, lineHeight=%f", y, float64(TheState.ScrollOffset), visualLine, lineHeight)
// Group glyphs by their Y-baseline // Group glyphs by their Y-baseline
type lineGroup struct { type lineGroup struct {
@ -523,27 +526,56 @@ func SetCursorFromPoint(x, y float64) {
indices []int indices []int
} }
groups := []lineGroup{} groups := []lineGroup{}
seenY := make(map[float64]int) // maps Y to group index
// Find all unique baseline Ys
// The tap Y is based on line height (top of line).
// We need to associate Y-baseline with visual line index.
// Create map from visual line (0, 1, 2...) to baseline Y.
// Since line height is fixed:
// Line 0 baseline is at some Y0.
// Line 1 baseline is at Y0 + lineHeight.
// Let's find Y0 first.
minY := 1e9
for _, yVal := range layout.Y {
if float64(yVal) < minY {
minY = float64(yVal)
}
}
// Now group by baseline
groups = []lineGroup{} // Reset groups
for i, yVal := range layout.Y { for i, yVal := range layout.Y {
yFloat := float64(yVal) yFloat := float64(yVal)
idx, ok := seenY[yFloat] lineIdx := int((yFloat - minY) / lineHeight + 0.5) // round to nearest line
if !ok { if lineIdx < 0 { lineIdx = 0 }
idx = len(groups)
groups = append(groups, lineGroup{y: yFloat, indices: []int{}}) // Ensure enough groups
seenY[yFloat] = idx for len(groups) <= lineIdx {
groups = append(groups, lineGroup{
y: minY + float64(len(groups))*lineHeight,
indices: []int{},
})
} }
groups[idx].indices = append(groups[idx].indices, i) groups[lineIdx].indices = append(groups[lineIdx].indices, i)
} }
// Sort groups by Y
sort.Slice(groups, func(i, j int) bool { return groups[i].y < groups[j].y })
// If visualLine is out of bounds, clamp // If visualLine is out of bounds, clamp
if visualLine < 0 { if visualLine < 0 {
visualLine = 0 visualLine = 0
} }
if visualLine >= len(groups) { if visualLine >= len(groups) || len(groups[visualLine].indices) == 0 {
visualLine = len(groups) - 1 // Clamp to last valid group that has indices
for i := len(groups) - 1; i >= 0; i-- {
if len(groups[i].indices) > 0 {
visualLine = i
break
}
}
}
if len(groups[visualLine].indices) == 0 {
return
} }
targetGroup := groups[visualLine] targetGroup := groups[visualLine]
@ -558,6 +590,7 @@ func SetCursorFromPoint(x, y float64) {
rightmostIdx = i rightmostIdx = i
} }
} }
log.Printf("TargetGroup: Y=%f, Indices=%v, rightmostIdx=%d, x=%f, rightmostX=%f", targetGroup.y, targetGroup.indices, rightmostIdx, x, rightmostX)
// 4. Check if tap is to the right of the last character // 4. Check if tap is to the right of the last character
if rightmostIdx != -1 && x > rightmostX { if rightmostIdx != -1 && x > rightmostX {
@ -585,12 +618,15 @@ func SetCursorFromPoint(x, y float64) {
if dist < 0 { if dist < 0 {
dist = -dist dist = -dist
} }
log.Printf("Checking glyph %d: x=%f, centerX=%f, dist=%f", i, x, glyphCenterX, dist)
if dist < minDist { if dist < minDist {
minDist = dist minDist = dist
bestIdx = i bestIdx = i
} }
} }
log.Printf("BestIdx=%d, ByteOffset=%d", bestIdx, layout.ByteOffsets[bestIdx])
if bestIdx != -1 { if bestIdx != -1 {
TheState.Editor.CursorPosition = layout.ByteOffsets[bestIdx] TheState.Editor.CursorPosition = layout.ByteOffsets[bestIdx]
log.Printf("After SetCursor: CursorPosition=%d", TheState.Editor.CursorPosition)
} }
} }

View File

@ -0,0 +1,88 @@
package e2e_test
import (
"testing"
"time"
"pad/internal/editor"
"pad/internal/test/e2e"
"pad/internal/ui"
)
func TestEditorClickToMoveCursorWithScroll(t *testing.T) {
h := e2e.NewHarnessWithDefaults()
h.Run() // Start the harness!
defer h.Cleanup()
// Switch to editor page
editor.GoToEditor(nil)
h.SendConfig(780, 1688)
// Give it a moment to initialize
time.Sleep(200 * time.Millisecond)
// Wait for frame
_, err := h.WaitForFrameCount(1, 5*time.Second)
if err != nil {
t.Fatalf("timeout waiting for frame: %v", err)
}
// Setup:
// - 2 lines, each 10 chars + newline.
// - Line height = 14 * 1.2 = 16.8 (rounded?) Let's check EditorLineHeight()
// EditorLineHeight is 14 * 1.2 = 16.8.
lineHeight := 16.8
editor.TheState.Editor.Buffer = "Line 1\nLine 2\nLine 3"
// Scroll to start of Line 2 (skip Line 1)
editor.TheState.ScrollOffset = ui.Dp(lineHeight)
// GlyphLayout:
// Line 1: y = 0
// Line 2: y = 16.8
// Line 3: y = 33.6
editor.TheState.Editor.GlyphLayout = ui.GlyphLayout{
ByteOffsets: []int{0, 7, 14}, // Start of each line
X: []ui.Dp{10, 10, 10},
Y: []ui.Dp{ui.Dp(0), ui.Dp(lineHeight), ui.Dp(lineHeight * 2)},
Advance: []ui.Dp{10, 10, 10},
}
// Tap at y=10 (within the text area).
// Because of scroll=16.8, this should correspond to:
// visualLine = (y + scroll) / lineHeight = (10 + 16.8) / 16.8 = 26.8 / 16.8 = 1.59 -> 1
// Line 1 is index 0. Line 2 is index 1.
// So visualLine 1 should be Line 2.
// The problem is that SetCursorFromPoint expects y in DP, but receives it as raw pixels
// if we're not careful. Let's pass the y value properly.
// The test harness sends raw pixel coordinates to handler.
// EditorLayout converts tap point:
// localY := float64(pt.Y - editorRegion.Y + TheState.ScrollOffset)
// So the handler receives Y as pt.Y, where pt.Y is relative to the screen.
// The test harness doesn't seem to account for region offset.
// Let's debug by printing in the test.
t.Logf("ScrollOffset: %v", editor.TheState.ScrollOffset)
h.SendInput([]ui.InputEvent{
{
Handler: func(data any) {
if pt, ok := data.(ui.Point); ok {
// Manually simulate the offset correction that EditorLayout does
// editorRegion.Y is margin(10) + statusBarH(52) = 62.
// Let's set pt.Y to 10 + 62 = 72.
localY := float64(pt.Y) - 62.0 + float64(editor.TheState.ScrollOffset)
editor.SetCursorFromPoint(float64(pt.X), localY)
}
},
Data: ui.Point{X: 10, Y: 72},
},
})
// Assert cursor moved to start of Line 2 (offset 7)
time.Sleep(100 * time.Millisecond) // Give logic goroutine a moment to process input
t.Logf("Cursor position: %d", editor.TheState.Editor.CursorPosition)
if editor.TheState.Editor.CursorPosition != 7 {
t.Errorf("expected cursor to move to 7 (Line 2), but got %d", editor.TheState.Editor.CursorPosition)
}
}