From 672dcbe6b3104ccf80497ee80ebf845de3eb7f5f Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Thu, 4 Jun 2026 13:53:42 -0400 Subject: [PATCH] Fix cursor movement bug in scrolled editor and add regression test --- internal/editor/state.go | 64 ++++++++++++++---- internal/test/e2e/scroll_cursor_test.go | 88 +++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 internal/test/e2e/scroll_cursor_test.go diff --git a/internal/editor/state.go b/internal/editor/state.go index 1a2b096..5781a5d 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -514,8 +514,11 @@ func SetCursorFromPoint(x, y float64) { // 1. Identify the intended line index based on y // layout.Y values are relative to the text region origin. - // We need to account for scroll offset. - visualLine := int((y + float64(TheState.ScrollOffset)) / lineHeight) + // We need to account for scroll offset: y is passed as relative to the text region top + scroll offset. + // 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 type lineGroup struct { @@ -523,27 +526,56 @@ func SetCursorFromPoint(x, y float64) { indices []int } 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 { yFloat := float64(yVal) - idx, ok := seenY[yFloat] - if !ok { - idx = len(groups) - groups = append(groups, lineGroup{y: yFloat, indices: []int{}}) - seenY[yFloat] = idx + lineIdx := int((yFloat - minY) / lineHeight + 0.5) // round to nearest line + if lineIdx < 0 { lineIdx = 0 } + + // Ensure enough groups + 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 < 0 { visualLine = 0 } - if visualLine >= len(groups) { - visualLine = len(groups) - 1 + if visualLine >= len(groups) || len(groups[visualLine].indices) == 0 { + // 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] @@ -558,6 +590,7 @@ func SetCursorFromPoint(x, y float64) { 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 if rightmostIdx != -1 && x > rightmostX { @@ -585,12 +618,15 @@ func SetCursorFromPoint(x, y float64) { if dist < 0 { dist = -dist } + log.Printf("Checking glyph %d: x=%f, centerX=%f, dist=%f", i, x, glyphCenterX, dist) if dist < minDist { minDist = dist bestIdx = i } } + log.Printf("BestIdx=%d, ByteOffset=%d", bestIdx, layout.ByteOffsets[bestIdx]) if bestIdx != -1 { TheState.Editor.CursorPosition = layout.ByteOffsets[bestIdx] + log.Printf("After SetCursor: CursorPosition=%d", TheState.Editor.CursorPosition) } } diff --git a/internal/test/e2e/scroll_cursor_test.go b/internal/test/e2e/scroll_cursor_test.go new file mode 100644 index 0000000..ae2add2 --- /dev/null +++ b/internal/test/e2e/scroll_cursor_test.go @@ -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) + } +}