From f8029f218a7591878a49073ef7e0383ebd130640 Mon Sep 17 00:00:00 2001 From: Chris Waldon Date: Wed, 19 Jun 2024 15:44:43 -0400 Subject: [PATCH] io/input: ensure full event delivery per input event 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 --- io/input/router.go | 50 +++++----- io/input/router_regression_test.go | 152 +++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 23 deletions(-) create mode 100644 io/input/router_regression_test.go diff --git a/io/input/router.go b/io/input/router.go index 9e07e9a6..b2c6efc3 100644 --- a/io/input/router.go +++ b/io/input/router.go @@ -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 @@ -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 } } } @@ -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:]...) } diff --git a/io/input/router_regression_test.go b/io/input/router_regression_test.go new file mode 100644 index 00000000..2c197eac --- /dev/null +++ b/io/input/router_regression_test.go @@ -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}) + }) +}