ebiten icon indicating copy to clipboard operation
ebiten copied to clipboard

handle asynchronous input events

Open hajimehoshi opened this issue 3 years ago • 26 comments

func (g *Game) HandleInput(input Input) {
    // input.IsKeyPressed(...)
}

This can reduce the global functions for inputting.

hajimehoshi avatar Jul 09 '21 13:07 hajimehoshi

This might be useful for finer input detection. See also #926

CC @Zyko0, @eihigh

hajimehoshi avatar Aug 21 '21 13:08 hajimehoshi

Let's separate this issue into introducing HandleInput and removing the global functions.

hajimehoshi avatar Aug 21 '21 13:08 hajimehoshi

In terms of public API, what about two public methods like:

  • library
package ebiten

type InputEvent interface {} // This wouldn't have to expose public methods, idk just an interface

type MouseClickEvent struct {
	position, duration, ebiten.MouseButton
}

type MouseMoveEvent struct {
	position
}

type KeyboardKeyPressEvent struct {
	ebiten.Key, duration
}

type TouchPositionEvent struct {
	position
}

type GamedPadButtonPressed struct {
	ebiten.GamepadButton, duration
}

// more input events...

type Device byte

const (
	DeviceKeyboard Device = iota
	DeviceMouse
	DeviceGamepad
	DeviceTouch
)

func RegisterDevices(devices ...Device) {
	// manage an internal list of the devices the user wants to listen to
	// if this is not called, default to every available device ?
}
  • user
func (g *Game) HandleInput(events []InputEvent/interface{}) {
	for i := range events {
		switch e := events[i].(type) {
			case ebiten.MouseClickEvent:
				// populate click
			case ebiten.MouseMoveEvent:
				// adjust game cursor position / player orientation
			case ebiten.TouchPositionEvent:
				// adjust game cursor position / player orientation AS WELL if it ever changed
			default:
				// not handling any other event, but useless events to user should be reduced by the call to ebiten.RegisterDevices
		}
	}
}

func main() {
	g := &Game{}
	ebiten.RegisterDevices(ebiten.DeviceMouse, ebiten.DeviceTouch) // I don't want to listen for gamepad or keyboard for example might help internal perf to skip checks, and also user loop and number of events to check
	ebiten.RunGame(g)
}

This is pretty random, probably not idiomatic with the .(type) check and the weird InputEvent interface. I know it also introduces new events (move/position changed). Not the greatest idea overall, but there might be something to take or to bounce off ? 😅

Zyko0 avatar Oct 23 '21 10:10 Zyko0

Yes such 'EventArgs' style makes sense.

hajimehoshi avatar Oct 23 '21 10:10 hajimehoshi

Before changing the API, we should consider to get realtime input states (see #2053)

hajimehoshi avatar Jul 05 '22 16:07 hajimehoshi

Tinne's note: https://rentry.co/5sxu7

hajimehoshi avatar Sep 21 '22 14:09 hajimehoshi

https://github.com/hajimehoshi/ebiten/issues/2487#issuecomment-1341481107

From this discussion, global functions for inputting cannot work expectedly (It might be possible to design to make them work, but this requires a lot of work...). The input state can be changed anytime during Update and the developer cannot get consistent results from global functions. Sigh...

EDIT: This is treated at #2496

hajimehoshi avatar Dec 08 '22 03:12 hajimehoshi

Another idea instead of HandleInput is providing an API returning a channel

func NotifyInput(chan<- *InputEvent) // https://pkg.go.dev/os/signal#Notify
// or
func AsyncInput() <-chan *InputEvent

hajimehoshi avatar Dec 16 '22 08:12 hajimehoshi

Yet another idea:

package inputevent

func WaitForEvent() *Event
// Usage

func foo() {
    go func() {
        for {
            e := inputevent.WaitForEvent()
            switch e.(type) {
                // ...
            }
        }
    }()
}

hajimehoshi avatar Dec 18 '22 15:12 hajimehoshi

From the discussion on Discord, we are incliened to adopt callbacks instead of channels, since 1) callbacks is way much more efficient than channels and 2) users can use channels on a callback system if they want, but the opposite direction is not efficient.

