Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mouse Input Events called many times in between frames in 3.1 #26395

Closed
kubecz3k opened this issue Feb 28, 2019 · 15 comments
Closed

Mouse Input Events called many times in between frames in 3.1 #26395

kubecz3k opened this issue Feb 28, 2019 · 15 comments

Comments

@kubecz3k
Copy link
Contributor

kubecz3k commented Feb 28, 2019

Godot version:
3.1 Beta 8

OS/device including version:
GTX1060, Ubuntu 18.04

Issue description:
I encouraged this issue couple of times already in different projects, at a first glance it looks like the mouse input is a lot weaker in 3.1. After some investigation it turned out that Mouse Input Events are called many many times in between frames in 3.1 and this can lead to at least two problems:

  1. Performance drop (if we have some logic in input function)
  2. Feeling of a lot weaker input strength after porting project to 3.1 (when any of our logic that resides in process is based on the last input from mouse)

Steps to reproduce:
Quick way

  1. Download sample project
  2. Run it in 3.0, try to drag and drop sprite to the right edge of the window sceen
  3. Run the project in 3.1, try to drag and drop sprite to the right edge of the window sceen
  4. Notice that this is a lot harder in 3.1

Manual way

  1. Open Godot 3.0
  2. Create new scene, add a sprite as a child
  3. Add this code to the scene root:
onready var sprite = get_node("Sprite")
var last_relative = Vector2()

func _input(event):
	if event is InputEventMouseMotion && (event.button_mask & BUTTON_MASK_LEFT):
		last_relative = event.relative

func _physics_process(delta):
	sprite.translate(last_relative)
       last_relative = Vector2()
  1. Play a little, remember how it behaves
  2. Open the same project in 3.1 and notice the difference

Minimal reproduction project:
weak_mouse_reproduction_3.0.zip

@kubecz3k kubecz3k added this to the 3.1 milestone Feb 28, 2019
@kubecz3k
Copy link
Contributor Author

ping @hpvb

@hpvb hpvb self-assigned this Mar 1, 2019
@jonbonazza
Copy link
Contributor

I've noticed this as well when dealing with UI input. I have had to work around it with hacky flags. Thought maybe it was my hardware, so I hadn't reported it, but if others are seeing it as well, it likely is engine related.

@moiman100
Copy link
Contributor

I don't this is a bug. Event.relative is relative to the last event's position so
last_relative += event.relative
would work as long as it is zeroed after use.

@jonbonazza
Copy link
Contributor

Perhaps I am thinking of something different. The issue I saw was specifically around mouse clicks. Eaxh click was firing multiple times for me. I just chalked it up to my laptop's touch pad. I havebt tried it on my desktop and I haven't tried since the 3.1 alphas.

@akien-mga akien-mga assigned reduz and unassigned hpvb Mar 4, 2019
@reduz
Copy link
Member

reduz commented Mar 4, 2019

This is most likely due to raw input generating way more events.
@kubecz3k I added motion event accumulation some days ago, you can try enabling it with Input.set_use_accumulated_input(true)
let me know if this fixes the problem. I am considering enabling it by default (it will only be a problem if you do something like painting using the mouse as it will lose some resolution).

@kubecz3k
Copy link
Contributor Author

kubecz3k commented Mar 5, 2019

@reduz in current main (575f1d8) the problem is a lot smaller. In Beta8 count of input readings between frames was ~= 10 and in current main it's around 2.
Enabling set_use_accumulated_input fixes the problem entirely and I also think it might be good idea to use it by default - I think some people (including me) is used to performing some logic inside _input, so it would be a waste of computing power when that logic is called many times between frames :)

@Calinou
Copy link
Member

Calinou commented Mar 5, 2019

I am considering enabling it by default (it will only be a problem if you do something like painting using the mouse as it will lose some resolution).

This should be tested in first-person projects to make sure it doesn't increase mouse latency.

I think we should rather document this (along with ways to perform heavier processing in a real-time-friendly way).

@kubecz3k
Copy link
Contributor Author

kubecz3k commented Mar 5, 2019

I created an issue after porting third person game (and tested in it as well), also as I said this is new behavior that was not present in 3.0

@reduz
Copy link
Member

reduz commented Mar 5, 2019

@Calinou no.. it should not matter for any type of game except for games that may need painting or very fine gestue recognition, which is not really common... guess will enable it.

