io,layout: switch Source from struct to interface

[inkeliz]

I'm migrating from version 0.1 to 0.6, and I'm also migrating gio-plugins and other packages. However, the lack of interfaces makes it harder to intercept and add custom events into the layout.Context.

Background: In previous versions, Queue was an interface. This allowed for custom implementations, a feature that gio-plugins relies on. It was possible (even if not explicitly intended) to use:

gtx.Events(tag) // returning custom event

gioexplorer.OpenFileOp{Tag: p.tag, Mimetype: FileTypes}.Add(gtx.Ops) // add custom event to Ops

That Op was captured by a custom implementation of Queue. The same implementation was responsible for injecting another custom event, such as gioexplorer.OpenFileEvent.

---

Currently, in version 0.6, it's not possible to achieve the same results. It is impossible to gtx.Execute(gioexplorer.OpenFile{}) and then catch a transfer.DataEvent{}.

Alternatives and Failed Attempts:

1. go:linkname: Using go:linkname is impossible due to the inlining of Source.Execute and Source.Event. The only option would be using //go:noinline in the Gio source code, which requires changes anyway.

2. go -overlay: This command allows replacing files and supposedly changing the current router.go. However, it is harder to use (requires generator + compilation arguments) and is currently incompatible with gogio.

3. External events: The point of gio-plugins was to be similar to Gio, allowing for similar usage across components. While calling gioexplorer.Open() and gtx.Execute(gioexplorer.Open{}) is not much different, we need to get the result. Using the current gtx.Event helps to streamline everything. Otherwise, you need to keep some callback function, channel, or spawn goroutines.

---

Switching from Struct to Interface fixes this issue, and it's the cleaner solution. It may have some performance impact, due to dynamic-dispatch. The current code also caches the source as Source to prevent mitigate allocations. Furthermore, the disabled use the same Source, instead of creating new ones.

[inkeliz]

I'm kindly pinging @eliasnaur since that issue is a significant for gio-plugins and other libraries.

[eliasnaur]

gioexplorer.OpenFileOp{Tag: p.tag, Mimetype: FileTypes}.Add(gtx.Ops) // add custom event to Ops

If I understand this you're encoding a custom op into the internal op.Ops.Internal structure? This is not guaranteed to be stable.

How about calling gioexplorer.Open(w.Invalidate) so that a redraw can be triggered and the caller can check for incoming transfer.DataEvents?

[inkeliz]

If I understand this you're encoding a custom op into the internal op.Ops.Internal structure? This is not guaranteed to be stable.

That was how it used to work, before Gio 0.6. I know that it wasn't stable, and Gio, in general, isn't stable too. However, the goal was to make it similar to other kinds of Ops (like clipboard.ReadOp).

I want to replace it with gtx.Execute(): gtx.Execute(gioexplorer.Open{...}). However, since Source isn't interface, I can't intercept that.

How about calling gioexplorer.Open(w.Invalidate) so that a redraw can be triggered and the caller can check for incoming transfer.DataEvents?

But how I can send transfer.DataEvents into the Gio Router? Also, some plugins might require custom Events/Filters. For instance, webviewer sends events about cookies, javascript executions (...). That requires a custom Filter. The Sourcecan only get Events, but can't send custom ones and the current code seems to use switch for filtering, which makes impossible to handle custom events/filters.

If Source is an interface, then I can use some kind of "decorator"/"wrapper" around the current Source. Or, if it uses //go:noinline (but that is hacky).

---

EDIT:

That is how things used to work on Gio 0.1~0.4:

func (l *Plugin) Events(t event.Tag) []event.Event {
	l.eventsMutex.Lock()
	defer l.eventsMutex.Unlock()

	evtsGio := l.queue.Events(t)
	evtsCustom, _ := l.eventsCustom[t]

	switch {
	case len(evtsGio) > 0 && len(evtsCustom) > 0:
		l.eventsPool = l.eventsPool[:0]

		l.eventsPool = append(l.eventsPool, evtsGio...)
		l.eventsPool = append(l.eventsPool, evtsCustom...)

		l.eventsCustom[t] = l.eventsCustom[t][:0]

		return l.eventsPool
	case len(evtsGio) > 0:
		return evtsGio
	case len(evtsCustom) > 0:
		l.eventsCustom[t] = l.eventsCustom[t][:0]
		return evtsCustom
	default:
		return nil
	}
}

That implements the old Queue. In that case, when requesting for Event, it verifies get both events: the evtsGio := l.queue.Events(t) is the Gio one, and then evtsCustom is custom evts. Then, we return it, base on what we have.

[inkeliz]

Benchmarks using gio-example kitchen:

Current Patch (New = This Patch):

