From a5d1c4bee657da026a57a379d7e760655016eb9e Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Wed, 3 Jun 2026 19:01:05 -0400 Subject: [PATCH] Compute glyphLayout after text rendering. Implement correct cursor movement. --- cmd/pad/main.go | 16 +++++++------ doc/editor_implementation_plan.md | 4 ++-- internal/editor/logic.go | 23 +++++++++++------- internal/editor/state.go | 38 ++++++++++++++++++++++------- internal/ui/render.go | 40 ++++++++++++++++++++++++++----- internal/ui/unit.go | 12 ++++++++++ 6 files changed, 102 insertions(+), 31 deletions(-) diff --git a/cmd/pad/main.go b/cmd/pad/main.go index 4e189b5..ee1c276 100644 --- a/cmd/pad/main.go +++ b/cmd/pad/main.go @@ -37,6 +37,8 @@ func run(w *app.Window) error { renderer := ui.New(ui.Theme{FontSize: 14}, shaper, logic.State()) var mu sync.Mutex var elems []ui.Element + var curScale, newScale float32 + log.Printf("run: starting frameReceiver") go frameReceiver(w, &mu, &elems, logic.FrameChan()) log.Printf("run: starting logic.Run") @@ -55,16 +57,13 @@ func run(w *app.Window) error { case app.FrameEvent: log.Printf("run: FrameEvent") gtx := app.NewContext(&ops, e) - newScale := gtx.Metric.PxPerDp - curScale := logic.State().Scale() - if newScale != curScale { - logic.ConfigChan() <- editor.ScaleEvent{newScale} - } + newScale = gtx.Metric.PxPerDp + curScale = logic.State().Scale() // Deadlock risk: ensure no channel sends while lock is held mu.Lock() currentElems := elems renderer.Draw(gtx, currentElems) - lastLineY := int(renderer.LastLineY()) + glyphLayout := renderer.GlyphLayout() // Send search query update to the logic goroutine when it changes. // The logic goroutine handles filtering and triggers a new frame. newQuery := logic.State().Browser.SearchEditor.Text() @@ -112,13 +111,16 @@ func run(w *app.Window) error { } e.Frame(&ops) mu.Unlock() + if newScale != curScale { + logic.ConfigChan() <- editor.ScaleEvent{newScale} + } if len(events) > 0 { logic.InputChan() <- events } if sendQuery { logic.SearchQueryChan() <- newQuery } - logic.DisplayLineChan() <- lastLineY + logic.LayoutChan() <- glyphLayout } } } diff --git a/doc/editor_implementation_plan.md b/doc/editor_implementation_plan.md index c0db20e..a4202cb 100644 --- a/doc/editor_implementation_plan.md +++ b/doc/editor_implementation_plan.md @@ -154,7 +154,7 @@ func (r *Renderer) drawWrappedText(...) GlyphLayout { } ``` -`TextField.Draw` will store the returned layout on the renderer for the main loop to pick up. +`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`. ### 8.5 Feedback Path @@ -280,7 +280,7 @@ func HandleCursorMove(delta int) { |---|---|---| | `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; expose via `r.GlyphLayout()` | -| `internal/ui/element.go` | `TextField.Draw` calls `drawWrappedText` but ignores any layout return value | Store returned `GlyphLayout` on renderer | +| `internal/ui/element.go` | `TextField.Draw` calls `drawWrappedText` | **No change** — layout capture and storage happens entirely inside `drawWrappedText` in `render.go` | | `internal/editor/logic.go` | Uses `lastLineYChan` for feedback | Add `layoutChan`; handle it in `Run()` loop; remove `lastLineYChan` | | `internal/editor/state.go` | `EditorLayout` uses fixed-width approximation (`EditorFontSize * 0.6`) and `byteOffsetToLineCol`; `HandleCursorMove` is byte-based (±1) | `EditorState.GlyphLayout` field; `EditorLayout` uses it for cursor positioning; `HandleCursorMove` uses it for navigation | | `cmd/pad/main.go` | Sends `int(renderer.LastLineY())` on `DisplayLineChan()` after each frame | Send `r.GlyphLayout()` on `layoutChan` after draw; remove `lastLineYChan` usage | diff --git a/internal/editor/logic.go b/internal/editor/logic.go index 8195413..61a26b2 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -49,7 +49,7 @@ type Logic struct { configChan chan ConfigUpdate frameChan chan []ui.Element inputChan chan []ui.InputEvent - lastLineYChan chan int // last line Y (Dp, sent as int) feedback from renderer + layoutChan chan ui.GlyphLayout // per-glyph layout feedback from renderer (replaces lastLineYChan) resultChan chan ResultEvent searchQueryChan chan string // search text updates from main goroutine openFileChan chan string // Added for asynchronous file loading @@ -82,7 +82,7 @@ func NewLogic() *Logic { configChan: make(chan ConfigUpdate), frameChan: make(chan []ui.Element), inputChan: make(chan []ui.InputEvent), - lastLineYChan: make(chan int), + layoutChan: make(chan ui.GlyphLayout), resultChan: make(chan ResultEvent), searchQueryChan: make(chan string), openFileChan: make(chan string), // Initialized @@ -110,9 +110,9 @@ func (l *Logic) InputChan() chan<- []ui.InputEvent { return l.inputChan } -// DisplayLineChan returns the last line Y feedback channel. -func (l *Logic) DisplayLineChan() chan<- int { - return l.lastLineYChan +// LayoutChan returns the glyph layout feedback channel. +func (l *Logic) LayoutChan() chan<- ui.GlyphLayout { + return l.layoutChan } // ResultChan returns the result channel for the logic goroutine. @@ -148,9 +148,16 @@ func (l *Logic) Run() { log.Printf("Logic: ConfigEvent") update.apply(l.state) l.frameChan <- l.state.layout(l.browserManager) - case y := <-l.lastLineYChan: - if ui.Dp(y) != l.state.LastLineY { - l.state.LastLineY = ui.Dp(y) + case layout := <-l.layoutChan: + // Store the full GlyphLayout on editor state. + // Derive LastLineY from it for scroll clamping. + 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) } case events := <-l.inputChan: diff --git a/internal/editor/state.go b/internal/editor/state.go index 573842d..019b40c 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -2,6 +2,7 @@ package editor import ( "log" + "sort" "pad/internal/browser" "pad/internal/ui" @@ -45,6 +46,7 @@ const ( type EditorState struct { Buffer string // Document content CursorPosition int // Byte offset + GlyphLayout ui.GlyphLayout // Per-glyph layout from renderer SelectionStart int // -1 if no selection SelectionEnd int // -1 if no selection Dirty bool // Needs save @@ -342,15 +344,35 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { // for Gio to deliver key events to this element. editorElem.Focused = TheState.FocusedElementID == "editor_text" - // Convert byte offset (CursorPosition) to line/column for cursor rendering. - line, col := byteOffsetToLineCol(TheState.Editor.Buffer, TheState.Editor.CursorPosition) - - // Compute cursor X/Y in screen coordinates. - // We approximate character width as EditorFontSize * 0.6 for monospace. - charWidth := ui.Dp(float32(EditorFontSize) * 0.6) + // Position cursor from GlyphLayout. + layout := TheState.Editor.GlyphLayout + pos := TheState.Editor.CursorPosition lineHeight := EditorLineHeight() - cursorX := editorRegion.X + ui.Dp(col)*charWidth - cursorY := editorRegion.Y + ui.Dp(line)*lineHeight - TheState.ScrollOffset + + var cursorX, cursorY ui.Dp + if len(layout.ByteOffsets) == 0 { + // Empty buffer — place cursor at the top-left of the editor region. + cursorX = editorRegion.X + cursorY = editorRegion.Y + } else { + // Binary search for the glyph at or after the cursor byte offset. + idx := sort.Search(len(layout.ByteOffsets), func(i int) bool { + return layout.ByteOffsets[i] >= pos + }) + if idx < len(layout.ByteOffsets) { + // Cursor is at a glyph position. + cursorX = editorRegion.X + layout.X[idx] + cursorY = editorRegion.Y - TheState.ScrollOffset + layout.Y[idx] - EditorFontSize + } else { + // Cursor is 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 - TheState.ScrollOffset + layout.Y[len(layout.Y)-1] - EditorFontSize + } + } + + // Derive line/col for bottom bar display (still using byte-based + // approximation until GlyphLayout-based line tracking is added). + line, col := byteOffsetToLineCol(TheState.Editor.Buffer, TheState.Editor.CursorPosition) // Create the cursor element at the computed position cursorElem := ui.NewCursor( diff --git a/internal/ui/render.go b/internal/ui/render.go index 72e13a9..a762df3 100644 --- a/internal/ui/render.go +++ b/internal/ui/render.go @@ -8,6 +8,7 @@ import ( _ "image/png" "log" "time" + "unicode/utf8" "gioui.org/f32" "gioui.org/gesture" @@ -60,8 +61,9 @@ type Renderer struct { clicks map[string]clickReg Keys map[string]keyReg // Exported Keys map scrolls map[string]scrollReg - displayLineCount int // number of display lines from last drawWrappedText - lastLineY Dp // last line baseline offset from text origin, in Dp + displayLineCount int // number of display lines from last drawWrappedText + lastLineY Dp // last line baseline offset from text origin, in Dp (derived from GlyphLayout) + glyphLayout GlyphLayout // captured per-glyph layout from last drawWrappedText } // New creates a new Renderer. @@ -229,6 +231,13 @@ func (r *Renderer) LastLineY() Dp { return r.lastLineY } +// GlyphLayout returns the glyph layout data captured during the last +// drawWrappedText call. Used by the logic goroutine to position the cursor +// and navigate by glyph instead of byte offset. +func (r *Renderer) GlyphLayout() GlyphLayout { + return r.glyphLayout +} + // clippableElement is implemented by elements that need their own clip region // around all their content and interaction registrations. type clippableElement interface { @@ -472,12 +481,28 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w var glyphs [32]text.Glyph line := glyphs[:0] lineCount := 0 - var lastGlyphY float32 // shaper Y value of last glyph drawn + + // Capture per-glyph layout data for cursor positioning and navigation. + var layout GlyphLayout + byteOffset := 0 for g, ok := r.shp.NextGlyph(); ok; g, ok = r.shp.NextGlyph() { + // Record layout data for this glyph. + // g.X is in fixed.Int26_6 — shift >> 6 for device pixels, divide by scale for Dp. + // g.Y is the baseline in device pixels. + layout.ByteOffsets = append(layout.ByteOffsets, byteOffset) + 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())) + + // Advance byteOffset by g.Runes. + for i := uint16(0); i < g.Runes; i++ { + _, sz := utf8.DecodeRuneInString(str[byteOffset:]) + byteOffset += sz + } + line = append(line, g) if g.Flags&text.FlagLineBreak != 0 || cap(line)-len(line) == 0 { r.drawLine(gtx, line, reg.X, y, col) - lastGlyphY = float32(line[len(line)-1].Y) line = line[:0] if g.Flags&text.FlagLineBreak != 0 { lineCount++ @@ -486,7 +511,6 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w } if len(line) > 0 { r.drawLine(gtx, line, reg.X, y, col) - lastGlyphY = float32(line[len(line)-1].Y) lineCount++ } call := m.Stop() @@ -494,7 +518,11 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w textClip.Pop() r.displayLineCount = lineCount - r.lastLineY = Dp(lastGlyphY / r.scale.Scale()) // pixels → Dp + // Store captured layout; derive lastLineY from it. + r.glyphLayout = layout + if len(layout.Y) > 0 { + r.lastLineY = layout.Y[len(layout.Y)-1] + } } func (r *Renderer) drawPng(gtx layout.Context, img image.Image, reg Region, width, height Dp) { diff --git a/internal/ui/unit.go b/internal/ui/unit.go index 96b2dc0..617f256 100644 --- a/internal/ui/unit.go +++ b/internal/ui/unit.go @@ -63,3 +63,15 @@ func FromDp(r Region, scale float32) RegionPx { H: ToPx(r.H, scale), } } + +// GlyphLayout holds per-glyph layout data captured during text shaping. +// Each index i represents one glyph (one rune in the source string). +// ByteOffsets[i] is the byte position in the buffer, (X[i], Y[i]) is the +// glyph's screen location in Dp (X relative to text region origin, Y is the +// shaper baseline), and Advance[i] is the glyph's width in Dp. +type GlyphLayout struct { + ByteOffsets []int // byte offset of each glyph in the buffer + X []Dp // screen X (Dp) of each glyph, relative to text region origin + Y []Dp // screen Y (Dp) baseline of each glyph (shaper value) + Advance []Dp // advance width (Dp) of each glyph +}