hajimehoshi avatar Dec 18 '22 17:12 hajimehoshi

https://github.com/glfw/glfw/issues/2158#issuecomment-1198385958

I think we need to separate the current rendering thread from the main thread.

hajimehoshi avatar Dec 28 '22 05:12 hajimehoshi

I would like to resolve #2664 first, as this doesn't change any APIs but should improve latency of inputting.

hajimehoshi avatar May 09 '23 10:05 hajimehoshi

My current idea

  • Create a package github.com/hajimehoshi/ebiten/v2/exp/ebitensuperinput for example. I couldn't come up with a good name :-)
  • The new package imports the original github.com/hajimehoshi/ebiten/v2
  • Define these APIs
package ebitensuperinput

import (
    "github.com/hajimehoshi/ebiten/v2"
)

type InputEvent struct {
    // ...
}

func (*InputEvent) InputType() InputType // to report input events e.g. a mouse is pressed, for example
// ...

type Game interface {
    HandleInput(inputEvent *InputEvent) error
    ebiten.Game
}

func RunGame(game Game, options *ebiten.RunGameOptions) error
  • HandleInput is called whenever user inputs happen. So, this can catch every inputs e.g. mouse dragging, while the current Update might miss some input events.
    • HandleInput is called even when a file is dropped.
  • HandleInput takes one argument InputEvent, which represents the latest input event.
  • The global functions for inputting in ebiten package are still available.

