From 9bfa6bc1c2dca43312d3ba442b9004cb87fb2ff6 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Thu, 16 Apr 2026 14:46:32 -0700 Subject: [PATCH] app: relock contexts after refresh This fixes an Android regression introduced by 3e601e73c421929ce9e82a22786998d9fd8322f2 app: optimize window context locking On Android, that change can leave the app window visible but unrendered: the screen stays black instead of drawing the UI. The regression is in app/window.go: Refresh can update the underlying surface state without guaranteeing a subsequent Lock before rendering. That is a problem for Android, where Refresh and Lock have distinct roles: - androidContext.Refresh stages the native window for the EGL surface - androidContext.Lock performs CreateSurface, when needed, and MakeCurrent In other words, after Refresh, Android may require another Lock before GPU work can proceed correctly. This patch keeps explicit locking, but avoids unconditional per-frame Lock/Unlock calls. It adds a small amount of state to Window: - ctxNeedsLock is set when creating a context - ctxNeedsLock is set after Refresh - ctxNeedsLock is set after explicit unlock/release paths - Lock is called before GPU work only when ctxNeedsLock is true That keeps the optimized steady-state path while restoring the required Refresh -> Lock sequencing. I also added a regression test for validateAndProcess that checks: - Refresh forces a re-lock before drawing/present - steady-state frames do not re-lock redundantly Verification: - go test ./app Signed-off-by: Joe Julian --- app/window.go | 38 +++++++++++----- app/window_test.go | 107 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 app/window_test.go diff --git a/app/window.go b/app/window.go index 977ab6a1..20666e56 100644 --- a/app/window.go +++ b/app/window.go @@ -46,6 +46,10 @@ type Window struct { ctx context gpu gpu.GPU + // ctxNeedsLock tracks whether the rendering context must be made + // current again before the next GPU operation. Refresh paths, surface + // loss, and explicit unlocks all invalidate the current binding. + ctxNeedsLock bool // timer tracks the delayed invalidate goroutine. timer struct { // quit is shuts down the goroutine. @@ -146,6 +150,7 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops, if err != nil { return err } + w.ctxNeedsLock = true sync = true } } @@ -162,17 +167,19 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops, } return err } + w.ctxNeedsLock = true } - if w.ctx != nil { + if w.ctx != nil && w.ctxNeedsLock { if err := w.ctx.Lock(); err != nil { w.destroyGPU() return err } + w.ctxNeedsLock = false } if w.gpu == nil && !w.nocontext { gpu, err := gpu.New(w.ctx.API()) if err != nil { - w.ctx.Unlock() + w.unlockContext() w.destroyGPU() return err } @@ -180,7 +187,7 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops, } if w.gpu != nil { if err := w.frame(frame, size); err != nil { - w.ctx.Unlock() + w.unlockContext() if errors.Is(err, errOutOfDate) { // GPU surface needs refreshing. sync = true @@ -200,7 +207,6 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops, var err error if w.gpu != nil { err = w.ctx.Present() - w.ctx.Unlock() } return err } @@ -503,16 +509,26 @@ func (c *callbacks) ActionAt(p f32.Point) (system.Action, bool) { return c.w.queue.ActionAt(p) } +func (w *Window) unlockContext() { + if w.ctx == nil || w.ctxNeedsLock { + return + } + w.ctx.Unlock() + w.ctxNeedsLock = true +} + func (w *Window) destroyGPU() { if w.gpu != nil { - w.ctx.Lock() - w.gpu.Release() - w.ctx.Unlock() + if err := w.ctx.Lock(); err == nil { + w.gpu.Release() + w.ctx.Unlock() + } w.gpu = nil } if w.ctx != nil { w.ctx.Release() w.ctx = nil + w.ctxNeedsLock = false } } @@ -655,10 +671,12 @@ func (w *Window) processEvent(e event.Event) bool { w.coalesced.destroy = &e2 case ViewEvent: if !e2.Valid() && w.gpu != nil { - w.ctx.Lock() - w.gpu.Release() + if err := w.ctx.Lock(); err == nil { + w.gpu.Release() + w.ctx.Unlock() + } w.gpu = nil - w.ctx.Unlock() + w.ctxNeedsLock = true } w.coalesced.view = &e2 case ConfigEvent: diff --git a/app/window_test.go b/app/window_test.go new file mode 100644 index 00000000..a3838ed2 --- /dev/null +++ b/app/window_test.go @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: Unlicense OR MIT + +package app + +import ( + "image" + "image/color" + "testing" + + "gioui.org/gpu" + "gioui.org/op" +) + +func TestValidateAndProcessRelocksAfterRefresh(t *testing.T) { + ctx := &testContext{} + w := &Window{ + ctx: ctx, + gpu: &testGPU{}, + ctxNeedsLock: false, + } + + if err := w.validateAndProcess(image.Pt(320, 240), true, new(op.Ops), nil); err != nil { + t.Fatalf("validateAndProcess returned error: %v", err) + } + + want := []string{"refresh", "lock", "render-target", "present"} + if got := ctx.ops; !equalStringSlices(got, want) { + t.Fatalf("unexpected call order:\n got %v\n want %v", got, want) + } + if w.ctxNeedsLock { + t.Fatalf("context should remain current after a successful frame") + } +} + +func TestValidateAndProcessSkipsRedundantRelock(t *testing.T) { + ctx := &testContext{} + w := &Window{ + ctx: ctx, + gpu: &testGPU{}, + ctxNeedsLock: false, + } + + if err := w.validateAndProcess(image.Pt(320, 240), false, new(op.Ops), nil); err != nil { + t.Fatalf("validateAndProcess returned error: %v", err) + } + + want := []string{"render-target", "present"} + if got := ctx.ops; !equalStringSlices(got, want) { + t.Fatalf("unexpected call order:\n got %v\n want %v", got, want) + } +} + +type testContext struct { + ops []string +} + +func (c *testContext) API() gpu.API { + return nil +} + +func (c *testContext) RenderTarget() (gpu.RenderTarget, error) { + c.ops = append(c.ops, "render-target") + return gpu.OpenGLRenderTarget{}, nil +} + +func (c *testContext) Present() error { + c.ops = append(c.ops, "present") + return nil +} + +func (c *testContext) Refresh() error { + c.ops = append(c.ops, "refresh") + return nil +} + +func (c *testContext) Release() {} + +func (c *testContext) Lock() error { + c.ops = append(c.ops, "lock") + return nil +} + +func (c *testContext) Unlock() { + c.ops = append(c.ops, "unlock") +} + +type testGPU struct{} + +func (g *testGPU) Release() {} + +func (g *testGPU) Clear(color.NRGBA) {} + +func (g *testGPU) Frame(*op.Ops, gpu.RenderTarget, image.Point) error { + return nil +} + +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +}