app: avoid relocking contexts every frame

Signed-off-by: Joe Julian <me@joejulian.name>

app/gl_macos.go

@@ -111,8 +111,6 @@ func (c *glContext) Unlock() {
 }
 
 func (c *glContext) Refresh() error {
-	c.Lock()
-	defer c.Unlock()
 	C.gio_updateContext(c.ctx)
 	return nil
 }

app/os.go

@@ -165,11 +165,12 @@ type frameEvent struct {
 	Sync bool
 }
 
+// The caller must hold the context lock while using API, Refresh,
+// RenderTarget, or Present.
 type context interface {
 	API() gpu.API
 	RenderTarget() (gpu.RenderTarget, error)
 	Present() error
-	// Refresh assumes the context is already locked and current.
 	Refresh() error
 	Release()
 	Lock() error

app/window.go

@@ -154,6 +154,10 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops,
 				sync = true
 			}
 		}
+		if err := w.lockContext(); err != nil {
+			w.destroyGPU()
+			return err
+		}
 		if sync && w.ctx != nil {
 			if err := w.ctx.Refresh(); err != nil {
 				if errors.Is(err, errOutOfDate) {
@@ -167,14 +171,11 @@ func (w *Window) validateAndProcess(size image.Point, sync bool, frame *op.Ops,
 				}
 				return err
 			}
-			w.ctxNeedsLock = true
-		}
-		if w.ctx != nil && w.ctxNeedsLock {
-			if err := w.ctx.Lock(); err != nil {
+			w.unlockContext()
+			if err := w.lockContext(); err != nil {
 				w.destroyGPU()
 				return err
 			}
-			w.ctxNeedsLock = false
 		}
 		if w.gpu == nil && !w.nocontext {
 			gpu, err := gpu.New(w.ctx.API())
@@ -509,6 +510,17 @@ func (c *callbacks) ActionAt(p f32.Point) (system.Action, bool) {
 	return c.w.queue.ActionAt(p)
 }
 
+func (w *Window) lockContext() error {
+	if w.ctx == nil || !w.ctxNeedsLock {
+		return nil
+	}
+	if err := w.ctx.Lock(); err != nil {
+		return err
+	}
+	w.ctxNeedsLock = false
+	return nil
+}
+
 func (w *Window) unlockContext() {
 	if w.ctx == nil || w.ctxNeedsLock {
 		return
@@ -519,9 +531,9 @@ func (w *Window) unlockContext() {
 
 func (w *Window) destroyGPU() {
 	if w.gpu != nil {
-		if err := w.ctx.Lock(); err == nil {
+		if err := w.lockContext(); err == nil {
 			w.gpu.Release()
-			w.ctx.Unlock()
+			w.unlockContext()
 		}
 		w.gpu = nil
 	}
@@ -671,12 +683,11 @@ func (w *Window) processEvent(e event.Event) bool {
 		w.coalesced.destroy = &e2
 	case ViewEvent:
 		if !e2.Valid() && w.gpu != nil {
-			if err := w.ctx.Lock(); err == nil {
+			if err := w.lockContext(); err == nil {
 				w.gpu.Release()
-				w.ctx.Unlock()
+				w.unlockContext()
 			}
 			w.gpu = nil
-			w.ctxNeedsLock = true
 		}
 		w.coalesced.view = &e2
 	case ConfigEvent:

app/window_test.go

@@ -1,107 +0,0 @@
-// 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
-}