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

Prohibit execution of delayed input events by different means #89920

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Mar 26, 2024

In some cases it can happen, that the order of input events and window events is not followed, when input buffering or input accumulation is active.

The display server order "InputEvent => window-event" gets changed to "window-event => InputEvent" which becomes problematic in certain situations.
An example, where this was noticed is #80334, where the problem was mitigated by different means:

godot/scene/main/window.cpp

Lines 2720 to 2723 in 7d151c8

} else {
// Prevent update based on delayed InputEvents from DisplayServer.
return;
}

However this solution was problematic in some situations like Android. (fix #89683)

This PR makes sure, that the order is adhered to by flushing input events before a window event is sent.

The solution of this PR is global for all platforms, since now the new behavior is ensured in the Window-class.

fix #85695

@Alex2782
Copy link
Contributor

Alex2782 commented Mar 27, 2024

#80011
Godot v4.1.1.stable - Debian GNU/Linux trixie/sid trixie

I have verified, that this problem also happens on Windows platforms

My tests on Windows 10 and MacOS 14.3 (Mac Mini M1), v4.1.1.stable and v4.2.1.stable.
I could not reproduce delayed InputEvents from DisplayServer.

Tests

Windows 10 (original MRP, #80011):

Screen-2024-03-27-151601.mp4

MacOS

Screen-2024-03-27-154137.mp4

Perhaps also a solution if it only occurs under Linux? #ifdef LINUXBSD_ENABLED

void Window::_update_mouse_over(Vector2 p_pos) {
	if (!mouse_in_window) {
		if (is_embedded()) {
			mouse_in_window = true;
			_propagate_window_notification(this, NOTIFICATION_WM_MOUSE_ENTER);
		} else {
#ifdef LINUXBSD_ENABLED
			// Prevent update based on delayed InputEvents from DisplayServer.
			return;
#endif
		}
	}

return prevents the input events from arriving on Android, probably also on iOS, because there are no WINDOW* MOUSE events.

@Sauermann Sauermann requested a review from a team as a code owner March 27, 2024 19:31
@Sauermann Sauermann changed the title Prohibit execution of delayed input events Prohibit execution of delayed input events by different means Mar 27, 2024
@Sauermann
Copy link
Contributor Author

Just to be clear: I was able to replicate the problematic behavior also in Windows.
So I have adjusted the PR to apply to all platforms.

Also note, that this PR doesn't supersede #89914. Both PRs address valid problems.

In some cases it can happen, that the order of input events and
window events is not followed, when input buffering or input accumulation
is active.

The display server order `InputEvent` => window-event gets changed to
window-event => `InputEvent` which becomes problematic in certain
situations.

This PR makes sure, that the order is adhered to by flushing input events
before a window event is sent.
Previously this problem was mitigated by discarding these delayed events.
But this solution was problematic in the setting of android input events.
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the customary TIWAGOS, this makes sense to me.

@akien-mga akien-mga merged commit 32dcaa0 into godotengine:master Apr 4, 2024
16 checks passed
@Sauermann Sauermann deleted the fix-event-order branch April 4, 2024 12:46
@akien-mga
Copy link
Member

Thanks!

@Sauermann
Copy link
Contributor Author

@akien-mga Since this PR fixes a "very bad" release blocker #85695, it might be considered for cherry-picking. I would consider the changes not risky for cherry-pickinig.

@Sauermann Sauermann added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Apr 27, 2024
@akien-mga
Copy link
Member

Awesome, thanks for bisecting!

@akien-mga
Copy link
Member

Cherry-picked for 4.2.3.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants