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}) + }) +}