~eliasnaur/gio#550: 
Overhaul of event routing

This issue tracks my proposal for an overhaul in Gio's event handling system. The work is a result of quite a long time thinking about the fundamental issues in Gio, and they crystalized during discussions with Dominik and Chris about their issues.

The changes are tracked in the event-filters branch. You should be able to use them by issuing the command

$ go get gioui.org@event-filters

Below is a description of the major changes.

#Commands

Operations with side-effects have been replaced with commands. Commands are executed with gtx.Execute and take effect after the latest received input event.

For example, instead of FocusOp there is now a FocusCmd that can be used like so:

gtx.Execute(key.FocusCmd{Tag: &widget})

The advantage of commands is that ops are now idempotent and as such can be freely manipulated in macros without fear of losing effects. Another advantage is that commands (optimistically) take effect immediately instead of after frame end, resulting in lower latency.

#Filters

The type-specific InputOps have been merged into a generic event.Op and replaced with filters. Filters are specified when calling gtx.Event. Example:

e, ok := gtx.Event(key.Filter{name: "A"}, key.FocusFilter{Target: &widget})

The advantage of filters is that widgets can now use a single tag, by convention its own address, for multiple event types, because filters determine which matched events are delivered at each call to gtx.Event. This also allows gtx.Focused for querying the focused state of a tag.

Another advantage is that filters don't have to be determined at layout which may not be near the code that updates the widget state.

#Fine-grained event delivery

Events are now delivered one at a time, like the good old days of Gio. This results in a few more lines for looping through events, but allow the precise mixing of events and commands. In particular, it's now possible to have two key press+release of, say, the spacebar hit two different handlers in the same frame.

#Key event delivery

Key events are now delivered to the first caller of gtx.Event whose filter(s) match. This leads to simpler routing and lower latency; for example, a newly added handler may immediately receive key events without going through a frame first.

#Feedback

I invite you to test and review the changes. I'm especially interested in design flaws, missing special-cases, but I would also appreciate hearing about bad naming, outdated documentation or typos. With such a big change I almost certainly made mistakes.

Status
RESOLVED CLOSED
Submitter
~eliasnaur
Assigned to
No-one
Submitted
4 months ago
Updated
a month ago
Labels
No labels applied.

~beikege 4 months ago

Windows 11
go 1.21.4
go get gioui.org@event-filters

https://go.dev/play/p/vccxZ9U8B_-

layout.List scrolling not working

~gedw99 4 months ago

This is wonderful work and just wanted to say thanks.

Will my code need refactoring ? I assume yes.

I guess eventually the examples will be updated to use this if the proposal passes. Happy to help with it . Is there an example so I get a feel for it ?

~egonelbre 4 months ago

Here is some of the refactoring you'll need:

gofmt -w -r "system.DestroyEvent -> app.DestroyEvent" .
gofmt -w -r "system.FrameEvent -> app.FrameEvent" .
gofmt -w -r "layout.NewContext -> app.NewContext" .
gofmt -w -r "op.InvalidateOp{}.Add(gtx.Ops) -> gtx.Execute(op.InvalidateCmd{})" .
goimports -w .

Remove any code related to package "gioui.org/profile"

For gesture.drag:

