From 58725a5e6c81a4f971498843b3dd06d17836a5e3 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Sun, 16 Aug 2026 01:32:27 -0400 Subject: [PATCH] Make state single-owner and stabilize race detector Frame now carries view state (scale/focus/query) to the main goroutine, which reads only the frame-receiver-stored snapshot. Renderer owns Gio widget editors (registered by ID) and the draw scale. Autosave timer sends a token to the logic goroutine instead of touching state; Shutdown waits for the owner to exit before FlushAll. Tests access state only through owner-side Inspect/WithState helpers (harness + in-package). Fixed double-Harness.Run race in two e2e tests, made TestWorkerPool_PriorityPreemption deterministic, and raised the lazy-loading test timeout that was too short under -race. go test -race ./... is now green. --- cmd/pad/main.go | 48 +++--- internal/browser/layout.go | 44 +----- internal/browser/lazy_loading_test.go | 6 +- internal/browser/types.go | 9 +- internal/editor/e2e_test.go | 111 ++++++++----- internal/editor/frame.go | 67 ++++++++ internal/editor/integration_test.go | 17 +- internal/editor/logic.go | 155 +++++++++++-------- internal/io/pool/worker_pool_test.go | 57 +++++-- internal/test/e2e/browser_files_test.go | 25 ++- internal/test/e2e/cursor_interaction_test.go | 38 +++-- internal/test/e2e/edit_lifecycle_test.go | 45 ++++-- internal/test/e2e/harness.go | 74 ++++++++- internal/test/e2e/harness_test.go | 6 +- internal/test/e2e/scroll_cursor_test.go | 68 +++++--- internal/test/e2e/search_test.go | 106 ++++++++----- internal/test/e2e/sort_mode_test.go | 86 ++++++---- internal/test/e2e/type_at_start_test.go | 48 ++++-- internal/ui/element.go | 22 +-- internal/ui/render.go | 60 ++++--- 20 files changed, 723 insertions(+), 369 deletions(-) create mode 100644 internal/editor/frame.go diff --git a/cmd/pad/main.go b/cmd/pad/main.go index 93cc4eb..7417649 100644 --- a/cmd/pad/main.go +++ b/cmd/pad/main.go @@ -13,6 +13,7 @@ import ( "gioui.org/unit" "gioui.org/font/gofont" "gioui.org/io/key" + "gioui.org/widget" "pad/internal/editor" "pad/internal/io/pool/real" @@ -53,13 +54,22 @@ func run(w *app.Window) error { log.Printf("using filesystem at / (startup directory: %s)", startAbs) logic := editor.NewLogic(fs, startAbs, OpenFile) - renderer := ui.New(ui.Theme{FontSize: 14}, shaper, logic.State()) + renderer := ui.New(ui.Theme{FontSize: 14}, shaper) + + // The search bar's widget.Editor is owned by the MAIN goroutine: Gio + // mutates it during draw, and the logic goroutine must never touch it + // (architecture.md §1). Its text is forwarded to the logic goroutine via + // SearchQueryChan; the logic only stores the result in Browser.Query. + var searchEditor widget.Editor + renderer.RegisterGioEditor("search_bar", &searchEditor) + + // frame is the frame-receiver-stored handoff (architecture.md §2.2/§9): + // the ONLY data the main goroutine reads from the logic side. var mu sync.Mutex - var elems []ui.Element - var curScale, newScale float32 + var frame editor.Frame log.Printf("run: starting frameReceiver") - go frameReceiver(w, &mu, &elems, logic.FrameChan()) + go frameReceiver(w, &mu, &frame, logic.FrameChan()) log.Printf("run: starting logic.Run") go logic.Run() @@ -76,24 +86,24 @@ func run(w *app.Window) error { } case app.FrameEvent: gtx := app.NewContext(&ops, e) - newScale = gtx.Metric.PxPerDp - curScale = logic.State().Scale() - // Deadlock risk: ensure no channel sends while lock is held + newScale := gtx.Metric.PxPerDp + // Read ONLY the frame-receiver-stored snapshot; the main goroutine + // never touches logic State (architecture.md §1). mu.Lock() - currentElems := elems - renderer.Draw(gtx, currentElems) + curScale := frame.Scale + if curScale <= 0 { + curScale = 1 // no frame yet + } + renderer.Draw(gtx, frame.Elems, curScale) 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() - sendQuery := false - if newQuery != logic.State().Browser.Query { - sendQuery = true - } + newQuery := searchEditor.Text() + sendQuery := newQuery != frame.Query events := renderer.CheckGestures(e.Source, gtx.Metric) // Gather key events - focusedID := logic.State().FocusedElementID + focusedID := frame.FocusedElementID if focusedID != "" { if reg, ok := renderer.Keys[focusedID]; ok { // Use key.Filter to only receive events destined for the focused element. @@ -134,7 +144,7 @@ func run(w *app.Window) error { e.Frame(&ops) mu.Unlock() if newScale != curScale { - logic.ConfigChan() <- editor.ScaleEvent{newScale} + logic.ConfigChan() <- editor.ScaleEvent{Scale: newScale} } if len(events) > 0 { logic.InputChan() <- events @@ -150,13 +160,13 @@ func run(w *app.Window) error { } } -func frameReceiver(w *app.Window, mu *sync.Mutex, elems *[]ui.Element, frameChan <-chan []ui.Element) { +func frameReceiver(w *app.Window, mu *sync.Mutex, frame *editor.Frame, frameChan <-chan editor.Frame) { log.Printf("frameReceiver: loop starting") for { - frame := <-frameChan + f := <-frameChan //log.Printf("frameReceiver: received frame") mu.Lock() - *elems = frame + *frame = f w.Invalidate() mu.Unlock() } diff --git a/internal/browser/layout.go b/internal/browser/layout.go index baafc91..1cf0bc0 100644 --- a/internal/browser/layout.go +++ b/internal/browser/layout.go @@ -37,43 +37,10 @@ func BrowserLayout(screenW, screenH ui.Dp, state *BrowserState, sortHandler func X: margin, Y: searchY, W: contentWidth, H: searchHeight, } - state.SearchEditor.SingleLine = true - searchBar := ui.NewGioEditor("search_bar", searchRegion, &state.SearchEditor) - - var searchPlaceholder ui.Element - if state.SearchEditor.Len() == 0 { - // Place holder should not be interactive or clickable. - // Its region must be inside the search bar, but does it overlap? - // "assertions.go:321: elements 1 (ui.GioEditor) and 2 (ui.Label) overlap" - // The GioEditor (element 1) is the search bar. - // The Label (element 2) is the "Search..." text. - // If they overlap, they should ideally be the same element, or the label should be drawn differently. - // Since GioEditor draws its own background and content, maybe the Label is redundant - // or they just need to be explicitly placed so they don't trigger overlap checks? - // Actually, if the editor *is* the input field, the label is just a placeholder. - // If the editor doesn't support placeholders natively, the label must be placed - // *inside* the search bar region. - // The overlap check might be too strict if elements are allowed to overlap - // (e.g. text over a background). - // Wait, the error says: - // GioEditor: region=Region{x=10 y=39 w=760 h=36} - // Label: region=Region{x=18 y=47 w=744 h=20} - // They definitely overlap. - // Let's make them NOT overlap if possible, or is this check incorrect? - // Actually, in many UI systems, text elements *are* allowed to overlap containers. - // Maybe the test harness's overlap check is too simplistic? - // Let's assume the overlap check is intended to catch errors. - // If I make the Label invisible when the Editor is focused, or just not add it? - // The code adds it only if Len() == 0. - // Can I make the Label smaller? Or not added? - - // To fix the test, let's remove the label and rely on GioEditor to handle the placeholder if possible? - // Or if we must keep it, let's change the region so it doesn't overlap? - // But it's supposed to be inside the search bar. - // Let's try to make the label NOT an element for now, just to pass the test, - // and see if the browser still works. - searchPlaceholder = nil - } + // The search widget itself is owned by the main goroutine and registered + // with the renderer by ID ("search_bar"); the logic goroutine only sees + // its text via the searchQuery channel (state.Query). + searchBar := ui.NewGioEditor("search_bar", searchRegion) // --- ListView --- listY := searchY + searchHeight + margin/2 @@ -115,9 +82,6 @@ func BrowserLayout(screenW, screenH ui.Dp, state *BrowserState, sortHandler func ) elems := []ui.Element{headerBar, searchBar} - if searchPlaceholder != nil { - elems = append(elems, searchPlaceholder) - } elems = append(elems, listView) return elems } diff --git a/internal/browser/lazy_loading_test.go b/internal/browser/lazy_loading_test.go index 4b325b4..b39c2f4 100644 --- a/internal/browser/lazy_loading_test.go +++ b/internal/browser/lazy_loading_test.go @@ -46,14 +46,14 @@ func TestLazyLoadingLargeDirectory(t *testing.T) { } // Wait for index build result and process it - res, ok := getResult(2 * time.Second) + res, ok := getResult(15 * time.Second) if !ok { t.Fatal("Timeout waiting for BuildIndex result") } bm.HandleResult(res) // Wait for initial page load result and process it - res, ok = getResult(2 * time.Second) + res, ok = getResult(15 * time.Second) if !ok { t.Fatal("Timeout waiting for initial LoadPages result") } @@ -69,7 +69,7 @@ func TestLazyLoadingLargeDirectory(t *testing.T) { bm.OnScroll() // Wait for load pages result for scroll - res, ok = getResult(2 * time.Second) + res, ok = getResult(15 * time.Second) if !ok { t.Fatal("Timeout waiting for scroll LoadPages result") } diff --git a/internal/browser/types.go b/internal/browser/types.go index cdb71c0..86032f8 100644 --- a/internal/browser/types.go +++ b/internal/browser/types.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - "gioui.org/widget" "pad/internal/ui" ) @@ -48,9 +47,13 @@ type BrowserState struct { SortMode SortMode // Current sort mode // Search - Query string // Current search query + Query string // Current search query (forwarded from the main-owned search widget) SearchResults []int // Indices of matching entries (empty = no filter) - SearchEditor widget.Editor // Gio editor for search input + // NOTE: the search bar's widget.Editor is intentionally NOT part of this + // state: Gio mutates widget state on the main goroutine during draw, and + // this state is owned by the logic goroutine (architecture.md §1). The + // widget lives in the renderer; its text is forwarded to the logic + // goroutine via the searchQuery channel, landing in Query. // Alphabetical index ActiveLetter string // Currently pressed letter (for highlighting) diff --git a/internal/editor/e2e_test.go b/internal/editor/e2e_test.go index c341c7b..230aa98 100644 --- a/internal/editor/e2e_test.go +++ b/internal/editor/e2e_test.go @@ -9,6 +9,20 @@ import ( "pad/internal/io/pool/mock" ) +// withState runs fn on the logic goroutine (the sole state owner) and fails +// the test on timeout. Test-only helper (architecture.md §1): tests must +// never touch l.state directly while Run() is active. +func withState(t *testing.T, l *Logic, fn func(*State)) { + t.Helper() + _, ok := l.Inspect(func(st *State) any { + fn(st) + return nil + }) + if !ok { + t.Fatalf("withState: inspect timed out") + } +} + func TestAutoSaveE2E(t *testing.T) { // 1. Setup mockFS := mock.NewFileSystem() @@ -18,7 +32,7 @@ func TestAutoSaveE2E(t *testing.T) { l := NewLogic(mockFS, "/", func(string) {}) go l.Run() - defer l.Done() + defer l.Shutdown() // Drain frameChan to prevent deadlocks go func() { @@ -26,16 +40,21 @@ func TestAutoSaveE2E(t *testing.T) { } }() - l.state.Editor.Filename = filename - TheState = l.state + withState(t, l, func(st *State) { + TheState = st + }) - // 2. Open File + // 2. Open File (OpenFile also sets Editor.Filename on the owner) OpenFile(filename) - + // Wait for the file to be loaded by checking if ChunkedBuffer is populated success := false for i := 0; i < 20; i++ { - if l.state.Editor.ChunkedBuffer != nil && l.state.Editor.ChunkedBuffer.FileLen() > 0 { + v, ok := l.Inspect(func(st *State) any { + cb := st.Editor.ChunkedBuffer + return cb != nil && cb.FileLen() > 0 + }) + if ok && v.(bool) { success = true break } @@ -45,12 +64,16 @@ func TestAutoSaveE2E(t *testing.T) { t.Fatal("Timed out waiting for file to load") } - // 3. Edit - l.state.Editor.CursorPosition = len(initialContent) - HandleInsert(" World") + // 3. Edit (owner-side) + withState(t, l, func(st *State) { + st.Editor.CursorPosition = len(initialContent) + HandleInsert(" World") + }) - // 4. Trigger auto-save - l.markDirty() + // 4. Trigger auto-save (owner-side) + withState(t, l, func(st *State) { + l.markDirty() + }) // 5. Wait for the write to complete success = false @@ -95,7 +118,7 @@ func TestLargeFileChunkBoundary(t *testing.T) { l := NewLogic(mockFS, "/", func(string) {}) go l.Run() - defer l.Done() + defer l.Shutdown() // Drain frameChan to prevent deadlocks go func() { @@ -103,15 +126,20 @@ func TestLargeFileChunkBoundary(t *testing.T) { } }() - l.state.Editor.Filename = filename - TheState = l.state + withState(t, l, func(st *State) { + TheState = st + }) // --- 2. Open File --- OpenFile(filename) success := false for i := 0; i < 20; i++ { - if l.state.Editor.ChunkedBuffer != nil && l.state.Editor.ChunkedBuffer.FileLen() > 0 { + v, ok := l.Inspect(func(st *State) any { + cb := st.Editor.ChunkedBuffer + return cb != nil && cb.FileLen() > 0 + }) + if ok && v.(bool) { success = true break } @@ -121,25 +149,28 @@ func TestLargeFileChunkBoundary(t *testing.T) { t.Fatal("Timed out waiting for large file to load") } - cb := l.state.Editor.ChunkedBuffer - if cb == nil { - t.Fatal("ChunkedBuffer is nil after opening large file") - } - // --- 3. Edit near the end of chunk 1 (position 65000, inside chunk 1) --- - l.state.Editor.CursorPosition = 65000 - HandleInsert("CHUNK1") + withState(t, l, func(st *State) { + st.Editor.CursorPosition = 65000 + HandleInsert("CHUNK1") + }) // --- 4. Edit right at the chunk 0 / chunk 1 boundary (position 65535) --- - l.state.Editor.CursorPosition = 65535 - HandleInsert("BOUNDARY") + withState(t, l, func(st *State) { + st.Editor.CursorPosition = 65535 + HandleInsert("BOUNDARY") + }) // --- 5. Edit near the end of chunk 3 (position 262000) --- - l.state.Editor.CursorPosition = 262000 - HandleInsert("CHUNK3") + withState(t, l, func(st *State) { + st.Editor.CursorPosition = 262000 + HandleInsert("CHUNK3") + }) - // --- 6. Trigger save --- - l.FlushAll() + // --- 6. Trigger save (owner-side) --- + withState(t, l, func(st *State) { + l.FlushAll() + }) // --- 7. Read back from mockFS and verify byte-for-byte --- savedContent, err := mockFS.ReadFile(filename) @@ -234,7 +265,7 @@ func TestFlushOnExitE2E(t *testing.T) { l := NewLogic(mockFS, "/", func(string) {}) go l.Run() - defer l.Done() + defer l.Shutdown() // Drain frameChan to prevent deadlocks go func() { @@ -242,16 +273,22 @@ func TestFlushOnExitE2E(t *testing.T) { } }() - l.state.Editor.Filename = filename - l.state.Editor.Buffer = initialContent - TheState = l.state + withState(t, l, func(st *State) { + st.Editor.Filename = filename + st.Editor.Buffer = initialContent + TheState = st + }) - // 2. Edit - l.state.Editor.CursorPosition = len(initialContent) - HandleInsert(" World") + // 2. Edit (owner-side) + withState(t, l, func(st *State) { + st.Editor.CursorPosition = len(initialContent) + HandleInsert(" World") + }) - // 3. Trigger Flush - l.FlushAll() + // 3. Trigger Flush (owner-side) + withState(t, l, func(st *State) { + l.FlushAll() + }) // 4. Assert content, _ := mockFS.ReadFile(filename) diff --git a/internal/editor/frame.go b/internal/editor/frame.go new file mode 100644 index 0000000..feba2c4 --- /dev/null +++ b/internal/editor/frame.go @@ -0,0 +1,67 @@ +package editor + +import ( + "time" + + "pad/internal/ui" +) + +// Frame is the unit of handoff from the logic goroutine to the frame +// receiver. Per architecture.md §9, the frame is the ONLY cross-goroutine +// state carrier: the main goroutine must never read *State directly, it +// reads the snapshot stored here by the frame receiver (under the frame +// mutex). +// +// Elems is the computed element tree for the next draw. +// The remaining fields are the view-state snapshot that the main goroutine +// needs to route events and forward input: +// +// - Scale: current px-per-Dp, so the main goroutine can detect scale +// changes and pass the scale to the renderer's Draw call. +// - FocusedElementID: which registered element receives key/edit events. +// - Query: the search query the logic goroutine is currently filtering +// with; the main goroutine compares it against the (main-owned) search +// widget's text and forwards changes via SearchQueryChan. +type Frame struct { + Elems []ui.Element + Scale float32 + FocusedElementID string + Query string +} + +// frameOf wraps a computed element tree with the current view-state +// snapshot. Must be called on the logic goroutine. +func (l *Logic) frameOf(elems []ui.Element) Frame { + return Frame{ + Elems: elems, + Scale: l.state.scale, + FocusedElementID: l.state.FocusedElementID, + Query: l.state.Browser.Query, + } +} + +// inspectReq is a test-only request to run fn on the logic goroutine. +// It preserves the single-owner invariant (architecture.md §1): the fn +// executes on the owner, not on the caller. fn must not block on sends to +// logic channels. +type inspectReq struct { + fn func(st *State) any + resp chan any +} + +// Inspect runs fn on the logic goroutine and returns its result. It is +// intended for tests; production code must use the regular channels. +func (l *Logic) Inspect(fn func(st *State) any) (any, bool) { + req := &inspectReq{fn: fn, resp: make(chan any, 1)} + select { + case l.inspectChan <- req: + case <-time.After(5 * time.Second): + return nil, false + } + select { + case v := <-req.resp: + return v, true + case <-time.After(5 * time.Second): + return nil, false + } +} diff --git a/internal/editor/integration_test.go b/internal/editor/integration_test.go index da1b420..3b58e2c 100644 --- a/internal/editor/integration_test.go +++ b/internal/editor/integration_test.go @@ -16,25 +16,29 @@ func TestOpenFileIntegration(t *testing.T) { l := NewLogic(mockFS, "/", func(string) {}) go l.Run() - defer l.Done() + defer l.Shutdown() // Drain frameChan to prevent deadlocks go func() { for range l.FrameChan() { } }() - - TheState = l.state // Initialize global state + + // Initialize global state on the owner + withState(t, l, func(st *State) { + TheState = st + }) // 2. Open File // OpenFile dispatches the task to openFileChan, which Run() will consume OpenFile(filename) - + // 3. Process the Result // We wait for the state to update, which happens when ReadChunkTask completes success := false for i := 0; i < 20; i++ { - if l.state.Editor.GetBuffer() == content { + v, ok := l.Inspect(func(st *State) any { return st.Editor.GetBuffer() }) + if ok && v.(string) == content { success = true break } @@ -43,6 +47,7 @@ func TestOpenFileIntegration(t *testing.T) { // 4. Assert if !success { - t.Errorf("Expected content %q, got %q", content, l.state.Editor.Buffer) + got, _ := l.Inspect(func(st *State) any { return st.Editor.Buffer }) + t.Errorf("Expected content %q, got %q", content, got) } } diff --git a/internal/editor/logic.go b/internal/editor/logic.go index 1e19b83..bcf669c 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -45,23 +45,29 @@ type ResultEvent struct { } // Logic runs the logic goroutine and provides channels for communication. +// +// Single-owner invariant (architecture.md §1): the logic goroutine is the +// sole reader/writer of l.state. Every other goroutine talks to it through +// the channels below. The only exception is Inspect, a test-only request +// channel whose fn still executes on the owner. type Logic struct { state *State browserManager *browser.BrowserManager configChan chan ConfigUpdate - frameChan chan []ui.Element + frameChan chan Frame // frames carry the view-state snapshot inputChan chan []ui.InputEvent layoutChan chan ui.GlyphLayout resultChan chan ResultEvent searchQueryChan chan string openFileChan chan string - retryChan chan string // Added for auto-save retries + retryChan chan string // auto-save retries + autosaveChan chan struct{} // auto-save debounce ticks (timer -> owner) + inspectChan chan *inspectReq workerPool *pool.WorkerPool mockFS pool.FileSystem - mu sync.Mutex done chan struct{} - saveTimer *time.Timer // Added for auto-save debounce - saveGeneration int // Added for stale timer filtering + exitWg sync.WaitGroup + saveTimer *time.Timer // auto-save debounce timer; non-nil while pending } // NewLogic creates a new Logic instance, accepting an optional mockFS. @@ -89,13 +95,15 @@ func NewLogic(mfs pool.FileSystem, path string, openfunc func(string)) *Logic { state: state, browserManager: bm, configChan: make(chan ConfigUpdate), - frameChan: make(chan []ui.Element, 1), + frameChan: make(chan Frame, 1), inputChan: make(chan []ui.InputEvent), layoutChan: make(chan ui.GlyphLayout), resultChan: make(chan ResultEvent), searchQueryChan: make(chan string), openFileChan: make(chan string), retryChan: make(chan string, 1), // Buffered channel + autosaveChan: make(chan struct{}), + inspectChan: make(chan *inspectReq), workerPool: wp, mockFS: mockFS, done: make(chan struct{}), @@ -110,7 +118,7 @@ func (l *Logic) ConfigChan() chan<- ConfigUpdate { } // FrameChan returns the frame channel for the logic goroutine. -func (l *Logic) FrameChan() <-chan []ui.Element { +func (l *Logic) FrameChan() <-chan Frame { return l.frameChan } @@ -140,13 +148,11 @@ func (l *Logic) SearchQueryChan() chan<- string { var TheState *State var TheLogic *Logic -// Scale returns the current scale factor (pixels per DP). -func (l *Logic) Scale() float32 { - return l.state.Scale() -} - // Run runs the logic goroutine loop. func (l *Logic) Run() { + l.exitWg.Add(1) + defer l.exitWg.Done() + // Dispatch initial directory index build on startup log.Printf("Logic: Dispatching BuildIndexTask") l.workerPool.Dispatch(pool.NewBuildIndexTask(l.state.Browser.CurrentPath, l.mockFS)) @@ -158,26 +164,26 @@ func (l *Logic) Run() { case update := <-l.configChan: log.Printf("Logic: ConfigEvent") update.apply(l.state) - l.frameChan <- l.state.layout(l.browserManager) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) case layout := <-l.layoutChan: // Store the full GlyphLayout on editor state. // Derive LastLineY from it for scroll clamping. l.state.Editor.GlyphLayout = layout - log.Printf("LOGIC received GlyphLayout: ByteOffsets=%d, VisualLineStarts=%d", len(layout.ByteOffsets), len(layout.VisualLineStarts)) + log.Printf("LOGIC received GlyphLayout: ByteOffsets=%d, VisualLineStarts=%d", len(layout.ByteOffsets), len(layout.VisualLineStarts)) 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) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) } case events := <-l.inputChan: log.Printf("Logic: InputEvents") for _, evt := range events { evt.Handler(evt.Data) } - l.frameChan <- l.state.layout(l.browserManager) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) case query := <-l.searchQueryChan: log.Printf("Logic: SearchQuery") if query != l.state.Browser.Query { @@ -186,7 +192,7 @@ func (l *Logic) Run() { browser.HandleSearch(&l.state.Browser, query) } } - l.frameChan <- l.state.layout(l.browserManager) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) case path := <-l.openFileChan: log.Printf("Logic: OpenFileChan %s", path) // Create chunked buffer for virtual scrolling @@ -201,30 +207,61 @@ func (l *Logic) Run() { log.Printf("Logic: Retrying save for %s", filename) if filename == l.state.Editor.Filename { // Reconstruct full content from chunked buffer for saving - var content []byte - if l.state.Editor.ChunkedBuffer != nil { - fullContent, err := l.state.Editor.ChunkedBuffer.FullContent() - if err != nil { - log.Printf("Error reconstructing full content for saving %s: %v", filename, err) - return - } - content = []byte(fullContent) - } else { - content = []byte(l.state.Editor.Buffer) + content, ok := l.fullContentBytes() + if !ok { + break } l.workerPool.DispatchNonBlocking( pool.NewWriteFileTask(filename, content, l.mockFS), ) } + case <-l.autosaveChan: + // Auto-save debounce tick. The timer goroutine only sent a token; + // the owner reconstructs content and dispatches the write. + l.saveTimer = nil + if l.state.Editor.Filename == "" { + break + } + content, ok := l.fullContentBytes() + if !ok { + break + } + log.Printf("Logic: Dispatching WriteFileTask for %s, content len=%d", l.state.Editor.Filename, len(content)) + l.workerPool.DispatchNonBlocking( + pool.NewWriteFileTask(l.state.Editor.Filename, content, l.mockFS), + ) + case req := <-l.inspectChan: + // Test-only: fn runs on the owner, preserving single ownership. + req.resp <- req.fn(l.state) case res := <-l.workerPool.ResultChan(): l.handleWorkerResult(res) case <-l.resultChan: - l.frameChan <- l.state.layout(l.browserManager) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) } } } +// fullContentBytes reconstructs the full file content from the chunked +// buffer (or the deprecated full Buffer). Returns ok=false on error. +// Must be called on the logic goroutine. +func (l *Logic) fullContentBytes() ([]byte, bool) { + if l.state.Editor.ChunkedBuffer != nil { + fullContent, err := l.state.Editor.ChunkedBuffer.FullContent() + if err != nil { + log.Printf("Error reconstructing full content: %v", err) + return nil, false + } + return []byte(fullContent), true + } + return []byte(l.state.Editor.Buffer), true +} + // markDirty triggers the auto-save debounce timer. +// Must be called on the logic goroutine. +// +// The timer callback runs on a timer goroutine and only sends a token on +// autosaveChan; the owner (Run loop) does all state reads and the worker +// dispatch. This keeps the timer goroutine out of state (architecture.md §1). func (l *Logic) markDirty() { if l.state.Editor.Filename == "" { return @@ -235,30 +272,8 @@ func (l *Logic) markDirty() { if l.saveTimer != nil { l.saveTimer.Stop() } - gen := l.saveGeneration - l.saveGeneration++ - filename := l.state.Editor.Filename - l.saveTimer = time.AfterFunc(1*time.Second, func() { - log.Printf("Logic: Auto-save timer fired for %s, gen=%d, current=%d", filename, gen+1, l.saveGeneration) - if l.saveGeneration == gen+1 { - // Reconstruct full content from chunked buffer for saving - var content []byte - if l.state.Editor.ChunkedBuffer != nil { - fullContent, err := l.state.Editor.ChunkedBuffer.FullContent() - if err != nil { - log.Printf("Error reconstructing full content for auto-save: %v", err) - return - } - content = []byte(fullContent) - } else { - content = []byte(l.state.Editor.Buffer) - } - log.Printf("Logic: Dispatching WriteFileTask for %s, content len=%d", filename, len(content)) - l.workerPool.DispatchNonBlocking( - pool.NewWriteFileTask(filename, content, l.mockFS), - ) - } + l.autosaveChan <- struct{}{} }) } @@ -344,7 +359,7 @@ func (l *Logic) handleWorkerResult(res pool.Result) { }) } } - l.frameChan <- l.state.layout(l.browserManager) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) } // applyBuildIndexResult applies a completed BuildIndexTask result to browser state. @@ -387,24 +402,24 @@ func (l *Logic) Done() { close(l.done) } -// FlushAll triggers synchronous writes for all dirty files. -func (l *Logic) FlushAll() { - l.mu.Lock() - defer l.mu.Unlock() +// WaitForExit blocks until the logic goroutine has fully stopped. After +// this returns, the caller (e.g. Shutdown) may touch state single-threaded. +func (l *Logic) WaitForExit() { + l.exitWg.Wait() +} +// FlushAll triggers synchronous writes for all dirty files. +// +// Must be called either from the logic goroutine (e.g. via GoToBrowser) or +// after the logic goroutine has fully stopped (Shutdown). It touches state +// directly, so it must never run concurrently with Run (single-owner +// invariant, architecture.md §1). +func (l *Logic) FlushAll() { for filename := range l.state.Editor.fileVersion { if l.state.Editor.IsDirty() && l.state.Editor.Filename == filename { - // Reconstruct full content from chunked buffer for saving - var content []byte - if l.state.Editor.ChunkedBuffer != nil { - fullContent, err := l.state.Editor.ChunkedBuffer.FullContent() - if err != nil { - log.Printf("Error reconstructing full content for flush: %v", err) - return - } - content = []byte(fullContent) - } else { - content = []byte(l.state.Editor.Buffer) + content, ok := l.fullContentBytes() + if !ok { + continue } // In a real app, this would be a blocking call to the FS l.mockFS.WriteFileAtomic(filename, content) @@ -414,8 +429,12 @@ func (l *Logic) FlushAll() { } // Shutdown gracefully shuts down the logic goroutine and worker pool. +// The logic goroutine is stopped FIRST so that FlushAll can touch state +// single-threaded; the worker pool is stopped last so in-flight results +// still have a reader until then. func (l *Logic) Shutdown() { - l.FlushAll() l.Done() + l.WaitForExit() + l.FlushAll() l.workerPool.Stop() } diff --git a/internal/io/pool/worker_pool_test.go b/internal/io/pool/worker_pool_test.go index 6ba1c50..14a7d56 100644 --- a/internal/io/pool/worker_pool_test.go +++ b/internal/io/pool/worker_pool_test.go @@ -156,19 +156,39 @@ func TestWorkerPool_PriorityPreemption(t *testing.T) { var lowExecuted atomic.Bool var highExecuted atomic.Bool - // Low priority task that takes time + // Gate task: occupies the single worker until we release it, so that the + // low- and high-priority test tasks both sit in their queues first. + gateStarted := make(chan struct{}) + release := make(chan struct{}) + gateTask := &stubTask{ + id: "gate", + taskType: TypeSaveState, + priority: HighPriority, + execute: func() (any, error) { + close(gateStarted) + <-release + return nil, nil + }, + } + pool.Dispatch(gateTask) + + // Wait until the worker is inside the gate task. + select { + case <-gateStarted: + case <-time.After(2 * time.Second): + t.Fatal("Timed out waiting for gate task to start") + } + + // Queue a low-priority task, then a high-priority one. lowTask := &stubTask{ id: "low", taskType: TypeSaveState, priority: LowPriority, execute: func() (any, error) { - time.Sleep(100 * time.Millisecond) lowExecuted.Store(true) return nil, nil }, } - - // High priority task that completes quickly highTask := &stubTask{ id: "high", taskType: TypeReadFile, @@ -178,34 +198,43 @@ func TestWorkerPool_PriorityPreemption(t *testing.T) { return "fast", nil }, } - - // Dispatch low priority first pool.Dispatch(lowTask) - - // Immediately dispatch high priority pool.Dispatch(highTask) - // Wait for high priority result (should complete before low) + // Release the worker; it must pick the high-priority task first. + close(release) + + // Gate result first. + select { + case result := <-pool.resultChan: + if result.TaskID != "gate" { + t.Fatalf("First result TaskID = %q, want %q", result.TaskID, "gate") + } + case <-time.After(2 * time.Second): + t.Fatal("Timed out waiting for gate result") + } + + // High priority result before low priority. select { case result := <-pool.resultChan: if result.TaskID != "high" { - t.Errorf("First result TaskID = %q, want %q", result.TaskID, "high") + t.Errorf("Second result TaskID = %q, want %q", result.TaskID, "high") } if !highExecuted.Load() { - t.Error("High priority task should have executed first") + t.Error("High priority task should have executed before low") } case <-time.After(2 * time.Second): t.Fatal("Timed out waiting for high priority result") } - // Wait for low priority result + // Low priority result last. select { case result := <-pool.resultChan: if result.TaskID != "low" { - t.Errorf("Second result TaskID = %q, want %q", result.TaskID, "low") + t.Errorf("Third result TaskID = %q, want %q", result.TaskID, "low") } if !lowExecuted.Load() { - t.Error("Low priority task should have executed second") + t.Error("Low priority task should have executed last") } case <-time.After(2 * time.Second): t.Fatal("Timed out waiting for low priority result") diff --git a/internal/test/e2e/browser_files_test.go b/internal/test/e2e/browser_files_test.go index 6775617..4988bb3 100644 --- a/internal/test/e2e/browser_files_test.go +++ b/internal/test/e2e/browser_files_test.go @@ -26,12 +26,17 @@ func TestBrowserFilesVisible(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - // Set VisibleCount based on viewport - state := h.State() - state.Browser.VisibleCount = computeVisibleCount(state.PixelHeight, state.Scale()) + // Set VisibleCount based on viewport (owner-side write) + if err := h.WithState(func(st *editor.State) { + st.Browser.VisibleCount = computeVisibleCount(st.PixelHeight, st.Scale()) + }); err != nil { + t.Fatalf("WithState: %v", err) + } // Switch to browser page - editor.GoToBrowser(nil) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } // Trigger a frame h.SendConfig(780, 1688) @@ -68,10 +73,14 @@ func TestBrowserFilesVisible(t *testing.T) { // The mock filesystem has 5 root-level files and 6 directories = 11 entries if len(listView.Items) == 0 { t.Errorf("browser ListView has 0 items — files from mock filesystem not showing up") - t.Logf("BrowserState: TotalEntries=%d, VisibleCount=%d, ScrollOffset=%.1f", - state.Browser.TotalEntries, - state.Browser.VisibleCount, - state.Browser.ScrollOffset) + v, err := h.Inspect(func(st *editor.State) any { + return fmt.Sprintf("BrowserState: TotalEntries=%d, VisibleCount=%d, ScrollOffset=%.1f", + st.Browser.TotalEntries, st.Browser.VisibleCount, float64(st.Browser.ScrollOffset)) + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + t.Log(v) } // Verify some known files are present diff --git a/internal/test/e2e/cursor_interaction_test.go b/internal/test/e2e/cursor_interaction_test.go index 61b15dd..ec2e602 100644 --- a/internal/test/e2e/cursor_interaction_test.go +++ b/internal/test/e2e/cursor_interaction_test.go @@ -10,12 +10,18 @@ import ( ) // TestEditorClickToMoveCursor tests that clicking/tapping in the editor moves the cursor. +// +// All state access goes through the harness's owner-side helpers +// (WithState / CursorPosition): the logic goroutine is the sole owner of +// State (architecture.md §1). func TestEditorClickToMoveCursor(t *testing.T) { h := e2e.NewHarnessWithDefaults() defer h.Cleanup() // Switch to editor page and load some text - editor.GoToEditor(nil) + if err := h.WithState(func(st *editor.State) { editor.GoToEditor(nil) }); err != nil { + t.Fatalf("GoToEditor: %v", err) + } h.SendConfig(780, 1688) // Wait for frame to ensure page switch @@ -24,18 +30,26 @@ func TestEditorClickToMoveCursor(t *testing.T) { t.Fatalf("timeout waiting for frame: %v", err) } - // Initialize GlyphLayout for the test - editor.TheState.Editor.GlyphLayout = ui.GlyphLayout{ - ByteOffsets: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, - X: []ui.Dp{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120}, - Y: []ui.Dp{70, 70, 70, 70, 70, 70, 70, 70, 70, 70, 70, 70}, - Advance: []ui.Dp{10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10}, + // Initialize GlyphLayout for the test (owner-side write) + if err := h.WithState(func(st *editor.State) { + st.Editor.GlyphLayout = ui.GlyphLayout{ + ByteOffsets: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, + X: []ui.Dp{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120}, + Y: []ui.Dp{70, 70, 70, 70, 70, 70, 70, 70, 70, 70, 70, 70}, + Advance: []ui.Dp{10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10}, + } + }); err != nil { + t.Fatalf("set GlyphLayout: %v", err) } // Initial cursor position should be 0 (or end of text, depending on implementation) // For this test, let's assume it starts at 0. - if editor.TheState.Editor.CursorPosition != 0 { - t.Errorf("expected initial cursor 0, got %d", editor.TheState.Editor.CursorPosition) + pos, err := h.CursorPosition() + if err != nil { + t.Fatalf("CursorPosition: %v", err) + } + if pos != 0 { + t.Errorf("expected initial cursor 0, got %d", pos) } // Simulate tap at a position that should move the cursor. @@ -58,7 +72,11 @@ func TestEditorClickToMoveCursor(t *testing.T) { } // Assert cursor moved - if editor.TheState.Editor.CursorPosition == 0 { + pos, err = h.CursorPosition() + if err != nil { + t.Fatalf("CursorPosition: %v", err) + } + if pos == 0 { t.Errorf("expected cursor to move from 0, but it remained at 0") } } diff --git a/internal/test/e2e/edit_lifecycle_test.go b/internal/test/e2e/edit_lifecycle_test.go index 3b1ca17..9b97b75 100644 --- a/internal/test/e2e/edit_lifecycle_test.go +++ b/internal/test/e2e/edit_lifecycle_test.go @@ -1,6 +1,7 @@ package e2e_test import ( + "strings" "testing" "time" @@ -9,22 +10,35 @@ import ( "pad/internal/ui" ) +// TestEditLifecycle verifies that edits made through the editor are persisted +// when the file is closed (via GoToBrowser -> FlushAll). +// +// All state access goes through the harness's owner-side helpers: the logic +// goroutine is the sole owner of State (architecture.md §1). func TestEditLifecycle(t *testing.T) { h := e2e.NewHarnessWithDefaults() defer h.Cleanup() // 1. Go to browser page - editor.GoToBrowser(nil) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } time.Sleep(200 * time.Millisecond) // 2. Open File filename := "/notes.txt" - editor.OpenFile(filename) + if err := h.WithState(func(st *editor.State) { editor.OpenFile(filename) }); err != nil { + t.Fatalf("OpenFile: %v", err) + } // Wait for file to load success := false for i := 0; i < 20; i++ { - if h.State().Editor.ChunkedBuffer != nil && h.State().Editor.ChunkedBuffer.FileLen() > 0 { + loaded, err := h.FileLoaded() + if err != nil { + t.Fatalf("FileLoaded: %v", err) + } + if loaded { success = true break } @@ -36,20 +50,26 @@ func TestEditLifecycle(t *testing.T) { // 3. Edit File // Original content of notes.txt is 198 bytes, let's append " UPDATED" - editor.HandleInsert(" UPDATED") + if err := h.WithState(func(st *editor.State) { editor.HandleInsert(" UPDATED") }); err != nil { + t.Fatalf("HandleInsert: %v", err) + } time.Sleep(100 * time.Millisecond) // 4. Close File (Go back to browser) // This will trigger FlushAll() - editor.GoToBrowser(nil) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } time.Sleep(500 * time.Millisecond) // Ensure it had time to process close // 5. Re-open File - editor.OpenFile(filename) + if err := h.WithState(func(st *editor.State) { editor.OpenFile(filename) }); err != nil { + t.Fatalf("OpenFile: %v", err) + } time.Sleep(1000 * time.Millisecond) // Wait for re-open and load // 6. Verify Edits - fullContent, err := h.State().Editor.ChunkedBuffer.FullContent() + fullContent, err := h.FullContent() if err != nil { t.Fatalf("Failed to reconstruct content: %v", err) } @@ -66,7 +86,7 @@ func TestEditLifecycle(t *testing.T) { for _, child := range container.Children { if label, ok := child.(ui.Label); ok { // The cursor position text is in the middle, containing "/" - if len(label.Text) > 0 && label.Text[0] != '/' && contains(label.Text, "/") { + if len(label.Text) > 0 && label.Text[0] != '/' && strings.Contains(label.Text, "/") { sizeText = label.Text } } @@ -79,12 +99,3 @@ func TestEditLifecycle(t *testing.T) { t.Errorf("Bottom bar size label mismatch: got %q, expected %q", sizeText, expectedText) } } - -func contains(s, substr string) bool { - for i := 0; i < len(s); i++ { - if s[i] == substr[0] { - return true - } - } - return false -} diff --git a/internal/test/e2e/harness.go b/internal/test/e2e/harness.go index 25e571d..682e033 100644 --- a/internal/test/e2e/harness.go +++ b/internal/test/e2e/harness.go @@ -1,6 +1,7 @@ package e2e import ( + "fmt" "sync" "time" @@ -8,6 +9,8 @@ import ( "pad/internal/ui" ) +var errInspectTimeout = fmt.Errorf("e2e: inspect timed out waiting for the logic goroutine") + // Harness orchestrates the test environment for e2e tests. type Harness struct { logic *editor.Logic @@ -15,6 +18,7 @@ type Harness struct { logicWg sync.WaitGroup frameReceiverDone chan struct{} wg sync.WaitGroup + started bool } // HarnessOption configures the test harness. @@ -36,7 +40,13 @@ func NewHarness(opts ...HarnessOption) *Harness { } // Run starts the logic goroutine and frame capture. +// It must be called at most once: two Run loops on the same channels and +// state would race (NewHarnessWithDefaults already calls it). func (h *Harness) Run() { + if h.started { + panic("e2e: Harness.Run called twice") + } + h.started = true h.logicWg.Add(1) h.wg.Add(1) go func() { @@ -51,7 +61,7 @@ func (h *Harness) Run() { for { select { case frame := <-h.logic.FrameChan(): - h.capture.CaptureFrame(frame) + h.capture.CaptureFrame(frame.Elems) case <-h.frameReceiverDone: return } @@ -92,9 +102,65 @@ func (h *Harness) FrameCount() int { return h.capture.FrameCount() } -// State returns the editor state for inspection. -func (h *Harness) State() *editor.State { - return h.logic.State() +// Inspect runs fn on the logic goroutine and returns its result. +// This is the ONLY sanctioned way for a test to read or write state: the fn +// executes on the owner, preserving the single-owner invariant +// (architecture.md §1). fn must not block on sends to logic channels. +func (h *Harness) Inspect(fn func(st *editor.State) any) (any, error) { + v, ok := h.logic.Inspect(fn) + if !ok { + return nil, errInspectTimeout + } + return v, nil +} + +// WithState runs fn on the logic goroutine for state setup or mutation. +func (h *Harness) WithState(fn func(st *editor.State)) error { + _, err := h.Inspect(func(st *editor.State) any { + fn(st) + return nil + }) + return err +} + +// FileLoaded reports whether the active file's chunked buffer has loaded +// content (owner-side check). +func (h *Harness) FileLoaded() (bool, error) { + v, err := h.Inspect(func(st *editor.State) any { + cb := st.Editor.ChunkedBuffer + return cb != nil && cb.FileLen() > 0 + }) + if err != nil { + return false, err + } + return v.(bool), nil +} + +// FullContent returns the active file's full content (owner-side). +func (h *Harness) FullContent() (string, error) { + v, err := h.Inspect(func(st *editor.State) any { + if st.Editor.ChunkedBuffer != nil { + full, err := st.Editor.ChunkedBuffer.FullContent() + if err != nil { + return "" + } + return full + } + return st.Editor.Buffer + }) + if err != nil { + return "", err + } + return v.(string), nil +} + +// CursorPosition returns the editor cursor position (owner-side). +func (h *Harness) CursorPosition() (int, error) { + v, err := h.Inspect(func(st *editor.State) any { return st.Editor.CursorPosition }) + if err != nil { + return 0, err + } + return v.(int), nil } // WaitForFrameCount blocks until at least N frames are captured. diff --git a/internal/test/e2e/harness_test.go b/internal/test/e2e/harness_test.go index 3d4770e..f6a6b73 100644 --- a/internal/test/e2e/harness_test.go +++ b/internal/test/e2e/harness_test.go @@ -15,8 +15,10 @@ func TestEditorInitialLayout(t *testing.T) { h := e2e.NewHarnessWithDefaults() defer h.Cleanup() - // Switch to editor page - editor.GoToEditor(nil) + // Switch to editor page (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToEditor(nil) }); err != nil { + t.Fatalf("GoToEditor: %v", err) + } h.SendConfig(780, 1688) // Wait for a new frame after switching to editor page diff --git a/internal/test/e2e/scroll_cursor_test.go b/internal/test/e2e/scroll_cursor_test.go index ae2add2..7435fc9 100644 --- a/internal/test/e2e/scroll_cursor_test.go +++ b/internal/test/e2e/scroll_cursor_test.go @@ -9,13 +9,21 @@ import ( "pad/internal/ui" ) +// TestEditorClickToMoveCursorWithScroll verifies that a tap in the editor +// accounts for the current scroll offset and lands on the right line. +// +// State setup/assertions use the harness's owner-side helpers (WithState / +// CursorPosition / Inspect); the tap handler closure runs on the logic +// goroutine via SendInput, so its reads of editor.TheState are safe. func TestEditorClickToMoveCursorWithScroll(t *testing.T) { + // NewHarnessWithDefaults already starts the logic goroutine. h := e2e.NewHarnessWithDefaults() - h.Run() // Start the harness! defer h.Cleanup() - // Switch to editor page - editor.GoToEditor(nil) + // Switch to editor page (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToEditor(nil) }); err != nil { + t.Fatalf("GoToEditor: %v", err) + } h.SendConfig(780, 1688) // Give it a moment to initialize time.Sleep(200 * time.Millisecond) @@ -31,20 +39,24 @@ func TestEditorClickToMoveCursorWithScroll(t *testing.T) { // - 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) + if err := h.WithState(func(st *editor.State) { + st.Editor.Buffer = "Line 1\nLine 2\nLine 3" - // 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}, + // Scroll to start of Line 2 (skip Line 1) + st.ScrollOffset = ui.Dp(lineHeight) + + // GlyphLayout: + // Line 1: y = 0 + // Line 2: y = 16.8 + // Line 3: y = 33.6 + st.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}, + } + }); err != nil { + t.Fatalf("WithState: %v", err) } // Tap at y=10 (within the text area). @@ -52,7 +64,7 @@ func TestEditorClickToMoveCursorWithScroll(t *testing.T) { // 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. @@ -60,10 +72,14 @@ func TestEditorClickToMoveCursorWithScroll(t *testing.T) { // 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) - + + // Let's debug by printing in the test (owner-side read). + v, err := h.Inspect(func(st *editor.State) any { return float64(st.ScrollOffset) }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + t.Logf("ScrollOffset: %v", v) + h.SendInput([]ui.InputEvent{ { Handler: func(data any) { @@ -81,8 +97,12 @@ func TestEditorClickToMoveCursorWithScroll(t *testing.T) { // 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) + pos, err := h.CursorPosition() + if err != nil { + t.Fatalf("CursorPosition: %v", err) + } + t.Logf("Cursor position: %d", pos) + if pos != 7 { + t.Errorf("expected cursor to move to 7 (Line 2), but got %d", pos) } } diff --git a/internal/test/e2e/search_test.go b/internal/test/e2e/search_test.go index 99b0358..c365043 100644 --- a/internal/test/e2e/search_test.go +++ b/internal/test/e2e/search_test.go @@ -22,8 +22,10 @@ func TestSearchFiltersList(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - // Navigate to browser page - editor.GoToBrowser(nil) + // Navigate to browser page (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) @@ -84,8 +86,10 @@ func TestSearchWithSortModeChange(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - // Navigate to browser page - editor.GoToBrowser(nil) + // Navigate to browser page (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) @@ -112,8 +116,10 @@ func TestSearchWithSortModeChange(t *testing.T) { } } - // Now toggle sort mode WHILE search is active - editor.ToggleSortOrder(nil) + // Now toggle sort mode WHILE search is active (owner-side) + if err := h.WithState(func(st *editor.State) { editor.ToggleSortOrder(nil) }); err != nil { + t.Fatalf("ToggleSortOrder: %v", err) + } h.SendConfig(780, 1688) time.Sleep(300 * time.Millisecond) @@ -194,10 +200,10 @@ func TestSortModeChangePreservesSearchFilter(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - state := h.State() - - // Navigate to browser - editor.GoToBrowser(nil) + // Navigate to browser (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) @@ -206,25 +212,33 @@ func TestSortModeChangePreservesSearchFilter(t *testing.T) { h.SendSearchQuery(query) time.Sleep(200 * time.Millisecond) - // Record search results before sort change - searchResultsBefore := make([]int, len(state.Browser.SearchResults)) - copy(searchResultsBefore, state.Browser.SearchResults) - t.Logf("Search results before sort change: %v", searchResultsBefore) + // Record search results before sort change (owner-side snapshot) + _, err = h.Inspect(func(st *editor.State) any { + searchResultsBefore := make([]int, len(st.Browser.SearchResults)) + copy(searchResultsBefore, st.Browser.SearchResults) + t.Logf("Search results before sort change: %v", searchResultsBefore) - // Verify all pre-sort results are valid matches - for _, sortedIdx := range state.Browser.SearchResults { - positionMap := state.Browser.GetSortedIndices() - rawIdx := positionMap[sortedIdx] - if rawIdx < len(state.Browser.SortIndex.Entries) { - entry := state.Browser.SortIndex.Entries[rawIdx] - if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { - t.Errorf("pre-sort: index %d (raw %d) -> %q does not match query %q", sortedIdx, rawIdx, entry.Name, query) + // Verify all pre-sort results are valid matches + for _, sortedIdx := range st.Browser.SearchResults { + positionMap := st.Browser.GetSortedIndices() + rawIdx := positionMap[sortedIdx] + if rawIdx < len(st.Browser.SortIndex.Entries) { + entry := st.Browser.SortIndex.Entries[rawIdx] + if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { + t.Errorf("pre-sort: index %d (raw %d) -> %q does not match query %q", sortedIdx, rawIdx, entry.Name, query) + } } } + return nil + }) + if err != nil { + t.Fatalf("Inspect: %v", err) } - // Toggle sort mode - editor.ToggleSortOrder(nil) + // Toggle sort mode (owner-side) + if err := h.WithState(func(st *editor.State) { editor.ToggleSortOrder(nil) }); err != nil { + t.Fatalf("ToggleSortOrder: %v", err) + } h.SendConfig(780, 1688) time.Sleep(300 * time.Millisecond) @@ -237,23 +251,29 @@ func TestSortModeChangePreservesSearchFilter(t *testing.T) { // Let's print the entries to see if they are still correct, // ignoring the index values themselves. - - for _, sortedIdx := range state.Browser.SearchResults { - if sortedIdx >= len(state.Browser.SortIndex.Entries) { - t.Errorf("search result index %d is out of bounds (total: %d)", - sortedIdx, state.Browser.TotalEntries) - continue - } - - // Map sortedIdx back to rawIdx to check the actual entry - positionMap := state.Browser.GetSortedIndices() - rawIdx := positionMap[sortedIdx] - - entry := state.Browser.SortIndex.Entries[rawIdx] - if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { - t.Errorf("post-sort: search result sortedIdx %d -> rawIdx %d -> %q does not match query %q", - sortedIdx, rawIdx, entry.Name, query) + // (Owner-side snapshot: all reads happen on the logic goroutine.) + _, err = h.Inspect(func(st *editor.State) any { + for _, sortedIdx := range st.Browser.SearchResults { + if sortedIdx >= len(st.Browser.SortIndex.Entries) { + t.Errorf("search result index %d is out of bounds (total: %d)", + sortedIdx, st.Browser.TotalEntries) + continue + } + + // Map sortedIdx back to rawIdx to check the actual entry + positionMap := st.Browser.GetSortedIndices() + rawIdx := positionMap[sortedIdx] + + entry := st.Browser.SortIndex.Entries[rawIdx] + if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { + t.Errorf("post-sort: search result sortedIdx %d -> rawIdx %d -> %q does not match query %q", + sortedIdx, rawIdx, entry.Name, query) + } } + return nil + }) + if err != nil { + t.Fatalf("Inspect: %v", err) } } @@ -268,8 +288,10 @@ func TestSearchClearRestoresFullList(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - // Navigate to browser page - editor.GoToBrowser(nil) + // Navigate to browser page (owner-side) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) diff --git a/internal/test/e2e/sort_mode_test.go b/internal/test/e2e/sort_mode_test.go index 8306eb3..84f94af 100644 --- a/internal/test/e2e/sort_mode_test.go +++ b/internal/test/e2e/sort_mode_test.go @@ -11,6 +11,42 @@ import ( "pad/internal/ui" ) +// sortModeOf reads the browser sort mode on the logic goroutine (owner). +func sortModeOf(t *testing.T, h *e2e.Harness) browser.SortMode { + t.Helper() + v, err := h.Inspect(func(st *editor.State) any { return st.Browser.SortMode }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + return v.(browser.SortMode) +} + +// pageCountOf reads the number of cached browser pages on the logic goroutine. +func pageCountOf(t *testing.T, h *e2e.Harness) int { + t.Helper() + v, err := h.Inspect(func(st *editor.State) any { return len(st.Browser.Pages) }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + return v.(int) +} + +// goBrowser navigates to the browser page on the logic goroutine. +func goBrowser(t *testing.T, h *e2e.Harness) { + t.Helper() + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } +} + +// toggleSort toggles the browser sort mode on the logic goroutine. +func toggleSort(t *testing.T, h *e2e.Harness) { + t.Helper() + if err := h.WithState(func(st *editor.State) { editor.ToggleSortOrder(nil) }); err != nil { + t.Fatalf("ToggleSortOrder: %v", err) + } +} + // TestSortModeToggleCyclesThroughAllModes verifies that clicking the // sort mode label cycles through all four sort modes. // @@ -26,10 +62,8 @@ func TestSortModeToggleCyclesThroughAllModes(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - state := h.State() - // Navigate to browser page - editor.GoToBrowser(nil) + goBrowser(t, h) // Trigger a frame h.SendConfig(780, 1688) @@ -41,7 +75,7 @@ func TestSortModeToggleCyclesThroughAllModes(t *testing.T) { // Record the initial order of list items initialItems := getListItems(t, lastFrame) - initialSortMode := state.Browser.SortMode + initialSortMode := sortModeOf(t, h) t.Logf("Initial list items: %v", initialItems) t.Logf("Initial SortMode: %d", initialSortMode) @@ -51,9 +85,9 @@ func TestSortModeToggleCyclesThroughAllModes(t *testing.T) { // Tap the sort label to cycle through all 4 modes for cycle := 0; cycle < 4; cycle++ { - // Simulate tapping the sort label by calling the editor's ToggleSortOrder directly + // Simulate tapping the sort label by calling the editor's ToggleSortOrder // (since SendInput doesn't route to the correct handler in e2e context) - editor.ToggleSortOrder(nil) + toggleSort(t, h) // Trigger a new frame and wait for reload after cache clear h.SendConfig(780, 1688) @@ -63,7 +97,7 @@ func TestSortModeToggleCyclesThroughAllModes(t *testing.T) { lastFrame = frames[len(frames)-1] newItems := getListItems(t, lastFrame) - newSortMode := state.Browser.SortMode + newSortMode := sortModeOf(t, h) t.Logf("Cycle %d: SortMode=%d, items=%v", cycle, newSortMode, newItems) @@ -81,7 +115,7 @@ func TestSortModeToggleCyclesThroughAllModes(t *testing.T) { } // Verify the sort mode was actually changed in state - t.Logf("Final SortMode: %d", state.Browser.SortMode) + t.Logf("Final SortMode: %d", sortModeOf(t, h)) } // TestSortModeToggleChangesEntryOrder verifies that toggling the sort mode @@ -95,10 +129,8 @@ func TestSortModeToggleChangesEntryOrder(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - state := h.State() - // Navigate to browser page - editor.GoToBrowser(nil) + goBrowser(t, h) h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) @@ -107,7 +139,7 @@ func TestSortModeToggleChangesEntryOrder(t *testing.T) { // Record initial state initialItems := getListItems(t, lastFrame) - initialSortMode := state.Browser.SortMode + initialSortMode := sortModeOf(t, h) t.Logf("Before toggle: SortMode=%d, items=%v", initialSortMode, initialItems) @@ -116,7 +148,7 @@ func TestSortModeToggleChangesEntryOrder(t *testing.T) { } // Toggle sort mode using editor's function (browser's was removed) - editor.ToggleSortOrder(nil) + toggleSort(t, h) // Trigger a new frame h.SendConfig(780, 1688) @@ -126,7 +158,7 @@ func TestSortModeToggleChangesEntryOrder(t *testing.T) { lastFrame = frames[len(frames)-1] newItems := getListItems(t, lastFrame) - newSortMode := state.Browser.SortMode + newSortMode := sortModeOf(t, h) t.Logf("After toggle: SortMode=%d, items=%v", newSortMode, newItems) @@ -159,28 +191,26 @@ func TestSortModePagesNotClearedAfterToggle(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - state := h.State() - // Navigate to browser page - editor.GoToBrowser(nil) + goBrowser(t, h) h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) // Record the number of cached pages before toggle - pagesBefore := len(state.Browser.Pages) - sortModeBefore := state.Browser.SortMode + pagesBefore := pageCountOf(t, h) + sortModeBefore := sortModeOf(t, h) t.Logf("Before toggle: SortMode=%d, Pages=%d", sortModeBefore, pagesBefore) // Toggle sort mode using editor's function (browser's was removed) - editor.ToggleSortOrder(nil) + toggleSort(t, h) // Force a new frame to be rendered with the updated state h.SendConfig(780, 1688) time.Sleep(300 * time.Millisecond) - sortModeAfter := state.Browser.SortMode - pagesAfter := len(state.Browser.Pages) + sortModeAfter := sortModeOf(t, h) + pagesAfter := pageCountOf(t, h) t.Logf("After toggle: SortMode=%d, Pages=%d", sortModeAfter, pagesAfter) @@ -219,7 +249,7 @@ func TestSortModeLabelInHeader(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - editor.GoToBrowser(nil) + goBrowser(t, h) h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) @@ -264,20 +294,18 @@ func TestSortModeLabelChangesAfterToggle(t *testing.T) { t.Fatalf("timeout waiting for initial frames: %v", err) } - state := h.State() - // Navigate to browser page - editor.GoToBrowser(nil) + goBrowser(t, h) h.SendConfig(780, 1688) time.Sleep(200 * time.Millisecond) // Record initial sort mode - initialSortMode := state.Browser.SortMode + initialSortMode := sortModeOf(t, h) // Toggle sort mode using editor's function (browser's was removed) - editor.ToggleSortOrder(nil) + toggleSort(t, h) - newSortMode := state.Browser.SortMode + newSortMode := sortModeOf(t, h) t.Logf("SortMode changed from %d to %d", initialSortMode, newSortMode) diff --git a/internal/test/e2e/type_at_start_test.go b/internal/test/e2e/type_at_start_test.go index 595d22f..cd44df0 100644 --- a/internal/test/e2e/type_at_start_test.go +++ b/internal/test/e2e/type_at_start_test.go @@ -13,23 +13,36 @@ import ( // TestTypeAtStartOfBuffer verifies that typing at cursor position 0 inserts // characters at the start of the buffer without deleting characters from the end, // and that they actually appear on screen (in the visible TextField). +// +// State reads go through the harness's owner-side helpers (Inspect runs the +// callback on the logic goroutine, the sole state owner). func TestTypeAtStartOfBuffer(t *testing.T) { + // NewHarnessWithDefaults already starts the logic goroutine. h := e2e.NewHarnessWithDefaults() - h.Run() defer h.Cleanup() // 1. Navigate to browser, then open a file. - editor.GoToBrowser(nil) + if err := h.WithState(func(st *editor.State) { editor.GoToBrowser(nil) }); err != nil { + t.Fatalf("GoToBrowser: %v", err) + } time.Sleep(200 * time.Millisecond) filename := "/notes.txt" - editor.OpenFile(filename) + if err := h.WithState(func(st *editor.State) { editor.OpenFile(filename) }); err != nil { + t.Fatalf("OpenFile: %v", err) + } // 2. Wait for the file content to be loaded into the chunked buffer and line index built. loaded := false for i := 0; i < 30; i++ { - cb := h.State().Editor.ChunkedBuffer - if cb != nil && cb.FileLen() > 0 && cb.LineIndex != nil { + v, err := h.Inspect(func(st *editor.State) any { + cb := st.Editor.ChunkedBuffer + return cb != nil && cb.FileLen() > 0 && cb.LineIndex != nil + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + if v.(bool) { loaded = true break } @@ -39,18 +52,27 @@ func TestTypeAtStartOfBuffer(t *testing.T) { t.Fatal("timed out waiting for file to load and line index to build") } - // Record original file length and content - cb := h.State().Editor.ChunkedBuffer - originalLen := int(cb.FileLen()) - originalContent, err := cb.FullContent() + // Record original file length and content (owner-side reads) + v, err := h.Inspect(func(st *editor.State) any { + return int(st.Editor.ChunkedBuffer.FileLen()) + }) + if err != nil { + t.Fatalf("Inspect: %v", err) + } + originalLen := v.(int) + originalContent, err := h.FullContent() if err != nil { t.Fatalf("failed to read original content: %v", err) } originalTail := originalContent[max(0, originalLen-20):] // Confirm cursor is at position 0 (top of file). - if h.State().Editor.CursorPosition != 0 { - t.Fatalf("expected cursor at 0, got %d", h.State().Editor.CursorPosition) + pos, err := h.CursorPosition() + if err != nil { + t.Fatalf("CursorPosition: %v", err) + } + if pos != 0 { + t.Fatalf("expected cursor at 0, got %d", pos) } // Get initial frame to verify the text field content before typing. @@ -85,7 +107,7 @@ func TestTypeAtStartOfBuffer(t *testing.T) { time.Sleep(200 * time.Millisecond) // 5. Assert on the buffer content. - newContent, err := h.State().Editor.ChunkedBuffer.FullContent() + newContent, err := h.FullContent() if err != nil { t.Fatalf("failed to read content after insert: %v", err) } @@ -135,7 +157,7 @@ func TestTypeAtStartOfBuffer(t *testing.T) { // 6c. The visible text field length must be correct. if len(finalTextField.Value) != expectedLen { - t.Errorf("expected visible text field length %d, got %d", expectedLen, len(finalTextField.Value)) + t.Errorf("expected visible text field length %d, got %d", len(finalTextField.Value), expectedLen) } } diff --git a/internal/ui/element.go b/internal/ui/element.go index 442d25e..48768ae 100644 --- a/internal/ui/element.go +++ b/internal/ui/element.go @@ -12,7 +12,6 @@ import ( "gioui.org/op/clip" "gioui.org/op/paint" "gioui.org/unit" - "gioui.org/widget" "gioui.org/io/key" ) @@ -642,13 +641,14 @@ func NewSpacer(height Dp) Spacer { } } -// GioEditor wraps a Gio widget.Editor for single-line or multiline text input. +// GioEditor is a reference to a main-owned widget.Editor (see +// Renderer.RegisterGioEditor). The element itself is pure data: the mutable +// widget state lives in the renderer, which is owned by the main goroutine. type GioEditor struct { id string region Region visible bool interactions []Interaction - Editor *widget.Editor } func (ge GioEditor) Type() string { return "gioeditor" } @@ -662,9 +662,13 @@ func (ge GioEditor) String() string { return fmt.Sprintf("GioEditor[%s] region=%+v", ge.id, ge.region) } -// Draw renders the Gio Editor widget. +// Draw renders the Gio Editor widget registered for this element's ID. func (ge GioEditor) Draw(gtx layout.Context, r *Renderer) { - ge.Editor.SingleLine = true + ed, ok := r.GioEditor(ge.id) + if !ok { + return + } + ed.SingleLine = true // Position and clip to the editor's region stack := op.Offset(image.Pt(int(r.toPx(ge.region.X)), int(r.toPx(ge.region.Y)))).Push(gtx.Ops) @@ -696,16 +700,16 @@ func (ge GioEditor) Draw(gtx layout.Context, r *Renderer) { } r.RegisterClick(gtx, ge.id, ge.region, tapHandler) - ge.Editor.Layout(gtx, r.shp, font.Font{}, r.theme.FontSize, textColor, selectionColor) + ed.Layout(gtx, r.shp, font.Font{}, r.theme.FontSize, textColor, selectionColor) } -// NewGioEditor creates a GioEditor element wrapping a widget.Editor. -func NewGioEditor(id string, region Region, editor *widget.Editor) GioEditor { +// NewGioEditor creates a GioEditor element referencing the widget.Editor +// registered with the renderer under id. +func NewGioEditor(id string, region Region) GioEditor { return GioEditor{ id: id, region: region, visible: true, - Editor: editor, } } diff --git a/internal/ui/render.go b/internal/ui/render.go index 3d9907f..48271c6 100644 --- a/internal/ui/render.go +++ b/internal/ui/render.go @@ -22,6 +22,7 @@ import ( "gioui.org/op/paint" "gioui.org/text" "gioui.org/unit" + "gioui.org/widget" "golang.org/x/image/math/fixed" ) @@ -31,11 +32,6 @@ const maxInt32 = 1<<31 - 1 //go:embed icons/*.png var iconFS embed.FS -// ScaleProvider provides access to the current scale factor. -type ScaleProvider interface { - Scale() float32 -} - // clickReg pairs a gesture.Click with its handler. type clickReg struct { click *gesture.Click @@ -54,34 +50,52 @@ type scrollReg struct { } // Renderer consumes a slice of elements and draws them. +// +// The Renderer is owned by the main goroutine. It is the home of any state +// that Gio mutates during draw (e.g. the search bar's widget.Editor): such +// state must not live in the logic goroutine's State (architecture.md §1). type Renderer struct { theme Theme shp *text.Shaper - scale ScaleProvider + scale float32 // px-per-Dp for the current draw pass; set in Draw icons map[string]image.Image clicks map[string]clickReg Keys map[string]keyReg // Exported Keys map scrolls map[string]scrollReg + gioEditors map[string]*widget.Editor // main-owned widget editors by element ID 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. -func New(th Theme, shp *text.Shaper, scale ScaleProvider) *Renderer { +func New(th Theme, shp *text.Shaper) *Renderer { r := &Renderer{ - theme: th, - shp: shp, - scale: scale, - icons: make(map[string]image.Image), - clicks: make(map[string]clickReg), - Keys: make(map[string]keyReg), - scrolls: make(map[string]scrollReg), + theme: th, + shp: shp, + icons: make(map[string]image.Image), + clicks: make(map[string]clickReg), + Keys: make(map[string]keyReg), + scrolls: make(map[string]scrollReg), + gioEditors: make(map[string]*widget.Editor), } r.loadIcons() return r } +// RegisterGioEditor attaches a main-owned widget.Editor to an element ID. +// GioEditor elements with that ID render the widget during draw. Must be +// called from the main goroutine before the first frame. +func (r *Renderer) RegisterGioEditor(id string, ed *widget.Editor) { + r.gioEditors[id] = ed +} + +// GioEditor returns the main-owned widget editor registered for id, if any. +func (r *Renderer) GioEditor(id string) (*widget.Editor, bool) { + ed, ok := r.gioEditors[id] + return ed, ok +} + // loadIcons loads PNG icons from the embedded filesystem. func (r *Renderer) loadIcons() { for _, name := range []string{"back", "cut", "copy", "paste"} { @@ -104,16 +118,20 @@ func (r *Renderer) icon(name string) image.Image { // toPx converts Dp to physical pixels using State's scale. func (r *Renderer) toPx(dp Dp) Px { - return ToPx(dp, r.scale.Scale()) + return ToPx(dp, r.scale) } // toDp converts physical pixels to Dp using State's scale. func (r *Renderer) toDp(px Px) Dp { - return ToDp(px, r.scale.Scale()) + return ToDp(px, r.scale) } // Draw iterates elements and draws each in slice order (back-to-front). -func (r *Renderer) Draw(gtx layout.Context, elems []Element) { +// Draw renders the given elements. scale is the px-per-Dp factor for this +// draw pass, taken from the frame's view-state snapshot (the renderer never +// reads logic state directly). +func (r *Renderer) Draw(gtx layout.Context, elems []Element, scale float32) { + r.scale = scale // Gio sets constraints to layout.Exact(windowSize), so Min==Max. Use (0,0) as Min. winW := gtx.Constraints.Max.X winH := gtx.Constraints.Max.Y @@ -374,7 +392,7 @@ func (r *Renderer) drawText(gtx layout.Context, str string, size unit.Sp, reg Re for g, ok := r.shp.NextGlyph(); ok; g, ok = r.shp.NextGlyph() { totalAdvance += g.Advance } - textW := Dp(float32(totalAdvance>>6) / r.scale.Scale()) + textW := Dp(float32(totalAdvance>>6) / r.scale) // Compute aligned X position var drawX Dp @@ -503,9 +521,9 @@ func (r *Renderer) drawWrappedText(gtx layout.Context, str string, reg Region, w // 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())) + layout.X = append(layout.X, Dp(float32(g.X>>6)/r.scale)) + layout.Y = append(layout.Y, Dp(float32(g.Y)/r.scale)) + layout.Advance = append(layout.Advance, Dp(float32(g.Advance>>6)/r.scale)) // Advance byteOffset by g.Runes. for i := uint16(0); i < g.Runes; i++ {