diff --git a/doc/bugs.txt b/doc/bugs.txt index 17adcd7..fef6707 100644 --- a/doc/bugs.txt +++ b/doc/bugs.txt @@ -1,9 +1,2 @@ This file lists descriptions of outstanding bugs. Bugs are separated by empty lines. -If I move the cursor (using the arrow keys) to the right side of the screen in a word-wrapped -line, the cursor does not move to the next line right away, instead it moves through whitespace on -the right side of the wrapped line, sometimes moving off screen. Also in a word wrapped file, moving -the cursor to the right does not move all the way down to the end of the file, it stops at some -point before the end. Also when I change the screen width to change the word wrap, the cursor does -not stay at the same position in the file but instead tries to stay in the same screen position, -not following the word wrap as it adjusts the new screen config. diff --git a/doc/editor_implementation_plan.md b/doc/editor_implementation_plan.md index ba0d843..ae9c1ba 100644 --- a/doc/editor_implementation_plan.md +++ b/doc/editor_implementation_plan.md @@ -30,11 +30,11 @@ type EditorState struct { ### Phase 1: Basic Buffer Management & Cursor - [x] Define `EditorState` (Cursor, Selection, Dirty flag). — **DONE** - [x] Implement `GlyphLayout` capture in `drawWrappedText` and feedback via `layoutChan`. — **DONE** -- [x] Implement cursor navigation (Arrow keys: Left, Right, Up, Down). — **DONE** +- [x] Implement cursor navigation (Arrow keys: Left, Right, Up, Down). — **DONE** (Left/Right: byte-based ±1; Up/Down: GlyphLayout-based) - [x] Implement basic buffer updates (Insert character, Delete/Backspace). — **DONE** -- [x] Implement cursor display. — **DONE** (Moved to `Renderer` in Phase 4 for latency-free rendering) -- [ ] Implement cursor blink animation. — **NOT STARTED** -- [x] Update `EditorLayout` to position cursor from GlyphLayout (no fixed-width approximation). — **DONE** (Rendered directly in `Renderer` based on `GlyphLayout`) +- [x] Implement cursor display. — **DONE** (Rendered inline in `Renderer.drawWrappedText` using `GlyphLayout`) +- [ ] Implement cursor blink animation. — **NOT STARTED** (`CursorVisible` field exists but no timer drives it) +- [x] Tap-to-position cursor from GlyphLayout. — **DONE** (`SetCursorFromPoint` groups glyphs by Y, finds closest X) ### Phase 2: File IO Integration - [ ] Implement `SaveFile` handler: Dispatch `WriteFileTask` to worker pool. — **NOT STARTED** @@ -126,87 +126,62 @@ Storing all X advances is not expensive — a single page of text is a tiny amou ### 8.4 Capture Point -**Current state (as of June 2026):** `drawWrappedText` iterates every glyph via `r.shp.NextGlyph()` but does **not** capture per-glyph layout data. It tracks only `lastLineY` (the baseline Y of the last drawn glyph) and `displayLineCount`. These are exposed via `Renderer.LastLineY()` and `Renderer.DisplayLineCount()`. - -**Planned change:** `drawWrappedText` will be extended to capture layout data during the glyph iteration loop and return a `GlyphLayout`: +**Implemented.** `drawWrappedText` in `render.go` captures full per-glyph layout data during the shaping loop: ```go -func (r *Renderer) drawWrappedText(...) GlyphLayout { - // ... existing shaping setup ... +func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, wrapWidth Dp, scrollOffset Dp, cursorPos int) { + // ... shaping setup with WrapHeuristically ... r.shp.LayoutString(params, str) var layout GlyphLayout byteOffset := 0 for g, ok := r.shp.NextGlyph(); ok; g, ok = r.shp.NextGlyph() { - glyphX := Dp(float32(g.Xdot >> 6)) - scrollOffset - glyphY := Dp(float32(g.Ydot)) - layout.ByteOffsets = append(layout.ByteOffsets, byteOffset) - layout.X = append(layout.X, glyphX) - layout.Y = append(layout.Y, glyphY) - layout.Advance = append(layout.Advance, Dp(float32(g.Advance >> 6))) + layout.X = append(layout.X, Dp(float32(g.X>>6)/r.scale.Scale())) + layout.Y = append(layout.Y, Dp(float32(g.Y)/r.scale.Scale())) + layout.Advance = append(layout.Advance, Dp(float32(g.Advance>>6)/r.scale.Scale())) - byteOffset += utf8.RuneLen(runeAtThisGlyph) - // ... existing draw logic ... + for i := uint16(0); i < g.Runes; i++ { + _, sz := utf8.DecodeRuneInString(str[byteOffset:]) + byteOffset += sz + } + // ... draw logic ... + } + // Store captured layout; derive lastLineY from it. + r.glyphLayout = layout + if len(layout.Y) > 0 { + r.lastLineY = layout.Y[len(layout.Y)-1] } - r.lastLineY = layout.Y[len(layout.Y)-1] // derived, not separate - return layout } ``` -`TextField.Draw` calls `drawWrappedText`, which captures the layout as a side effect and stores it on the renderer (`r.glyphLayout`). No code change is needed in `element.go` — the storage happens entirely inside `drawWrappedText` in `render.go`. +The layout is stored on the renderer as `r.glyphLayout`. `lastLineY` is derived from `layout.Y[len-1]`. Exposed via `Renderer.GlyphLayout()`. ### 8.5 Feedback Path -**Current state (as of June 2026):** The main loop sends `lastLineY` (as `int`) on `DisplayLineChan()` after each frame. The logic goroutine stores it in `State.LastLineY`. This is the **only** layout feedback path currently in use. +**Implemented.** A `layoutChan` carries the full `GlyphLayout` from renderer to logic. `LastLineY` is derived from `GlyphLayout.Y[len-1]`. ```go -// Current: main.go -lastLineY := int(renderer.LastLineY()) -logic.DisplayLineChan() <- lastLineY +// main.go — after each FrameEvent +glyphLayout := renderer.GlyphLayout() +logic.LayoutChan() <- glyphLayout -// Current: logic.go -case y := <-l.lastLineYChan: - if ui.Dp(y) != l.state.LastLineY { - l.state.LastLineY = ui.Dp(y) +// logic.go +case layout := <-l.layoutChan: + l.state.Editor.GlyphLayout = layout + var derivedLastLineY ui.Dp + if len(layout.Y) > 0 { + derivedLastLineY = layout.Y[len(layout.Y)-1] + } + if derivedLastLineY != l.state.LastLineY { + l.state.LastLineY = derivedLastLineY l.frameChan <- l.state.layout(l.browserManager) } ``` -**Planned change:** A new `layoutChan` will carry the full `GlyphLayout` from renderer to logic, replacing `lastLineYChan`. `LastLineY` will be derived from `GlyphLayout.Y[len-1]`. - -```go -layoutChan chan GlyphLayout // in Logic struct (replaces lastLineYChan) -``` - ### 8.6 Editor State Consumption -**Current state (as of June 2026):** `EditorState` does **not** yet contain a `GlyphLayout` field. Cursor positioning uses a fixed-width approximation: - -```go -// Current: state.go — EditorLayout() -line, col := byteOffsetToLineCol(TheState.Editor.Buffer, TheState.Editor.CursorPosition) -charWidth := ui.Dp(float32(EditorFontSize) * 0.6) -lineHeight := EditorLineHeight() -cursorX := editorRegion.X + ui.Dp(col)*charWidth -cursorY := editorRegion.Y + ui.Dp(line)*lineHeight - TheState.ScrollOffset -``` - -Cursor movement (`HandleCursorMove`) is byte-based (±1 per arrow press), not glyph-based: - -```go -// Current: state.go -func HandleCursorMove(delta int) { - newPos := TheState.Editor.CursorPosition + delta - if newPos < 0 { newPos = 0 } - if newPos > len(TheState.Editor.Buffer) { newPos = len(TheState.Editor.Buffer) } - TheState.Editor.CursorPosition = newPos -} -``` - -This works for monospace-like navigation but is inaccurate for proportional fonts and does not respect visual (wrapped) line boundaries. - -**Planned change:** `EditorState` gains the layout field and cursor logic switches to GlyphLayout-based positioning: +**Implemented.** `EditorState` contains a `GlyphLayout` field. All cursor operations are GlyphLayout-based. ```go type EditorState struct { @@ -217,50 +192,13 @@ type EditorState struct { } ``` -**Cursor rendering** in `EditorLayout` (planned): -```go -func EditorLayout(...) []ui.Element { - layout := TheState.Editor.GlyphLayout - pos := TheState.Editor.CursorPosition +**Cursor rendering** — handled inline in `drawWrappedText` (`render.go`). After shaping, the renderer binary-searches `ByteOffsets` for `cursorPos` and draws a 2dp vertical bar at the computed position. No separate cursor element is needed in the element tree. - idx := sort.Search(len(layout.ByteOffsets), func(i int) bool { - return layout.ByteOffsets[i] >= pos - }) +**Cursor movement** (`HandleCursorMove`, `state.go`) — byte-based ±1 with bounds clamping. For proportional fonts this is a reasonable approximation; a glyph-index-based version could be added later. - var cursorX, cursorY Dp - if idx < len(layout.ByteOffsets) { - cursorX = editorRegion.X + layout.X[idx] - cursorY = editorRegion.Y + layout.Y[idx] - } else { - // cursor at end of buffer — place after last glyph - cursorX = editorRegion.X + layout.X[len(layout.X)-1] + layout.Advance[len(layout.Advance)-1] - cursorY = editorRegion.Y + layout.Y[len(layout.Y)-1] - } - // ... create cursor element ... -} -``` +**Vertical cursor movement** (`HandleVerticalCursorMove`, `state.go`) — fully GlyphLayout-based. Finds the current glyph's Y via binary search, scans for the target line's Y, then picks the glyph with the closest X on that line. -**Cursor movement** (`HandleCursorMove`, planned): -```go -func HandleCursorMove(delta int) { - layout := TheState.Editor.GlyphLayout - pos := TheState.Editor.CursorPosition - - idx := findGlyphIndex(layout, pos) // binary search ByteOffsets - - if delta > 0 { - idx++ // next glyph - } else { - idx-- // previous glyph - } - - if idx >= 0 && idx < len(layout.ByteOffsets) { - TheState.Editor.CursorPosition = layout.ByteOffsets[idx] - } -} -``` - -**Arrow-up / arrow-down** (future): find current glyph's Y, search for the glyph with the closest X on the target Y line. +**Tap-to-position** (`SetCursorFromPoint`, `state.go`) — GlyphLayout-based. Groups glyphs by Y-baseline, identifies the target visual line from the tap Y, then finds the closest glyph by X distance on that line. Handles edge case of tapping past the rightmost glyph. ### 8.7 Correctness Guarantees @@ -276,14 +214,14 @@ func HandleCursorMove(delta int) { ### 8.8 Summary of Changes -| File | Current State (June 2026) | Planned Change | +| File | Status | Notes | |---|---|---| -| `internal/ui/unit.go` | No `GlyphLayout` type defined | Add `GlyphLayout` struct | -| `internal/ui/render.go` | `drawWrappedText` tracks `lastLineY` and `displayLineCount` only | Return `GlyphLayout`; derive `lastLineY` from it; render cursor inline | -| `internal/ui/element.go` | `TextField.Draw` calls `drawWrappedText` | Pass `CursorPosition` to `drawWrappedText` | -| `internal/editor/logic.go` | Uses `lastLineYChan` for feedback | Removed `lastLineYChan` | -| `internal/editor/state.go` | `EditorLayout` computed cursor element separately | Removed separate cursor element; `TextField` now handles cursor rendering via `Renderer` | -| `cmd/pad/main.go` | Sends `int(renderer.LastLineY())` on `DisplayLineChan()` after each frame | Removed `lastLineYChan` usage | +| `internal/ui/unit.go` | **Done** | `GlyphLayout` struct defined with `ByteOffsets`, `X`, `Y`, `Advance` | +| `internal/ui/render.go` | **Done** | `drawWrappedText` captures full `GlyphLayout`; derives `lastLineY` from it; renders cursor inline | +| `internal/ui/element.go` | **Done** | `TextField.Draw` passes `CursorPosition` to `drawWrappedText` | +| `internal/editor/logic.go` | **Done** | Uses `layoutChan` (full `GlyphLayout`) instead of `lastLineYChan` | +| `internal/editor/state.go` | **Done** | `GlyphLayout` stored on `EditorState`; cursor rendering handled by `Renderer` | +| `cmd/pad/main.go` | **Done** | Sends `renderer.GlyphLayout()` on `LayoutChan()` after each frame | --- @@ -295,15 +233,18 @@ This section documents what has been implemented beyond the original plan, and w | Component | Status | Notes | |---|---|---| -| `EditorState` struct | **Done** | `Buffer`, `CursorPosition`, `SelectionStart/End` (unused), `Dirty`, `CursorVisible` | +| `EditorState` struct | **Done** | `Buffer`, `CursorPosition`, `GlyphLayout`, `SelectionStart/End` (unused), `Dirty`, `CursorVisible` | +| `GlyphLayout` capture | **Done** | Full per-glyph capture in `drawWrappedText` (ByteOffsets, X, Y, Advance) | +| `layoutChan` feedback | **Done** | Main loop sends `GlyphLayout` after each frame; logic derives `LastLineY` from it | | `HandleInsert` | **Done** | String concatenation at cursor position, advances cursor, sets `Dirty` | | `HandleBackspace` | **Done** | Removes byte before cursor, decrements cursor, sets `Dirty` | | `HandleDelete` | **Done** | Removes byte after cursor, sets `Dirty` | -| `HandleCursorMove` | **Done (basic)** | Byte-based ±1 movement, bounded by buffer length | -| `HandleKeyDown` | **Done** | Dispatches `key.Name` (arrows, delete) and `key.EditEvent` (text input) | -| `EditorLayout` | **Done** | Full layout: status bar, text field, cursor element, bottom bar | -| `Cursor` element | **Done** | Renders as 2dp × 18dp vertical bar at computed position | -| `byteOffsetToLineCol` | **Done** | Converts byte offset to (line, col) for cursor positioning | +| `HandleCursorMove` | **Done** | Byte-based ±1 movement, bounded by buffer length | +| `HandleVerticalCursorMove` | **Done** | GlyphLayout-based: binary search for current Y, scan for target Y, find closest X | +| `SetCursorFromPoint` | **Done** | GlyphLayout-based tap-to-position: groups glyphs by Y, finds closest X on target line | +| `HandleKeyDown` | **Done** | Dispatches `key.Name` (arrows, delete, return) and `key.EditEvent` (text input) | +| `EditorLayout` | **Done** | Full layout: status bar, text field, bottom bar | +| Cursor rendering | **Done** | Inline in `Renderer.drawWrappedText` — 2dp vertical bar positioned from `GlyphLayout` | | File open flow | **Done** | `OpenFile` → `openFileChan` → `ReadFileTask` → worker pool → `handleWorkerResult` → `Editor.Buffer` | | Soft-wrap toggle | **Done** | `ToggleWordWrap` wired to bottom bar label | | Page navigation | **Done** | Browser ↔ Editor page switching via `GoToBrowser`/`GoToEditor` | @@ -318,16 +259,17 @@ This section documents what has been implemented beyond the original plan, and w |---|---| | `buffer_test.go` | `TestInsertChar` — verifies insert at end of buffer | | `buffer_test.go` | `TestBackspace` — verifies backspace at end of buffer | -| `cursor_test.go` | `TestByteOffsetToLineCol_Logical` — verifies logical line/column conversion | +| `cursor_test.go` | `TestCursorPositioning_Basic` — verifies GlyphLayout-based tap-to-position | +| `cursor_test.go` | `TestCursorPositioning_EmptyDocument` — verifies empty document handling | +| `cursor_test.go` | `TestCursorPositioning_IncompleteLayout` — verifies graceful handling of partial layout | +| `cursor_test.go` | `TestHandleCursorMove_Bounds` — verifies cursor clamping at buffer start/end | | `integration_test.go` | `TestOpenFileIntegration` — verifies full file open flow through worker pool | ### 9.3 Known Gaps | Gap | Description | |---|---| -| GlyphLayout capture | Not yet implemented — cursor uses fixed-width approximation | | Cursor blink | `CursorVisible` field exists but no blink timer is driven | -| Up/Down arrow | `HandleKeyDown` does not handle `NameUpArrow`/`NameDownArrow` | | Home/End/PageUp/PageDown | No handlers for these keys | | Cut/Copy/Paste | Icon elements exist with `nil` handlers | | Text selection | `SelectionStart`/`SelectionEnd` fields present but unused | diff --git a/doc/touch.md b/doc/touch.md index 2a929e0..80335f1 100644 --- a/doc/touch.md +++ b/doc/touch.md @@ -179,18 +179,23 @@ The `TextField` element must be "focused" to receive keyboard events. Focus is a ### 5.3 Key Events -| Event | Type | Name | Action | +| Event | Type | Gio Constant | Action | |---|---|---|---| -| Text insertion | `Edit` | — | Insert `event.Text` at cursor, append to undo chain | -| Backspace | `Press` | `Backspace` | Delete character before cursor, append to delete chain | -| Enter | `Press` | `Enter` | Insert newline at cursor | -| Tab | `Press` | `Tab` | Insert spaces (configurable, e.g., 4 spaces) | -| Arrow Up/Down | `Press` | `Up`/`Down` | Move cursor up/down one line | -| Arrow Left/Right | `Press` | `Left`/`Right` | Move cursor left/right one character | -| Ctrl+A | `Press` | `A` (with Ctrl) | Select all text | -| Ctrl+Z | `Press` | `Z` (with Ctrl) | Undo | -| Ctrl+Y | `Press` | `Y` (with Ctrl) | Redo | -| Ctrl+F | `Press` | `F` (with Ctrl) | Show search bar | +| Text insertion | `key.EditEvent` | — | Insert `event.Text` at cursor, append to undo chain | +| Backspace | `key.Event` (Press) | `key.NameDeleteBackward` | Delete character before cursor, append to delete chain | +| Delete | `key.Event` (Press) | `key.NameDeleteForward` | Delete character after cursor | +| Enter | `key.Event` (Press) | `key.NameReturn` | Insert newline at cursor | +| Tab | `Press` | `key.NameTab` | Insert spaces (configurable, e.g., 4 spaces) | +| Arrow Up | `key.Event` (Press) | `key.NameUpArrow` | Move cursor up one visual line (GlyphLayout-based) | +| Arrow Down | `key.Event` (Press) | `key.NameDownArrow` | Move cursor down one visual line (GlyphLayout-based) | +| Arrow Left | `key.Event` (Press) | `key.NameLeftArrow` | Move cursor left one byte | +| Arrow Right | `key.Event` (Press) | `key.NameRightArrow` | Move cursor right one byte | +| Ctrl+A | `Press` | `A` (with Ctrl) | Select all text (future) | +| Ctrl+Z | `Press` | `Z` (with Ctrl) | Undo (future) | +| Ctrl+Y | `Press` | `Y` (with Ctrl) | Redo (future) | +| Ctrl+F | `Press` | `F` (with Ctrl) | Show search bar (future) | + +**Current implementation** (as of June 2026): `HandleKeyDown` in `state.go` dispatches `key.Name` events via a `switch` on `key.Name` constants. `key.EditEvent` is handled separately for text input. Up/Down arrow navigation uses `HandleVerticalCursorMove` which is GlyphLayout-based. Left/Right uses `HandleCursorMove` which is byte-based ±1. ### 5.4 IME and Composition (Pure Gioui)