goos: darwin
goarch: arm64
pkg: gioui.org/example/kitchen
             │     old      │                new                 │
             │    sec/op    │    sec/op     vs base              │
UI-12          426.1µ ± ∞ ¹   441.3µ ± ∞ ¹  +3.58% (p=0.032 n=5)
UI_Offset-12   412.5µ ± ∞ ¹   416.8µ ± ∞ ¹  +1.04% (p=0.008 n=5)
UI_Scale-12    560.1µ ± ∞ ¹   555.4µ ± ∞ ¹       ~ (p=0.222 n=5)
UI_Rotate-12   670.5µ ± ∞ ¹   666.5µ ± ∞ ¹       ~ (p=0.151 n=5)
UI_All-12      672.5µ ± ∞ ¹   669.9µ ± ∞ ¹       ~ (p=0.690 n=5)
geomean        536.3µ         539.3µ        +0.55%
¹ need >= 6 samples for confidence interval at level 0.95

             │     old      │                 new                 │
             │    gc/op     │    gc/op      vs base               │
UI-12          13.80m ± ∞ ¹   15.50m ± ∞ ¹  +12.32% (p=0.008 n=5)
UI_Offset-12   13.11m ± ∞ ¹   14.65m ± ∞ ¹  +11.75% (p=0.008 n=5)
UI_Scale-12    18.90m ± ∞ ¹   20.70m ± ∞ ¹   +9.52% (p=0.008 n=5)
UI_Rotate-12   18.23m ± ∞ ¹   19.82m ± ∞ ¹   +8.72% (p=0.008 n=5)
UI_All-12      19.27m ± ∞ ¹   20.80m ± ∞ ¹   +7.94% (p=0.008 n=5)
geomean        16.44m         18.09m        +10.04%
¹ need >= 6 samples for confidence interval at level 0.95

             │     old      │                new                 │
             │  sec/frame   │  sec/frame    vs base              │
UI-12          391.3µ ± ∞ ¹   398.7µ ± ∞ ¹       ~ (p=0.056 n=5)
UI_Offset-12   377.3µ ± ∞ ¹   375.3µ ± ∞ ¹       ~ (p=0.841 n=5)
UI_Scale-12    525.5µ ± ∞ ¹   513.6µ ± ∞ ¹       ~ (p=0.151 n=5)
UI_Rotate-12   640.4µ ± ∞ ¹   627.7µ ± ∞ ¹  -1.98% (p=0.008 n=5)
UI_All-12      642.1µ ± ∞ ¹   630.8µ ± ∞ ¹  -1.76% (p=0.008 n=5)
geomean        502.1µ         497.3µ        -0.94%
¹ need >= 6 samples for confidence interval at level 0.95

             │     old      │                 new                 │
             │    sec/gc    │    sec/gc     vs base               │
UI-12          501.9n ± ∞ ¹   566.9n ± ∞ ¹  +12.95% (p=0.008 n=5)
UI_Offset-12   487.1n ± ∞ ¹   538.2n ± ∞ ¹  +10.49% (p=0.008 n=5)
UI_Scale-12    658.3n ± ∞ ¹   739.4n ± ∞ ¹  +12.32% (p=0.008 n=5)
UI_Rotate-12   676.1n ± ∞ ¹   744.5n ± ∞ ¹  +10.12% (p=0.008 n=5)
UI_All-12      748.8n ± ∞ ¹   764.7n ± ∞ ¹        ~ (p=0.095 n=5)
geomean        605.6n         663.3n         +9.53%
¹ need >= 6 samples for confidence interval at level 0.95

             │     old      │                 new                 │
             │  sec/layout  │  sec/layout   vs base               │
UI-12          34.49µ ± ∞ ¹   42.32µ ± ∞ ¹  +22.71% (p=0.008 n=5)
UI_Offset-12   35.01µ ± ∞ ¹   42.15µ ± ∞ ¹  +20.41% (p=0.008 n=5)
UI_Scale-12    34.27µ ± ∞ ¹   41.05µ ± ∞ ¹  +19.81% (p=0.008 n=5)
UI_Rotate-12   29.83µ ± ∞ ¹   38.36µ ± ∞ ¹  +28.59% (p=0.008 n=5)
UI_All-12      29.93µ ± ∞ ¹   38.51µ ± ∞ ¹  +28.66% (p=0.008 n=5)
geomean        32.62µ         40.44µ        +23.97%
¹ need >= 6 samples for confidence interval at level 0.95

[eliasnaur]

What about a memory benchmark? As mentioned in #136 (comment), changing Source.Event from a concrete method to an interface or function pointer causes the filters argument to escape.