Appendix:

  • In Ebitengine v3, this might be a standard Game interface, but I have not decided.
    • In this case, the ways in which handling inputs would be quite different between v2 and v3. We can convert the new Game to the current Game in the following converter. This solution could reduce global functions (#1778 ).
// These API are not included in the suggested package so far, but represents the current rough idea for v3.

// InputStateForCurrentTick represents a frozen input state for the current tick, like the current global functions do.
type InputStateForCurrentTick struct {
}

func (*InputStateForCurrentTick) IsMouseButtonPressed(m MouseButton) bool

type GameWithoutHandleInput interface {
    Update(inputState InputStateForCurrentTick) error
    Draw(screen *ebiten.Image)
    Layout(float64, float64) (int, int)
}

func GameFromGameWithoutHandleInput(GameWithoutHandleInput) Game

hajimehoshi avatar Oct 05 '23 16:10 hajimehoshi

@tinne26 pointed out that handling all the events could have performance impact. Also, having two Game interfaces doesn't good. I'll rethink my suggestion...

EDIT: Another concern: which goroutine is used to call HandleInput?

hajimehoshi avatar Oct 05 '23 17:10 hajimehoshi

@SpaiR

Hi SpaiR, I read your comment https://github.com/SpaiR/StrongDMM/issues/161#issuecomment-1517633824. I was wondering how async input handlings would satisfy your requirements. Perhaps, would you like to catch mouse move events in a real time manner, for example? Thanks,

hajimehoshi avatar Oct 05 '23 18:10 hajimehoshi

Brief summary of the discussion we had on discord:

  • Tick-based state freezing should remain the main method to handle input even in ebitengine/v3. New async APIs should be opt-in, not opt-out.
  • Filtering events and selecting which input sources are monitored can be an important topic, but we can decide more about it later when we have actual performance measurements. Upcoming API proposals may or may not include this feature, we don't have to decide yet.
  • We don't want multiple Game interfaces with different Update() signatures, we want only one Update() signature (either Update() error or Update(InputState) error.
  • Input being globally exposed is a debated topic. It can make implementation a bit trickier, but it's also convenient for users. In any case, even if we end up using global functions, at the very least we still want to make everything more structured, e.g. ebiten.Input().Cursor().GetPosition() (this also applies to window and monitor functions).
  • inpututil will end up on a weird spot after the changes. Currently, inpututil feels like an extension or expansion of what ebiten already provides. With the model change, inpututil may end up feeling a bit out of place. The Is*JustPressed() methods are quite vital in making Ebitengine feel "batteries included". Can we move part of that into ebiten, remove inpututil and otherwise let users rely on new third-party libs like https://github.com/quasilyte/ebitengine-input? Should we keep inpututil as it is? Should we reduce its scope? Something else? Unclear.

Here's one proposal I made, which actually omits HandleInput(), keeps only one Game interface and an Update() signature without params:

type Input struct { ... }
func Input() *Input
func (*Input) Keyboard() *InputKeyboard
func (*Input) Cursor() *InputCursor
func (*Input) Gamepads() *InputGamepads
// etc.

type InputKeyboard struct { ... }
type KeyboardEvent struct { ... }
func (*InputKeyboard) SetHandler(func (KeyboardEvent)) // set to nil for filtering?
func (*InputKeyboard) IsKeyPressed(ebiten.Key) bool // last tick frozen state
func (*InputKeyboard) KeyPressedTicks(ebiten.Key) int // unclear, see earlier inpututil notes
func (*InputKeyboard) AppendInputChars([]rune) []rune // this one is also slightly inpututil-like
// etc.

// Game interface remains the same, no HandleInput()
type Game interface {
    Update() error
    Draw(*ebiten.Image)
    Layout(float64, float64) (int, int)
}

(We could also have ebiten.Keyboard() instead of ebiten.Input().Keyboard(), and so on)

tinne26 avatar Oct 05 '23 20:10 tinne26

@tinne26 Thank you for the summary!

Before the discussion, I thought we had to replace the global functions with an argument at Update or something, but now I think the global functions are still useful, might not be elegant though. inpututil is still not elegant, but at least we can organize them.

I totally agree that we should organize the global functions anyway.

I've updated my proposal based on the discussion. Aside from event filtering, I prefer an 'optional' HandleInput function like we did for DrawFinalScreen.

package ebiten // or an experimetal package?

type InputEvent struct { ... }

type InputHandler interface {
    // HandleInput is invoked when any user inputs occur.
    // HandleInput is called in the same groutine as Update, Draw, and Layout.
    // (So, this might include a very slight delay from an actual input event)
    // For filtering input events for performance, let's revisit later when we find it necessary.
    HandleInput(inputEvent *InputEvent) error
}

func RunGame(game Game) error // If Game implements InputHandler, Ebitengine invokes it when necessary.

So, this is the same as https://github.com/hajimehoshi/ebiten/issues/1704#issuecomment-1749288978 basically, but without an appendix.

hajimehoshi avatar Oct 06 '23 04:10 hajimehoshi

If we go with an experimental package, InputHandler would no longer be optional.

package ebitensuperinput

import (
    "github.com/hajimehoshi/ebiten/v2/ebiten"
)

type InputEvent struct { ... }

type Game interface {
    HandleInput(inputEvent *InputEvent) error
    ebiten.Game
}

func RunGame(game Game, options *ebiten.RunGameOptions) error

As I am not sure how big the InputEvent would be, so I think I'll start with an experimental package.

hajimehoshi avatar Oct 06 '23 05:10 hajimehoshi

@SpaiR

Hi SpaiR, I read your comment SpaiR/StrongDMM#161 (comment). I was wondering how async input handlings would satisfy your requirements. Perhaps, would you like to catch mouse move events in a real time manner, for example? Thanks,

@hajimehoshi GLFW provides the ability to add direct callbacks for mouse (docs) and key (docs) changes. With these callbacks, you can immediately respond to user actions. Since these callbacks capture system I/O, you can be sure that you won't miss anything. This enhances overall input precision and UX responsiveness, whereas fixed-rate mouse/keyboard events cannot guarantee that nothing will be missed.

In my use case, I tried to use Dear ImGui binding with Ebiten as a general rendering pipeline. While the rendering worked fine, features like "holding a key and forwarding it to the UI" didn't work at all. The same issue applied to the mouse. For instance, when I have a tilemap, I want to move the mouse and see every tile I pass through while moving. If a user moves the mouse too fast, it becomes impossible to achieve this, as the pause between update cycles is too long.

In the Godot engine, there are two distinct process methods. One operates at a fixed-rate for handling physics, while the other runs at the CPU update rate. This second method is precisely what is needed to accurately capture inputs. (Source: https://ask.godotengine.org/140210/difference-between-_process-and-_physics_process)

SpaiR avatar Oct 06 '23 06:10 SpaiR

@SpaiR Thanks! I hope my latest proposal HandleInput would work.

One question: would it work for you if input events are not handled in a real time manner but 'accumulated' in every tick? In other words, if you can know all the accumulated input events for one tick at Update, would it be enough? As Draw is still called every frame in any cases, both HandleInput and this accumulating way should not cause delay.

EDIT: For a monitor with high refresh rate like 144Hz, handling events at Update might but be enough and HandleInput is better. So I think I'll go with HandleInput.

hajimehoshi avatar Oct 06 '23 07:10 hajimehoshi

Note to myself:

I expect HandleInput is called during rendering commands are flushing. graphicscommands.FlushCommands blocks until the previous command queue is flushed, and HandleInput should be called during this blocking. There are some tricky things in this timing. For example, as (*Image).At flushes a command queue, should we allow this call in HandleInput?

hajimehoshi avatar Oct 07 '23 19:10 hajimehoshi

Note to myself:

HandleInput actually improves the latency slightlly, but apprently disabling vsync improves much more. I'll double-check this later, but if this is true, I might deprioritize this task.

hajimehoshi avatar Nov 06 '23 15:11 hajimehoshi

Gentle reminder: this is most likely what we discussed on October 28th (early update + FIFO blocking leading to async events taking a longer time to be applied than one might initially expect). I have no great suggestion to offer in FIFO mode, but a nice experiment to do would be to measure the time between the event being sent to HandleInput and the time it's actually applied. Then, repeat with Update doing a wait at the start (like, 7ms or so (though Windows requires special care on short sleeps)) and see if the average "time to apply input" decreases, and how much. I don't think it's primarily the fault of HandleInput, but of FIFO buffer swap blocking.

tinne26 avatar Nov 07 '23 15:11 tinne26

a nice experiment to do would be to measure the time between the event being sent to HandleInput and the time it's actually applied.

I think I did it. This was less than 1[ms] as far as I remember. I'll try this later.

hajimehoshi avatar Nov 07 '23 15:11 hajimehoshi

We can test examples/paint and examples/drag in the branch issue-1704-handleevent. In both examples, -event flag enables HandleEvent mode. I also added a code to enable/disable vsync on my local machine, and saw what was going on, rather than measuring latencies. I think it would be pretty hard to measure an accurate latency since disabling vsync changes the way how to render the screen in the hardware layer.

hajimehoshi avatar Nov 07 '23 16:11 hajimehoshi

So, for GUI application, I think it is pretty important how fast rendering results are visible to users, and disabling vsync improves this. HandleEvent (formerly known as HandleInput) also improves to some extent, but I guess this is not so visible. Even with vsync disabled, SetScreenClearedEveryFrame(false) can suppress to consume CPU and GPU power. So for GPU applications, disabling vsync should be fine.

For some special applications that need to handle events more accurately, yes, HandleEvent would be still important. But I still want to deprioritize this as the problems HandleEvent could resolve seem smaller than I expected first.

hajimehoshi avatar Nov 07 '23 16:11 hajimehoshi