Open hajimehoshi opened 3 years ago
This might be useful for finer input detection. See also #926
CC @Zyko0, @eihigh
Let's separate this issue into introducing HandleInput and removing the global functions.
In terms of public API, what about two public methods like:
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
```go
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 ? 😅
Yes such 'EventArgs' style makes sense.
Before changing the API, we should consider to get realtime input states (see #2053)
Tinne's note: https://rentry.co/5sxu7
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
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
Yet another idea:
package inputevent
func WaitForEvent() *Event
// Usage
func foo() {
go func() {
for {
e := inputevent.WaitForEvent()
switch e.(type) {
// ...
}
}
}()
}
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.
https://github.com/glfw/glfw/issues/2158#issuecomment-1198385958
I think we need to separate the current rendering thread from the main thread.
I would like to resolve #2664 first, as this doesn't change any APIs but should improve latency of inputting.
My current idea
github.com/hajimehoshi/ebiten/v2/exp/ebitensuperinput
for example. I couldn't come up with a good name :-)github.com/hajimehoshi/ebiten/v2
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.ebiten
package are still available.Appendix:
Game
interface, but I have not decided.
// 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
@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?
@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,
Brief summary of the discussion we had on discord:
Game
interfaces with different Update()
signatures, we want only one Update()
signature (either Update() error
or Update(InputState) error
.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 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.
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.
@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 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.
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
?
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.
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.
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.
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.
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.
This can reduce the global functions for inputting.