Compare commits

..

1 Commits

Author SHA1 Message Date
Joe Julian 9bfa6bc1c2 app: relock contexts after refresh
This fixes an Android regression introduced by

  3e601e73c4
  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 <me@joejulian.name>
2026-04-16 15:00:49 -07:00
13 changed files with 180 additions and 164 deletions
+28 -10
View File
@@ -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:
+107
View File
@@ -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
}
+1 -1
View File
@@ -106,7 +106,7 @@ func parseLoader(ld *opentype.Loader) (*fontapi.Font, giofont.Font, error) {
// Face many be invoked any number of times and is safe so long as each return value is
// only used by one goroutine.
func (f Face) Face() *fontapi.Face {
return fontapi.NewFace(f.face)
return &fontapi.Face{Font: f.face}
}
// FontFace returns a text.Font with populated font metadata for the
+12 -3
View File
@@ -61,8 +61,12 @@ func (h *Hover) Update(q input.Source) bool {
h.entered = false
}
case pointer.Enter:
h.pid = e.PointerID
h.entered = true
if !h.entered {
h.pid = e.PointerID
}
if h.pid == e.PointerID {
h.entered = true
}
}
}
return h.entered
@@ -218,7 +222,12 @@ func (c *Click) Update(q input.Source) (ClickEvent, bool) {
if e.Source == pointer.Mouse && e.Buttons != pointer.ButtonPrimary {
break
}
c.pid = e.PointerID
if !c.hovered {
c.pid = e.PointerID
}
if c.pid != e.PointerID {
break
}
c.pressed = true
if e.Time-c.clickedAt < doubleClickDuration {
c.clicks++
-72
View File
@@ -100,78 +100,6 @@ func TestMouseClicks(t *testing.T) {
}
}
func TestClickPointerIDReassignment(t *testing.T) {
// A Click must accept a Press from a PointerID that differs from the
// one its hovered state was previously associated with. Some backends
// reassign a single physical pointer's ID over its lifetime — e.g. the
// Windows pointer API across focus changes — and locking the gesture
// to the first observed ID would silently drop every subsequent press.
//
// The sequence below puts the gesture into the buggy state through
// public events alone: a press under PointerID 1 starts an active
// press cycle, a Move under PointerID 2 arrives mid-press (which the
// router routes as an Enter for PID 2 but the gesture's Enter handler
// is a no-op for pid while pressed), then PID 1 releases. After this,
// the router has the gesture entered for PID 2 (so the next event
// under PID 2 won't trigger another Enter) but the gesture itself
// still has pid=1.
var click Click
var ops op.Ops
rect := image.Rect(0, 0, 100, 100)
stack := clip.Rect(rect).Push(&ops)
click.Add(&ops)
stack.Pop()
var r input.Router
click.Update(r.Source())
r.Frame(&ops)
drain := func() {
for {
if _, ok := click.Update(r.Source()); !ok {
return
}
}
}
// Press under PointerID 1.
r.Queue(
pointer.Event{Kind: pointer.Move, Source: pointer.Mouse, Position: f32.Pt(50, 50), PointerID: 1},
pointer.Event{Kind: pointer.Press, Source: pointer.Mouse, Buttons: pointer.ButtonPrimary, Position: f32.Pt(50, 50), PointerID: 1},
)
drain()
// Move under PointerID 2 while PointerID 1 is still pressed. The
// router records the gesture as entered for PointerID 2 but the
// gesture's Enter handler is a no-op for pid because c.pressed.
r.Queue(pointer.Event{Kind: pointer.Move, Source: pointer.Mouse, Position: f32.Pt(50, 50), PointerID: 2})
drain()
// Release PointerID 1. PointerID 1's press tracking ends; the
// gesture's recorded pid stays at 1.
r.Queue(pointer.Event{Kind: pointer.Release, Source: pointer.Mouse, Position: f32.Pt(50, 50), PointerID: 1})
drain()
// Press under PointerID 2. The router won't refire Enter for PID 2
// (the gesture is already in PID 2's entered set), so the gesture's
// only chance to refresh its pid is the Press handler itself.
r.Queue(pointer.Event{Kind: pointer.Press, Source: pointer.Mouse, Buttons: pointer.ButtonPrimary, Position: f32.Pt(50, 50), PointerID: 2})
var sawPress bool
for {
ev, ok := click.Update(r.Source())
if !ok {
break
}
if ev.Kind == KindPress {
sawPress = true
}
}
if !sawPress {
t.Fatal("expected KindPress for press under reassigned PointerID; gesture dropped the press because of stale recorded pid")
}
}
func mouseClickEvents(times ...time.Duration) []event.Event {
press := pointer.Event{
Kind: pointer.Press,
+1 -1
View File
@@ -5,7 +5,7 @@ go 1.24.0
require (
eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d
gioui.org/shader v1.0.8
github.com/go-text/typesetting v0.3.4
github.com/go-text/typesetting v0.3.0
golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0
golang.org/x/exp/shiny v0.0.0-20250408133849-7e4ce0ab07d0
golang.org/x/image v0.26.0
+4 -4
View File
@@ -3,10 +3,10 @@ eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d/go.mod h1:OYVuxibdk9OSLX8v
gioui.org/cpu v0.0.0-20210808092351-bfe733dd3334/go.mod h1:A8M0Cn5o+vY5LTMlnRoK3O5kG+rH0kWfJjeKd9QpBmQ=
gioui.org/shader v1.0.8 h1:6ks0o/A+b0ne7RzEqRZK5f4Gboz2CfG+mVliciy6+qA=
gioui.org/shader v1.0.8/go.mod h1:mWdiME581d/kV7/iEhLmUgUK5iZ09XR5XpduXzbePVM=
github.com/go-text/typesetting v0.3.4 h1:YYurUOtEb9kGSOz4uE3k4OpBGsp1dDL8+fjCeaFamAU=
github.com/go-text/typesetting v0.3.4/go.mod h1:4qZCQphq4KSgGTAeI0uMEkVbROgfah8BuyF5LRYr7XY=
github.com/go-text/typesetting-utils v0.0.0-20260223113751-2d88ac90dae3 h1:drBZzMgdYPbmyXqOto4YhhJGrFIQCX94FpR4MzTCsos=
github.com/go-text/typesetting-utils v0.0.0-20260223113751-2d88ac90dae3/go.mod h1:3/62I4La/HBRX9TcTpBj4eipLiwzf+vhI+7whTc9V7o=
github.com/go-text/typesetting v0.3.0 h1:OWCgYpp8njoxSRpwrdd1bQOxdjOXDj9Rqart9ML4iF4=
github.com/go-text/typesetting v0.3.0/go.mod h1:qjZLkhRgOEYMhU9eHBr3AR4sfnGJvOXNLt8yRAySFuY=
github.com/go-text/typesetting-utils v0.0.0-20241103174707-87a29e9e6066 h1:qCuYC+94v2xrb1PoS4NIDe7DGYtLnU2wWiQe9a1B1c0=
github.com/go-text/typesetting-utils v0.0.0-20241103174707-87a29e9e6066/go.mod h1:DDxDdQEnB70R8owOx3LVpEFvpMK9eeH1o2r0yZhFI9o=
golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 h1:R84qjqJb5nVJMxqWYb3np9L5ZsaDtB+a39EqjV0JSUM=
golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0/go.mod h1:S9Xr4PYopiDyqSyp5NjCrhFrqg6A5zA2E/iPHPhqnS8=
golang.org/x/exp/shiny v0.0.0-20250408133849-7e4ce0ab07d0 h1:tMSqXTK+AQdW3LpCbfatHSRPHeW6+2WuxaVQuHftn80=
+3 -16
View File
@@ -739,10 +739,6 @@ func (q *pointerQueue) Push(handlers map[event.Tag]*handler, state pointerState,
state.pointers = nil
return state, evts
}
if e.Kind == pointer.Scroll {
// Scroll events are not bound to a pointer; see pointer.Event.PointerID.
return state, q.deliverScrollEvent(handlers, evts, e)
}
state, pidx := state.pointerOf(e)
p := state.pointers[pidx]
@@ -765,6 +761,9 @@ func (q *pointerQueue) Push(handlers map[event.Tag]*handler, state pointerState,
p.pressed = false
p, evts, state.cursor, _ = q.deliverEnterLeaveEvents(handlers, state.cursor, p, evts, e)
p, evts = q.deliverDropEvent(handlers, p, evts)
case pointer.Scroll:
p, evts, state.cursor, _ = q.deliverEnterLeaveEvents(handlers, state.cursor, p, evts, e)
evts = q.deliverEvent(handlers, p, evts, e)
default:
panic("unsupported pointer event type")
}
@@ -781,18 +780,6 @@ func (q *pointerQueue) Push(handlers map[event.Tag]*handler, state pointerState,
return state, evts
}
// deliverScrollEvent delivers scroll events to the handlers hit by the event coordinate.
func (q *pointerQueue) deliverScrollEvent(handlers map[event.Tag]*handler, evts []taggedEvent, e pointer.Event) []taggedEvent {
var hits []event.Tag
q.hitTest(e.Position, func(n *hitNode) bool {
if _, ok := handlers[n.tag]; ok {
hits = addHandler(hits, n.tag)
}
return true
})
return q.deliverEvent(handlers, pointerInfo{handlers: hits}, evts, e)
}
func (q *pointerQueue) deliverEvent(handlers map[event.Tag]*handler, p pointerInfo, evts []taggedEvent, e pointer.Event) []taggedEvent {
if p.pressed && len(p.handlers) == 1 {
e.Priority = pointer.Grabbed
-37
View File
@@ -1345,40 +1345,3 @@ func events(r *Router, n int, filters ...event.Filter) []event.Event {
}
return events
}
// TestPointerScrollDoesNotTrackPointer queues two events over two cursor
// regions. The Move puts the live pointer over the button (CursorPointer);
// the Scroll happens over the cell (CursorText) and must not update the
// cursor.
func TestPointerScrollDoesNotTrackPointer(t *testing.T) {
var ops op.Ops
button := clip.Rect(image.Rect(0, 0, 50, 50)).Push(&ops)
pointer.CursorPointer.Add(&ops)
button.Pop()
cell := clip.Rect(image.Rect(100, 0, 200, 50)).Push(&ops)
pointer.CursorText.Add(&ops)
cell.Pop()
var r Router
r.Frame(&ops)
r.Queue(
pointer.Event{
Kind: pointer.Move,
Source: pointer.Mouse,
Position: f32.Pt(25, 25),
},
pointer.Event{
Kind: pointer.Scroll,
Source: pointer.Mouse,
Position: f32.Pt(150, 25),
Scroll: f32.Pt(0, 1),
},
)
if got, want := r.Cursor(), pointer.CursorPointer; got != want {
t.Errorf("got %q, want %q (scroll position must not update the cursor; "+
"the live pointer's last position is what determines it)", got, want)
}
}
+1 -3
View File
@@ -19,9 +19,7 @@ type Event struct {
Source Source
// PointerID is the id for the pointer and can be used
// to track a particular pointer from Press to
// Release. Populated for Press, Release, Move, Drag,
// Enter, Leave, and Cancel; Scroll events are not
// bound to a tracked pointer and leave it zero.
// Release.
PointerID ID
// Priority is the priority of the receiving handler
// for this event.
+14 -12
View File
@@ -103,7 +103,8 @@ func (l *line) insertTrailingSyntheticNewline(newLineClusterIdx int) {
clusterIndex: newLineClusterIdx,
glyphCount: 0,
runeCount: 1,
advance: 0,
xAdvance: 0,
yAdvance: 0,
xOffset: 0,
yOffset: 0,
}
@@ -159,9 +160,9 @@ type glyph struct {
// runeCount is the quantity of runes in the source text that this glyph
// corresponds to.
runeCount int
// advance is the distance the dot moves when laying out the glyph along
// the run's primary axis.
advance fixed.Int26_6
// xAdvance and yAdvance describe the distance the dot moves when
// laying out the glyph on the X or Y axis.
xAdvance, yAdvance fixed.Int26_6
// xOffset and yOffset describe offsets from the dot that should be
// applied when rendering the glyph.
xOffset, yOffset fixed.Int26_6
@@ -269,9 +270,8 @@ func newShaperImpl(systemFonts bool, collection []FontFace) *shaperImpl {
// in the order in which they are loaded, with the first face being the default.
func (s *shaperImpl) Load(f FontFace) {
desc := opentype.FontToDescription(f.Font)
face := f.Face.Face()
s.fontMap.AddFace(face, fontscan.Location{File: fmt.Sprint(desc)}, desc)
s.addFace(face, f.Font)
s.fontMap.AddFace(f.Face.Face(), fontscan.Location{File: fmt.Sprint(desc)}, desc)
s.addFace(f.Face.Face(), f.Font)
}
func (s *shaperImpl) addFace(f *font.Face, md giofont.Font) {
@@ -437,7 +437,8 @@ func (s *shaperImpl) shapeText(ppem fixed.Int26_6, lc system.Locale, txt []rune)
Height: input.Size,
XBearing: 0,
YBearing: 0,
Advance: input.Size,
XAdvance: input.Size,
YAdvance: input.Size,
XOffset: 0,
YOffset: 0,
ClusterIndex: input.RunStart,
@@ -853,10 +854,11 @@ func toGioGlyphs(in []shaping.Glyph, ppem fixed.Int26_6, faceIdx int) []glyph {
bounds.Max = bounds.Min.Add(fixed.Point26_6{X: g.Width, Y: -g.Height})
out = append(out, glyph{
id: newGlyphID(ppem, faceIdx, g.GlyphID),
clusterIndex: g.TextIndex(),
runeCount: g.RunesCount(),
glyphCount: g.GlyphsCount(),
advance: g.Advance,
clusterIndex: g.ClusterIndex,
runeCount: g.RuneCount,
glyphCount: g.GlyphCount,
xAdvance: g.XAdvance,
yAdvance: g.YAdvance,
xOffset: g.XOffset,
yOffset: g.YOffset,
bounds: bounds,
+7 -3
View File
@@ -259,6 +259,10 @@ func WithCollection(collection []FontFace) ShaperOption {
}
// NewShaper constructs a shaper with the provided options.
//
// NewShaper must be called after [app.NewWindow], unless the [NoSystemFonts]
// option is specified. This is an unfortunate restriction caused by some platforms
// such as Android.
func NewShaper(options ...ShaperOption) *Shaper {
l := &Shaper{}
for _, opt := range options {
@@ -464,7 +468,7 @@ func (l *Shaper) NextGlyph() (_ Glyph, ok bool) {
if rtl {
// Modify the advance prior to computing runOffset to ensure that the
// current glyph's width is subtracted in RTL.
l.advance += g.advance
l.advance += g.xAdvance
}
// runOffset computes how far into the run the dot should be positioned.
runOffset := l.advance
@@ -477,7 +481,7 @@ func (l *Shaper) NextGlyph() (_ Glyph, ok bool) {
Y: int32(line.yOffset),
Ascent: line.ascent,
Descent: line.descent,
Advance: g.advance,
Advance: g.xAdvance,
Runes: uint16(g.runeCount),
Offset: fixed.Point26_6{
X: g.xOffset,
@@ -490,7 +494,7 @@ func (l *Shaper) NextGlyph() (_ Glyph, ok bool) {
}
l.glyph++
if !rtl {
l.advance += g.advance
l.advance += g.xAdvance
}
endOfRun := l.glyph == len(run.Glyphs)
+2 -2
View File
@@ -450,8 +450,8 @@ func printLinePositioning(t *testing.T, lines []line, glyphs []Glyph) {
for g := start; ; g += inc {
glyph := run.Glyphs[g]
if glyphCursor < len(glyphs) {
t.Logf("glyph %2d, adv %3d, runes %2d, glyphs %d - glyphs[%2d] flags %s", g, glyph.advance, glyph.runeCount, glyph.glyphCount, glyphCursor, glyphs[glyphCursor].Flags)
t.Logf("glyph %2d, adv %3d, runes %2d, glyphs %d - n/a", g, glyph.advance, glyph.runeCount, glyph.glyphCount)
t.Logf("glyph %2d, adv %3d, runes %2d, glyphs %d - glyphs[%2d] flags %s", g, glyph.xAdvance, glyph.runeCount, glyph.glyphCount, glyphCursor, glyphs[glyphCursor].Flags)
t.Logf("glyph %2d, adv %3d, runes %2d, glyphs %d - n/a", g, glyph.xAdvance, glyph.runeCount, glyph.glyphCount)
}
glyphCursor++
if g == end {