Skip to content

Commit

Permalink
io/input: ensure full event delivery per input event
Browse files Browse the repository at this point in the history
This commit tries to ensure that if any Gio event resulting from a given user input
event is delivered, the rest of the Gio events from the same user input are delivered
on the same frame.

The alternative could lead to rare cases in which Gio events were entirely dropped
because a subset of the resulting Gio events were delivered before a widget issued
a command.

Fixes: https://todo.sr.ht/~eliasnaur/gio/594
Signed-off-by: Chris Waldon <[email protected]>
  • Loading branch information
whereswaldon authored and builds.sr.ht committed Jun 20, 2024
1 parent 38fca9a commit f8029f2
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 23 deletions.
50 changes: 27 additions & 23 deletions io/input/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,10 @@ type taggedFilter struct {
// resulting from an incoming event.
type stateChange struct {
// event, if set, is the trigger for the change.
event event.Event
state inputState
events []taggedEvent
event event.Event
state inputState
events []taggedEvent
partiallyDelivered bool
}

// inputState represent a immutable snapshot of the state required
Expand Down Expand Up @@ -274,28 +275,30 @@ func (q *Router) Event(filters ...event.Filter) (event.Event, bool) {
}
}
}
if !q.deferring {
for i := range q.changes {
change := &q.changes[i]
for j, evt := range change.events {
match := false
switch e := evt.event.(type) {
case key.Event:
match = q.key.scratchFilter.Matches(change.state.keyState.focus, e, false)
default:
for _, tf := range q.scratchFilters {
if evt.tag == tf.tag && tf.filter.Matches(evt.event) {
match = true
break
}
for i := range q.changes {
change := &q.changes[i]
if q.deferring && !change.partiallyDelivered {
continue
}
for j, evt := range change.events {
match := false
switch e := evt.event.(type) {
case key.Event:
match = q.key.scratchFilter.Matches(change.state.keyState.focus, e, false)
default:
for _, tf := range q.scratchFilters {
if evt.tag == tf.tag && tf.filter.Matches(evt.event) {
match = true
break
}
}
if match {
change.events = append(change.events[:j], change.events[j+1:]...)
// Fast forward state to last matched.
q.collapseState(i)
return evt.event, true
}
}
if match {
change.events = append(change.events[:j], change.events[j+1:]...)
change.partiallyDelivered = true
// Fast forward state to last matched.
q.collapseState(i)
return evt.event, true
}
}
}
Expand All @@ -315,6 +318,7 @@ func (q *Router) collapseState(idx int) {
first.state = q.changes[idx].state
for i := 1; i <= idx; i++ {
first.events = append(first.events, q.changes[i].events...)
first.partiallyDelivered = first.partiallyDelivered || q.changes[i].partiallyDelivered
}
q.changes = append(q.changes[:1], q.changes[idx+1:]...)
}
Expand Down
152 changes: 152 additions & 0 deletions io/input/router_regression_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package input

import (
"image"
"image/color"
"testing"

"gioui.org/f32"
"gioui.org/io/event"
"gioui.org/io/key"
"gioui.org/io/pointer"
"gioui.org/op"
"gioui.org/op/clip"
"gioui.org/op/paint"
)

// TestRouterEventPartialDelivery ensures that the router delivers all events
// associated with a given user input event in the same frame, even if delivery
// of those events is interrupted by a command.
//
// In particular, this test builds a UI with keyboard focus defaulting elsewhere
// (which gives us an easy command to use to defer event delivery) and a
// button beneath an overlay that both want pointer press events. The test clicks
// on the button/overlay, generating press events for both, and then checks that
// the overlay actually receives a press (even though the button is laid out first
// and issues a command that defers event delivery).
func TestRouterEventPartialDelivery(t *testing.T) {
var ui ui
router := new(Router)
source := router.Source()

nextFrame := func() {
router.Frame(&ui.ops)
ui.ops.Reset()
}
ui.layout(&ui.ops, source)
nextFrame()
router.Queue(
// Click on the overlay. This ought to activate it.
pointer.Event{
Kind: pointer.Press,
Buttons: pointer.ButtonPrimary,
Position: f32.Point{X: 50, Y: 50},
},
)
ui.layout(&ui.ops, source)
if !ui.o {
t.Error("overlay failed to activate on the first click")
}
}

// btn implements a crude button which requests focus when clicked.
// It does not *use* the keyboard focus for anything, but this focus
// requesting behavior is necessary to trigger this bug.
type btn int

func (b *btn) layout(ops *op.Ops, source Source, c color.NRGBA) {
for {
ev, ok := source.Event(
pointer.Filter{
Target: b,
Kinds: pointer.Press | pointer.Release,
},
key.FocusFilter{
Target: b,
},
)
if !ok {
break
}
switch ev := ev.(type) {
case pointer.Event:
if ev.Kind == pointer.Press {
source.Execute(key.FocusCmd{
Tag: b,
})
}
}
}
defer clip.Rect{Max: image.Pt(100, 100)}.Push(ops).Pop()
event.Op(ops, b)
paint.Fill(ops, c)
}

// overlay displays a toggleable color on top of its child widget.
type overlay bool

func (o *overlay) layout(ops *op.Ops, source Source, c color.NRGBA, child func(*op.Ops, Source)) {
// We must update the state of the child widget *before* doing our own
// update, otherwise we can't reproduce the bug.
mac := op.Record(ops)
child(ops, source)
call := mac.Stop()
for {
ev, ok := source.Event(
pointer.Filter{
Target: o,
Kinds: pointer.Press | pointer.Release,
},
)
if !ok {
break
}
switch ev := ev.(type) {
case pointer.Event:
if ev.Kind == pointer.Press {
*o = !*o
}
}
}
defer clip.Rect{Max: image.Pt(100, 100)}.Push(ops).Pop()
event.Op(ops, o)
call.Add(ops)
if *o {
paint.Fill(ops, c)
}
}

// focusable steals focus the first time it is laid out.
type focusable bool

func (f *focusable) layout(ops *op.Ops, source Source) {
for {
_, ok := source.Event(key.FocusFilter{Target: f})
if !ok {
break
}
}
event.Op(ops, f)
if !*f {
source.Execute(key.FocusCmd{Tag: f})
*f = true
}
}

// UI bundles the state of this reproducer together to make it easier to use in both an
// interactive program and a test case.
type ui struct {
ops op.Ops
b btn
o overlay
f focusable
}

func (ui *ui) layout(ops *op.Ops, source Source) {
// Lay out a focusable to grab the keyboard focus.
ui.f.layout(ops, source)
// Lay out an invisible overlay atop a button.
ui.o.layout(ops, source, color.NRGBA{R: 255, A: 255}, func(ops *op.Ops, source Source) {
ui.b.layout(ops, source, color.NRGBA{G: 255, A: 100})
})
}

0 comments on commit f8029f2

Please sign in to comment.