From 65b227cf48f6c0f98a4a197c32945286ac8bd38e Mon Sep 17 00:00:00 2001 From: samyfodil Date: Thu, 30 Jul 2026 08:49:56 -0500 Subject: [PATCH] fix(desktop): clear the dirty boundary set before painting, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A boundary re-dirtied while it was being recorded re-registers itself for the next frame (layer_tree.go, "if boundary re-dirtied, register it for next frame"). draw() then called ClearDirtyBoundaries() at the end of the frame, which threw that registration away. The widget's own sceneDirty stayed true, so every later InvalidateScene took the already-dirty O(1) guard and returned without notifying the window — the boundary was never painted again. The set is only the O(1) frame-skip gate: painting walks the tree on each boundary's sceneDirty and never reads it. So it can be cleared as soon as the gate has consumed it, which leaves anything registered during painting intact. Any widget written from a goroutine other than the UI thread hits this on the first frame that overlaps a write, and animated widgets that re-dirty during Draw are the same shape. A terminal emulator under continuous output froze within one frame and stayed frozen after the output stopped; the render loop kept being woken 60 times a second and skipped every frame. The test covers the app-level contract the fix depends on — a boundary that re-dirties during recording is still registered after a paint pass that began with a cleared set. draw() itself has no test: it needs a live gogpu.Context. --- app/dirty_boundary_repaint_test.go | 98 ++++++++++++++++++++++++++++++ desktop/desktop.go | 17 +++++- 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 app/dirty_boundary_repaint_test.go diff --git a/app/dirty_boundary_repaint_test.go b/app/dirty_boundary_repaint_test.go new file mode 100644 index 0000000..2e259a4 --- /dev/null +++ b/app/dirty_boundary_repaint_test.go @@ -0,0 +1,98 @@ +package app + +import ( + "testing" + + "github.com/gogpu/ui/event" + "github.com/gogpu/ui/geometry" + "github.com/gogpu/ui/widget" +) + +// selfDirtyingLeaf re-dirties itself while it is being drawn, the way an animated +// widget does (spinner) and the way any widget written from another goroutine does +// when the write lands mid-record. +type selfDirtyingLeaf struct { + widget.WidgetBase + dirtyDuringDraw bool +} + +func (w *selfDirtyingLeaf) Layout(_ widget.Context, c geometry.Constraints) geometry.Size { + return c.Constrain(geometry.Sz(48, 48)) +} + +func (w *selfDirtyingLeaf) Draw(_ widget.Context, canvas widget.Canvas) { + canvas.DrawRect(w.Bounds(), widget.RGBA8(255, 0, 0, 255)) + if w.dirtyDuringDraw { + w.SetNeedsRedraw(true) + } +} + +func (w *selfDirtyingLeaf) Event(_ widget.Context, _ event.Event) bool { return false } +func (w *selfDirtyingLeaf) Children() []widget.Widget { return nil } + +// A boundary that is still scene-dirty when its recording ends registers itself for +// the next frame (recordBoundary). The frame's flat dirty set is only the O(1) +// frame-skip gate, so the render loop consumes it BEFORE painting — and that +// registration, made during painting, has to survive to the next frame. +// +// It did not: draw() cleared the set at the end of the frame and threw the +// registration away. That is unrecoverable rather than a dropped frame, because the +// widget's own sceneDirty is still true, so every later InvalidateScene takes the +// already-dirty O(1) guard and returns without telling the window anything. The +// boundary is never painted again. A terminal emulator under continuous output froze +// within one frame and stayed frozen after the output stopped, while the render loop +// kept being woken 60 times a second and skipping every frame. +func TestDirtyBoundaryRegisteredDuringPaintSurvivesTheFrame(t *testing.T) { + cleanup := setupSceneRecorder(t) + defer cleanup() + + a := New() + w := a.Window() + + root := &testContainer{} + root.SetVisible(true) + root.SetRepaintBoundary(true) + root.SetBounds(geometry.NewRect(0, 0, 800, 600)) + root.SetScreenOrigin(geometry.Pt(0, 0)) + + child := &selfDirtyingLeaf{} + child.SetVisible(true) + child.SetBounds(geometry.NewRect(10, 10, 48, 48)) + child.SetParent(root) + root.kids = []widget.Widget{child} + + w.SetRoot(root) + + // Settle: record once, then clear everything the first paint dirtied. + PaintBoundaryLayersWithContext(root, nil, w.Context()) + w.ClearDirtyBoundaries() + w.ClearAfterPaint() + root.ClearSceneDirty() + widget.ClearRedrawInTree(root) + if w.HasDirtyBoundaries() { + t.Fatal("pre-condition: the set should be empty after a clear") + } + + // Frame N: something dirties the child, which propagates to the boundary. + child.dirtyDuringDraw = true + child.SetNeedsRedraw(true) + if !w.HasDirtyBoundaries() { + t.Fatal("pre-condition: a dirtied child must register its boundary") + } + + // The render loop consumes the gate, then paints. The child re-dirties itself + // mid-record, so the boundary registers itself for frame N+1. + w.ClearDirtyBoundaries() + PaintBoundaryLayersWithContext(root, nil, w.Context()) + + if !w.HasDirtyBoundaries() { + t.Fatal("a boundary that re-dirtied while painting was not registered for the next frame") + } + + // And it is still scene-dirty, which is what makes losing that registration + // permanent rather than a dropped frame: nothing re-registers a boundary whose + // InvalidateScene already took the already-dirty guard. + if !root.IsSceneDirty() { + t.Fatal("expected the boundary to still be scene-dirty after re-dirtying") + } +} diff --git a/desktop/desktop.go b/desktop/desktop.go index a26a8f7..19fca31 100644 --- a/desktop/desktop.go +++ b/desktop/desktop.go @@ -297,6 +297,22 @@ func (rl *renderLoop) draw(dc *gogpu.Context) { //nolint:gocyclo,cyclop,gocognit win.CollectDirtyRegions() prePaintDirtyRegions := win.DirtyRegions() + // Clear the flat dirty set BEFORE painting, not after. + // + // The set is only the O(1) frame-skip gate (needsAnyWork above); painting + // walks the tree on each boundary's own sceneDirty and never reads it. But + // painting WRITES to it: a boundary re-dirtied while it was being recorded + // re-registers itself for the next frame (layer_tree.go, "if boundary + // re-dirtied, register it for next frame"). Clearing afterwards threw that + // registration away, and since the widget's sceneDirty stayed true, its + // InvalidateScene hit the already-dirty O(1) guard forever after and never + // notified the window again — the boundary went permanently unpainted. + // + // Any widget written from another goroutine hits this on the first frame + // that overlaps a write: a terminal under continuous output froze on screen + // within one frame and stayed frozen after the output stopped. + win.ClearDirtyBoundaries() + // Paint main tree boundaries. app.PaintBoundaryLayersWithContext(root, nil, winCtx) @@ -399,7 +415,6 @@ func (rl *renderLoop) draw(dc *gogpu.Context) { //nolint:gocyclo,cyclop,gocognit cc.TrackDamageRect(image.Rect(0, 0, cw, ch)) } win.ClearAfterPaint() - win.ClearDirtyBoundaries() // Debug overlay: cyan flash-and-fade on dirty widget regions (ADR-023). // Suppress damage tracking — overlay is visualization, not content.