for _, e := range drag.Update(gtx.Metric, gtx, gesture.Both) {

// ==>

for {
	e, ok := drag.Update(gtx.Metric, gtx.Source, gesture.Both)
	if !ok {
		break
	}

Didn't get further at the moment.

~ajstarks 4 months ago*

I updated giocanvas with the new event handling system, (many thanks for the gofmt commands from @egonelbre) and some clients work as expected (modulo the still existing bug #542). I'm not clear on the proper refactoring for clients with more extensive event handling. For example, is the best way to refactor:

case app.FrameEvent:
			canvas := giocanvas.NewCanvas(float32(e.Size.X), float32(e.Size.Y), app.FrameEvent{})
			key.InputOp{Tag: pressed}.Add(canvas.Context.Ops)
			pointer.InputOp{Tag: pressed, Grab: false, Kinds: pointer.Press | pointer.Move}.Add(canvas.Context.Ops)

This:

		case app.FrameEvent:
			canvas := giocanvas.NewCanvas(float32(e.Size.X), float32(e.Size.Y), app.FrameEvent{})
			canvas.Context.Execute(key.FocusCmd{Tag: pressed})
			canvas.Context.Event(pointer.Filter{Kinds: pointer.Press | pointer.Move})

~eliasnaur 4 months ago

~beikege: thank you, I've updated the branch to fix widget.List. Use go get gioui.org@cf3e0c744246b4ae to get around Go proxy caching of the branch.

~gedw99: yes, code will need refactoring, but mostly in low-level widget code. I'm using the kitchen example for testing:

diff --git a/kitchen/kitchen.go b/kitchen/kitchen.go
index 10d4c05..da8d2f0 100644
--- a/kitchen/kitchen.go
+++ b/kitchen/kitchen.go
@@ -22,8 +22,7 @@ import (
 	"gioui.org/font/gofont"
 	"gioui.org/gpu/headless"
 	"gioui.org/io/event"
-	"gioui.org/io/router"
-	"gioui.org/io/system"
+	"gioui.org/io/input"
 	"gioui.org/layout"
 	"gioui.org/op"
 	"gioui.org/op/clip"
@@ -96,7 +95,7 @@ func saveScreenshot(f string) error {
 			PxPerSp: scale,
 		},
 		Constraints: layout.Exact(sz),
-		Queue:       new(router.Router),
+		Source:      input.Source{},
 	}
 	th := material.NewTheme()
 	th.Shaper = text.NewShaper(text.WithCollection(gofont.Collection()))
@@ -126,7 +125,7 @@ func loop(w *app.Window) error {
 			ev := w.NextEvent()
 			events <- ev
 			<-acks
-			if _, ok := ev.(system.DestroyEvent); ok {
+			if _, ok := ev.(app.DestroyEvent); ok {
 				return
 			}
 		}
@@ -137,11 +136,11 @@ func loop(w *app.Window) error {
 		select {
 		case e := <-events:
 			switch e := e.(type) {
-			case system.DestroyEvent:
+			case app.DestroyEvent:
 				acks <- struct{}{}
 				return e.Err
-			case system.FrameEvent:
-				gtx := layout.NewContext(&ops, e)
+			case app.FrameEvent:
+				gtx := app.NewContext(&ops, e)
 				if *disable {
 					gtx = gtx.Disabled()
 				}
@@ -170,7 +169,7 @@ func transformedKitchen(gtx layout.Context, th *material.Theme) layout.Dimension
 	if !transformTime.IsZero() {
 		dt := float32(gtx.Now.Sub(transformTime).Seconds())
 		angle := dt * .1
-		op.InvalidateOp{}.Add(gtx.Ops)
+		gtx.Execute(op.InvalidateCmd{})
 		tr := f32.Affine2D{}
 		tr = tr.Rotate(f32.Pt(300, 20), -angle)
 		scale := 1.0 - dt*.5
@@ -332,7 +331,7 @@ func kitchen(gtx layout.Context, th *material.Theme) layout.Dimensions {
 						return material.Clickable(gtx, flatBtn, func(gtx C) D {
 							return layout.UniformInset(unit.Dp(12)).Layout(gtx, func(gtx C) D {
 								flatBtnText := material.Body1(th, "Flat")
-								if gtx.Queue == nil {
+								if !gtx.Enabled() {
 									flatBtnText.Color.A = 150
 								}
 								return layout.Center.Layout(gtx, flatBtnText.Layout)

~ajstarks: I believe this is what you want:

case app.FrameEvent:
    canvas := giocanvas.NewCanvas(float32(e.Size.X), float32(e.Size.Y), app.FrameEvent{})
    event.Op{Tag: pressed}.Add(canvas.Context.Ops)

The pointer.InputOp kinds are replaced with filters at your event handling:

for {
   e, ok := gtx.Event(pointer.Filter{Target: pressed, Kinds: pointer.Press | pointer.Move})
   if !ok { break }
}

~beikege 4 months ago

go get gioui.org@cf3e0c744246b4ae

Works perfectly, thank you.

~ajstarks 4 months ago

Previously I did this:

var pressed bool
for {
    ev := w.NextEvent() 
    switch e := ev.(type) {
    case app.FrameEvent:
       ....
        kbpointer(e.Queue) // handle key and pointer events
        e.Frame(....)
  }
}

// handle kb and pointer
func kbpointer(q event.Queue) {
    for _, ev := range q.Events(pressed) { 
         if k, ok := ev.(key.Event); ok { ... } // handle kb
         if p, ok := ev.(pointer.Event); ok { ...} // handle pointer
    }
}

What is the best re-factor?

~eliasnaur 4 months ago*

Try (untested):

kbpointer(e.Source)
...
// handle kb and pointer
func kbpointer(q input.Source) {
    for {
         e, ok := q.Event(
             key.Filter{},
             pointer.Filter{Target: pressed, Kinds: pointer.Press | pointer.Move | pointer.Release},
         )
         if !ok { break }
         switch e := e.(type) {
         case key.Event:
               // handle kb
         case pointer.Event:
              // handle pointer
         }
    }
}

~beikege 4 months ago*

~eliasnaur

https://go.dev/play/p/HFJWhntO9Ua

go get gioui.org@cf3e0c744246b4ae

Android Scrolling the list causes infinite frame refreshes.

Use adb logcat to view logs.


go get gioui.org@86fe42a4372c215b

Works perfectly, thank you.

~ajstarks 4 months ago

@eliasnaur: thanks for the pointers. I have it mostly working now with a slight correction:

kbpointer(e.Source)
...
// handle kb and pointer
func kbpointer(q input.Source) {
    for {
         e, ok := q.Event( // q.Event, not e.Event
             key.Filter{},
             pointer.Filter{Target: pressed, Kinds: pointer.Press | pointer.Move | pointer.Release},
         )
         if !ok { break }
         switch e := e.(type) {
         case key.Event:
               // handle kb
         case pointer.Event:
              // handle pointer
         }
    }
}

One issue: I cannot turning on modifiers for the key events:

key.Filter{Required: key.ModCtrl}

disables all keyboard events.

~egonelbre 4 months ago

Currently I'm unable to figure out how to set cursor for different regions without causing redraws. https://github.com/egonelbre/expgio/blob/event-filters/cursors/main.go#L100.

~eliasnaur 4 months ago

~ajstarks: use the Optional field of key.Filter instead of Required.

~ajstarks 4 months ago

~eliasnaur, the use of Optional works fine. thanks. Giocanvas and its clients are now converted. Overall the new API seems at bit cleaner and more straightforward.

New: https://gist.github.com/ajstarks/ef633da557c4f90429990d66f0f6c7c6

Old: https://gist.github.com/ajstarks/9224ad09306ecb240c6618bf32da45e6

Diff:

2c2
< func kbpointer(q event.Queue, cfg config) {
---
> func kbpointer(q input.Source, cfg config) {
6,9c6,18
< 	for _, ev := range q.Events(pressed) {
< 		// keyboard events
< 		if k, ok := ev.(key.Event); ok {
< 			switch k.State {
---
> 	for {
> 		e, ok := q.Event(
> 			key.Filter{Optional: key.ModCtrl},
> 			pointer.Filter{Kinds: pointer.Press | pointer.Move | pointer.Release},
> 		)
> 		if !ok {
> 			break
> 		}
> 		switch e := e.(type) {
> 
> 		case key.Event: // keyboard events
> 
> 			switch e.State {
11c20
< 				switch k.Name {
---
> 				switch e.Name {
31c40
< 					switch k.Modifiers {
---
> 					switch e.Modifiers {
38c47
< 					switch k.Modifiers {
---
> 					switch e.Modifiers {
45c54
< 					switch k.Modifiers {
---
> 					switch e.Modifiers {
52c61
< 					switch k.Modifiers {
---
> 					switch e.Modifiers {
62,65c71,74
< 		}
< 		// pointer events
< 		if p, ok := ev.(pointer.Event); ok {
< 			switch p.Kind {
---
> 
> 		case pointer.Event: // pointer events
> 
> 			switch e.Kind {
67c76
< 				mouseX, mouseY = pctcoord(p.Position.X, p.Position.Y, width, height)
---
> 				mouseX, mouseY = pctcoord(e.Position.X, e.Position.Y, width, height)
69c78
< 				switch p.Buttons {
---
> 				switch e.Buttons {
71c80
< 					bx, by = pctcoord(p.Position.X, p.Position.Y, width, height)
---
> 					bx, by = pctcoord(e.Position.X, e.Position.Y, width, height)
73c82
< 					ex, ey = pctcoord(p.Position.X, p.Position.Y, width, height)
---
> 					ex, ey = pctcoord(e.Position.X, e.Position.Y, width, height)
77d85
< 				pressed = true
96c104
< 		case system.DestroyEvent:
---
> 		case app.DestroyEvent:
100,103c108,109
< 		case system.FrameEvent:
< 			canvas := giocanvas.NewCanvas(float32(e.Size.X), float32(e.Size.Y), system.FrameEvent{})
< 			key.InputOp{Tag: pressed}.Add(canvas.Context.Ops)
< 			pointer.InputOp{Tag: pressed, Grab: false, Kinds: pointer.Press | pointer.Move}.Add(canvas.Context.Ops)
---
> 		case app.FrameEvent:
> 			canvas := giocanvas.NewCanvas(float32(e.Size.X), float32(e.Size.Y), app.FrameEvent{})
143c149
< 			kbpointer(e.Queue, cfg)
---
> 			kbpointer(e.Source, cfg)

also in giocanvas abs.go, changed

op.InvalidateOp{}.Add(ops)

to

c.Context.Execute(op.InvalidateCmd{})

~eliasnaur 4 months ago*

~beikege: another good catch, thank you. ~egonelbre: thanks, fixed.

go get gioui.org@86fe42a4372c215b should fix both issues.

~gedw99 4 months ago

thanks for all the tips. Great community !!

~eliasnaur 4 months ago

What do you think of the names introduced? In particular, Source.Event? If it's all good, I shall rename app.Window.NextEvent to match.

~beikege 4 months ago

The event exceeds the clipping area.

go get gioui.org@86fe42a4372c215b

https://go.dev/play/p/GweMCrv5A9x

~jeffwilliams 4 months ago

I believe the documentation for app.NewContext needs updating. It mentions:

    NewContext is shorthand for

        layout.Context{
          Ops: ops,
          Now: e.Now,
          Queue: e.Queue,
          Config: e.Config,
          Constraints: layout.Exact(e.Size),
        }

However FrameEvent no longer has some of the fields mentioned:

type FrameEvent struct {
	// Now is the current animation. Use Now instead of time.Now to
	// synchronize animation and to avoid the time.Now call overhead.
	Now time.Time
	// Metric converts device independent dp and sp to device pixels.
	Metric unit.Metric
	// Size is the dimensions of the window.
	Size image.Point
	// Insets represent the space occupied by system decorations and controls.
	Insets Insets
	// Frame completes the FrameEvent by drawing the graphical operations
	// from ops into the window.
	Frame func(frame *op.Ops)
	// Source is the interface between the window and widgets.
	Source input.Source
}

~jeffwilliams 4 months ago

I'm trying to think of how I can refactor some code to work with the new event delivery model, and I wonder if someone might have a suggestion. Previously, I had code like the following:

func loop(w *app.Window) error {
...
	for {
		select {
		case e := <-w.Events():
			err := handleEvent(e)
			if err != nil {
				return err
			}
		case w := <-editor.WorkChan():
			w.Service()
		}
	}
	

This has been slightly simplified to demonstrate the main problem.

The idea is that I have a select statement that will either handle UI events (the w.Events() case), or perform a unit of work that was requested by a different goroutine (the editor.WorkChan() case). The units of work usually modify the same datastructures and state that the handlers for the UI events also modify.

With the new model there is no channel to select for events and the function that returns the next event is blocking, which means that if the program blocked on Window.NextEvent() then editor.WorkChan() cannot be serviced. Can I call Window.NextEvent() from a separate goroutine? In which case I could do something like:

	events := make(chan event.Event)
	go func() {
		for {
			events <- w.NextEvent()
		}
	}()

	for {
		select {
		case e := <-events:
			err := handleEvent(e)
			if err != nil {
				return err
			}
		case w := <-editor.WorkChan():
			w.Service()
		}
	}

Assuming I cannot, is there a way that my program can inject custom events so that w.NextEvent() would return them, or some other method? I'd like to avoid having some sort of global mutex for the program datastructures.

~eliasnaur 4 months ago

~jeffwilliams: updated documentation, thanks. ~whereswaldon covers the options for refactoring channel-based events loops in his latest newsletter[0]. ~beikege: another great find, thanks (the issue was the unexpected nil pointer.Filter.Target). Fixed and added test.

Latest version: go get gioui.org@7392d2ff

[0] https://gioui.org/news/2023-11#api-change-window-event-iteration

~beikege 4 months ago

~eliasnaur

go get gioui.org@7392d2ff

Unable to receive any pointer events.

https://go.dev/play/p/GweMCrv5A9x

~eliasnaur 4 months ago

Yes, you need to fill out the pointer.Filter.Target field to receive events:

pointer.Filter{Target: l, Kinds: pointer.Press | pointer.Drag | pointer.Release | pointer.Cancel},

Before my fix, nil targets would work by accident.

~beikege 4 months ago

Thanks, works perfectly.

~jeffwilliams 4 months ago

~eliasnaur: Thanks, I think the last method in the newsletter should work. I found one more minor documentation bug. The documentation for io/clipboard.ReadCmd reads "ReadCmd requests the text of the clipboard, delivered to the handler through an op/transfer.DataEvent", but I think it should instead be io/transfer.DataEvent if I am not mistaken.

~eliasnaur 4 months ago

~jeffwilliams: thanks, fixed in ab776c65.

~eliasnaur referenced this from #553 4 months ago

~andybalholm referenced this from #553 4 months ago

~eliasnaur referenced this from #555 3 months ago

~whereswaldon 2 months ago

In the conversion of gio-x to this API, I encountered what I think is a bug. The follow program defines a whole-window area that listens for clicks and key events. If it is clicked, it tries to transfer keyboard focus to itself. However, keyboard focus transfer never occurs. Either I've misunderstood the API, or there's something swallowing the focus transfer:

// SPDX-License-Identifier: Unlicense OR MIT

package main

// A simple Gio program. See https://gioui.org for more information.

import (
	"fmt"
	"log"
	"os"

	"gioui.org/app"
	"gioui.org/io/event"
	"gioui.org/io/key"
	"gioui.org/io/pointer"
	"gioui.org/op"
)

func main() {
	go func() {
		w := app.NewWindow()
		if err := loop(w); err != nil {
			log.Fatal(err)
		}
		os.Exit(0)
	}()
	app.Main()
}

func loop(w *app.Window) error {
	tag := new(int)
	var ops op.Ops
	for {
		switch e := w.NextEvent().(type) {
		case app.DestroyEvent:
			return e.Err
		case app.FrameEvent:
			gtx := app.NewContext(&ops, e)
			for {
				ev, ok := gtx.Source.Event(pointer.Filter{
					Target: tag,
					Kinds:  pointer.Release,
				})
				if !ok {
					break
				}
				switch ev := ev.(type) {
				case pointer.Event:
					if ev.Kind == pointer.Release {
						gtx.Execute(key.FocusCmd{Tag: tag})
						fmt.Println("triggered focus command")
					}
				}
				fmt.Printf("%#+v\n", ev)
			}
			for {
				ev, ok := gtx.Source.Event(key.Filter{
					Focus: tag,
				})
				if !ok {
					break
				}
				fmt.Printf("%#+v\n", ev)
			}
			event.Op(gtx.Ops, tag)
			e.Frame(gtx.Ops)
		}
	}
}

~eliasnaur 2 months ago*

The issue is caused by an unfortunate combination:

  • You don't include key.FocusFilter for matching key.FocusEvents, which means your tag won't be considered a valid focus target.
  • key.Filter.Focus is for matching key events only if the specified tag is focused (e.g. an Editor is not interested in arrow keys when not focused).

I'm not sure what the right fix is. It's tempting to say that tags mentioned in key.Filter.Focus are also focusable, but that seems too subtle.

~whereswaldon 2 months ago

Thanks for the clarification; I had missed that requesting focusability was a separate filter type. I'm not sure if this really needs a fix. I was just holding it wrong.

~whereswaldon 2 months ago

Now that input.Router.Queue does not return a boolean indicating whether the event has a handler, how should custom rendering applications detect that Gio isn't handling an event? I'm trying to convert the glfw and opengl examples, both of which need this. I could register some kind of catch-all event handler to detect it, but I'm not sure that's the proper approach.

~whereswaldon 2 months ago

gioui.org/x and gioui.org/example now also have event-filters branches that are updated to be compatible with core's event-filters branch. My only API concern is the one I mention above about custom rendering event processing.

~eliasnaur 2 months ago

~whereswaldon: For app.Window I replaced the bool result with a call to Router.WakeupTime() that aggregates all the reasons for generating a new frame. Would that work for glfw and opengl?

Note: I've force-pushed event-filters with a few documentation fixes.

~whereswaldon a month ago

That does indeed seem to work; thank you!

~whereswaldon a month ago

Now that the whole ecosystem supports it, I'm going to merge event-filters and tag a new release

~whereswaldon REPORTED CLOSED a month ago

Release is out; this issue can be closed.

Register here or Log in to comment, or comment via email.