Bug 378452 - Escape key not detected by several games in wayland
Summary: Escape key not detected by several games in wayland
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: input (show other bugs)
Version: 5.9.4
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL: https://phabricator.kde.org/D5488
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-05 00:56 UTC by Rajdeep Nanua
Modified: 2017-05-12 14:08 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:
mgraesslin: Wayland+
mgraesslin: X11-
mgraesslin: ReviewRequest+


Attachments
Workaround to get escape key working in games (573 bytes, patch)
2017-04-11 01:30 UTC, Rajdeep Nanua
Details
p1.patch (539 bytes, patch)
2017-05-10 03:00 UTC, Rajdeep Nanua
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rajdeep Nanua 2017-04-05 00:56:40 UTC
When playing Team Fortress 2, Counter Strike Global Offensive or Euro Truck Simulator 2, escape key is not detected. Following are the steps to reproduce:
1. Launch game.
2. Enter a game session (or go anywhere escape input can be detected).
3. Scenario 1: escape key won't work. Scenario 2: escape key works, but stops working when after switching to another application and switching back to game window.
Comment 1 Martin Flöser 2017-04-05 04:55:00 UTC
Which Xwayland version are you using?
Comment 2 Rajdeep Nanua 2017-04-05 13:17:56 UTC
I am using Xwayland 1.19.3
Comment 3 Martin Flöser 2017-04-05 14:48:08 UTC
How are you pressing the escape key? A long press or a short press and release?
Comment 4 Rajdeep Nanua 2017-04-05 14:58:05 UTC
A short press and release.
Comment 5 Martin Flöser 2017-04-05 18:24:46 UTC
(In reply to Rajdeep Nanua from comment #4)
> A short press and release.

meh, a long press I would have understood what goes on.
Comment 6 Roman Gilg 2017-04-10 15:58:02 UTC
I just tried it on KDE Neon with self compiled Xorg Server / XWayland from Git and Neverball, that shows a similar behaviour in the following way:

* Start Neverball in XWayland mode: In main menu with mouse interaction everything fine. Esc works. Start a level and message "Pointer motion confined..." in the left up corner. Pointer motion is somewhat miscalculated though. Neither short or long press Esc does something. When the level finishes the message appears again shortly and the mouse cursor is shown again (end menu screen). I can't move the mouse cursor though. Long press Esc releases the mouse cursor and afterwards short Esc works again.

* Start Neverball in Wayland native mode (export SDL_VIDEODRIVER=wayland): No message appears. Esc and everything else works.
Comment 7 Rajdeep Nanua 2017-04-11 01:30:58 UTC
Created attachment 104956 [details]
Workaround to get escape key working in games

I narrowed down the problem to a specific problematic code in input.cpp. With escape-workaround.patch, escape key works. Hope that helps isolate the issue.
Comment 8 Martin Flöser 2017-04-11 15:02:16 UTC
That this code segment is responsible for it was already obvious to me :-)
Comment 9 Martin Flöser 2017-04-11 15:02:49 UTC
The bigger question is on the why it happens. The code should let short presses through and that is not happening.
Comment 10 Martin Flöser 2017-04-15 10:05:32 UTC
After re-reading the code I think I understand the problem. We send a keyboard leave event to Xwayland whenever escape gets pressed. We need to delay it till we trigger.
Comment 11 Martin Flöser 2017-04-18 05:15:48 UTC
Proposed patch: https://phabricator.kde.org/D5488
Comment 12 Rajdeep Nanua 2017-04-19 00:14:13 UTC
Unfortunately, that patch did not fix the issue for me.
Comment 13 Sandeep 2017-05-07 03:32:55 UTC
I am also facing the issue on KWin 5.9.5 and XWayland 1.19.3
Comment 14 Rajdeep Nanua 2017-05-10 03:00:19 UTC
Moving m_timer->start to after passToWayland has been called seems to fix the problem while allowing to break out of pointer confinement by holding esc for 3 seconds. See p1.patch.
Comment 15 Rajdeep Nanua 2017-05-10 03:00:54 UTC
Created attachment 105427 [details]
p1.patch
Comment 16 Martin Flöser 2017-05-10 05:17:30 UTC
I must say that I don't understand (yet) why moving the timer changes anything
Comment 17 Martin Flöser 2017-05-10 18:41:35 UTC
So I analyzed the patch and it is quite similar to my change. The result is partially the same. The moving of the timer has the same effect as the removal of "input()->keyboard()->update();" in my change. The main difference is that passToWaylandServer happens in your change but not in my change. Although I assumed that a later filter would do that. This might explain why my change doesn't work.

What your change doesn't support right now is the hiding of escape from the window on a long press which cancels the pointer grab. I guess we need the best of both approaches ;-)
Comment 18 Martin Flöser 2017-05-11 15:59:28 UTC
I updated the phab request. Given the autotest the escape key should be passed to the application now.
Comment 19 Rajdeep Nanua 2017-05-12 00:44:03 UTC
That works! Thanks a bunch, Martin.
Comment 20 Martin Flöser 2017-05-12 04:55:47 UTC
Will be fixed with the patch going into repo
Comment 21 Martin Flöser 2017-05-12 05:07:20 UTC
Git commit 229be65e4002987e2de0113ef9b7140492d2d681 by Martin Flöser, on behalf of Martin Gräßlin.
Committed on 12/05/2017 at 05:06.
Pushed by graesslin into branch 'Plasma/5.9'.

Improve the escape key handling for breaking constrained pointers

Summary:
So far KWin started to filter out the escape key as soon as it gets
pressed. This was done by unsetting keyboard focus. The idea was to
reset keyboard focus when it is only a short press and that then the
keyboard state is correct for the application. But in practice this
does not work. The only application currently supporting pointer
constraints (Xwayland) does not do anything on a key which is pressed
when gaining keyboard focus. The result is escape not working in
pointer constrained Xwayland windows.

This change addresses this problem by changing the interaction to only
unset keyboard focus when our break constraints condition is met. This
should also result in the application not handling the key release, but
it means it gets the key press. Unfortunately I don't have a good way
to test.

Test Plan: None

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D5488

M  +3    -2    autotests/integration/pointer_constraints_test.cpp
M  +5    -3    input.cpp
M  +1    -1    keyboard_input.cpp

https://commits.kde.org/kwin/229be65e4002987e2de0113ef9b7140492d2d681
Comment 22 Roman Gilg 2017-05-12 10:34:35 UTC
When testing this out I encountered one time a session breaking failure:
ASSERT: "m_spy.isNull()" in file /home/roman/Entwicklung/kde/source/kde/workspace/kwin/onscreennotification.cpp, line 204

I assume it's about the notification in upper left corner that the pointer constraining is active, but could be unrelated to the change here. Never seen it before though.
Comment 23 Martin Flöser 2017-05-12 14:08:15 UTC
(In reply to Roman Gilg from comment #22)
> When testing this out I encountered one time a session breaking failure:
> ASSERT: "m_spy.isNull()" in file
> /home/roman/Entwicklung/kde/source/kde/workspace/kwin/onscreennotification.
> cpp, line 204
> 
> I assume it's about the notification in upper left corner that the pointer
> constraining is active, but could be unrelated to the change here. Never
> seen it before though.

It's a bug, but unrelated. Unfortunately it's one of those things we cannot test in the integration tests as it requires QtQuick and that cannot be run on the CI system as we don't have OpenGL there.