At a higher level, I'm not convinced allowing external packages to use the Gio event routing is a good idea. As I wrote above, users of the various plugins can add an extra Plugin.Events call in their event handling. It seems to me the only missing piece is a convenient way to plumb through app.Window.Invalidate?

[inkeliz]

What about a memory benchmark?

I will check it, and the heap-escape.

At a higher level, I'm not convinced allowing external packages to use the Gio event routing is a good idea.

That was already previously discussed, https://lists.sr.ht/~eliasnaur/gio/%3Cfe3835f7-b4d4-4db9-81fb-dfd8ab06f2ed%40www.fastmail.com%3E.

That is why I made gio-plugins indepently from Gio itself. I'm not asking to introduce any CustomEventOp or CustomEventCmd, and Golang lacks powerfull meta-programming, so it's not possible to easy change switch{} on compile-time (except using a custom builder).

Until Gio 0.6 was possible to get/insert events and works fine. Even if it's not promised to work, it does work due to Queue-interface and the Op that accepts anything as Tag, making possible to transfer data as Tag.

So, most of my code was already written with just a single gtx and Events calls.

It seems to me the only missing piece is a convenient way to plumb through app.Window.Invalidate?

I think it's more than just Invalidate. Consider this code:

for {
	evt := gioplugins.Event(w) // << It will transfer events to plugins, if the plugin needs such event. It returns the Gio events, as normal (or modified).


	switch evt := evt.(type) {
	case app.FrameEvent:
		gtx := app.NewContext(ops, evt)
		p.Layout(gtx)
		evt.Frame(ops) // << This FRame funciton is replaced by gioplugins Frame function too.
	}
}

The p.Layout contains:

	if p.uploadClickable.Clicked(gtx) {
		gtx.Execute(gioexplorer.OpenFileCmd{Tag: p.tag, Mimetype: _FileTypes})
	}

	if p.saveClickable.Clicked(gtx) {
		gtx.Execute(gioexplorer.SaveFileCmd{Tag: p.tag, Mimetype: _FileTypes[0], Filename: "image.png"})
	}

	for {
		evt, ok := gtx.Event(gioexplorer.Filter{Target: p.tag})
		if !ok {
			break
		}

		if p.loading {
			continue
		}
		switch evt := evt.(type) {
		case gioexplorer.SaveFileEvent:
	                 // ...
		case gioexplorer.OpenFileEvent:
	                 // ...
		case gi oexplorer.ErrorEvent:
			// ...
		case gioexplorer.CancelEvent:
			// ...
		}
	}

That was a quick port, from previous Op (where Gio doesn't have TransferEvent).

It's important to mention that gioexplorer also needs some events from Gio itself, namely the ViewEvent. Other plugins (such as Hyperlink or WebView) require other types of events, such as FrameEvent (to redraw) and ConfigEvent (see if still visible). So gioplugins.Event(w) does the job of broadcasting Gio events (such as ViewEvent) to all plugins, internally. The user itself doesn't need to do anything, only importing gioexplorer is enough.

Other types of plugins, like "Auth" (which is Sign In With Apple/Goole), needs FrameEvent to retrieve some URLEvent from #117. So, the plugin itself calls gtx.Event to retrieve that and issue new events, if appropriate.

Note that is already possible to use all plugins without messing with Events or anything. However, you also need to provide all information (from many types of events) manually, and call invalidate is just a part of that.

So, usually you need to have some kind of global variable or passing it across multiple components. The current Haptic and Explorer from gio-x have such problem. Consider the most basic one, Haptic, you need to use https://pkg.go.dev/gioui.org/x/haptic#NewBuzzer store it somewhere and then call Buzz(). If it uses gio-plugins, that could be simplified as: gtx.Execute(giobuzz.BuzzCmd{}) (and evt := gioplugins.Event(w) in the main-loop). Some packages like pref from gio-x are also limited, because it don't issues events, you need to call it to get information.

[inkeliz]

Using -gcflags "m" it does escape to heap. However, it's possible to trick using unsafe such as:

// Event returns the next event that matches at least one of filters.
func (s Source) Event(filters ...event.Filter) (event.Event, bool) {
	if !s.Enabled() {
		return nil, false
	}
	evt, ok := SourceEventProcessor(s.r, uintptr(unsafe.Pointer(&filters)))
	if !ok {
		return nil, false
	}
	runtime.KeepAlive(filters)
	return evt, true
}

func (q *Router) Event(filtersPtr uintptr) (event.Event, bool) {
	filters := *(*[]event.Filter)(unsafe.Pointer(filtersPtr))
	// ...
	
}

However, that is illegal in unsafe.Pointer rules.

---

So, the last option is to use some -overlay file. The issue is that it must change the original file, which is more prone to break. I miss Zig comptime magic here.