The reason it's happening now is most likely due to raw input being implmented on Windows and X11.

@expikr
Copy link

expikr commented Feb 12, 2022

I am considering enabling it by default (it will only be a problem if you do something like painting using the mouse as it will lose some resolution).

This should be tested in first-person projects to make sure it doesn't increase mouse latency.

I think we should rather document this (along with ways to perform heavier processing in a real-time-friendly way).

An issue that arises from accumulating inputs for the whole frame is that input order is lost, i.e. the contextual position at which a button state change occurs relative to user motion during that frame cannot be determined because the engine dispatches them as a single event, effectively saying that "all of these events happened simultaneously". The higher the speed of user motion relative to the framerate, the more positional error this incurs.

Suggestion: instead of accumulating the entire frame's worth of input events, partition the emitted events by types. If a sequence of events are of alike type, then accumulate them until the dispatcher encounters an unlike type (e.g. mouse motion [x10] -> keyboard press -> mouse motion [x3] -> keyboard release -> mouse motion [x13])

@Calinou
Copy link
Member

Calinou commented Feb 12, 2022

@xeonmc Input accumulation is currently disabled by default as the result of a bug: #55037

That said, when it was enabled by default, it worked out just fine in my experience. If you need more precision when input accumulation is enabled, godotengine/godot-proposals#2785 is a cleaner solution.

@expikr
Copy link

expikr commented Feb 12, 2022

@xeonmc Input accumulation is currently disabled by default as the result of a bug: #55037

That said, when it was enabled by default, it worked out just fine in my experience. If you need more precision when input accumulation is enabled, godotengine/godot-proposals#2785 is a cleaner solution.

The mentioned proposal solves the timing problem of velocity estimation, but not quite the order problem of button events being out of sequence from motion, at least not without the gamedev being aware of the quirk.

According to the documentation for InputEventMouseMotion, it is a object that conntains a single delta field for the coalesced motion.

If the proposal is to be implemented in a way that maintains compatibility, it would need to do so via extending it with a new property of an array of timestamped vectors (x, y, t).

However, because of the fact that the like-events are coalesced into the same event, the relative position of a button press in its motion sequence can only be retrieved by manually looping through the array and sort against the timestamps of the other event types.

It's doable if the gamedev knows to do so beforehand, but ugly and kinda defeats the whole purpose of a timestamped sequence, especially when you tell a developer "oh hey timestamp info is also available", their expectation would be that it would be a singular timeline of events that they don't need to do extra stuff for.

If we are sticking to the premise of only-one-event-per-type per frame when accumulation is enabled, then the only elegant way to solve it would be to add a new InputEvent type (maybe call it "InputEventTimeline") that contains the sequence of all events and their timestamp, which a gamedev can choose to use instead of monitoring the existing InputEvent types.

Now you've basically just recreated the idea of the InputEvent queue, the only difference being Godot not explicitly dispatching them multiple times per frame.

In that case, why not just implement it at the point where the accumulation is happening, and smartly dispatch them partitioned based on event type change?

This way it ensures the correct sequence of arrival for events, while having the performance benefit of accumulation since it's unlikely that a player's finger is fast enough that he/she can emit buttonpresses multiple times within the same frame.

@Calinou
Copy link
Member

Calinou commented Feb 12, 2022

@xeonmc Feel free to open a pull request for this 🙂
However, note that the input event accumulation may not work as expected since 3.4 due to the issue I linked.

I haven't worked with the input event accumulation system, so I can't implement this myself.

@expikr
Copy link

expikr commented Feb 12, 2022

@xeonmc Feel free to open a pull request for this 🙂 However, note that the input event accumulation may not work as expected since 3.4 due to the issue I linked.

I haven't worked with the input event accumulation system, so I can't implement this myself.

I'm not too familiar with the codebase, would you mind giving me (or point me to someone who can give me) a quick highlight of the relevant spots in the repo so I can start digging into it a bit quicker? GitHub search reveals some key points but I'm not confident about whether I missed something. Many thanks in advance.

@Calinou
Copy link
Member

Calinou commented Feb 12, 2022

In master (where pull requests should be opened first), it seems it's called input buffering internally:

} else if (use_input_buffering) {

You can also search for flush_buffered_events across the codebase. There are OS-specific portions to this, especially for Android.

The project setting is called input_devices/buffering/agile_event_flushing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants