FakeInput: add support for keyboard key press and release
ClosedPublic

Authored by jgrulich on Sep 7 2019, 9:59 AM.

Details

Summary

Adds support for keyboard button press and release as defined in linux/input-event-codes.h

Diff Detail

Repository
R127 KWayland
Branch
master
Lint
Lint ErrorsExcuse: Q_ENUM is a standard Qt macro
SeverityLocationCodeMessage
Errorsrc/client/keystate.h:50CppcheckunknownMacro
Unit
No Unit Test Coverage
Build Status
Buildable 16229
Build 16247: arc lint + arc unit
jgrulich created this revision.Sep 7 2019, 9:59 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 7 2019, 9:59 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
jgrulich requested review of this revision.Sep 7 2019, 9:59 AM
jgrulich added inline comments.Sep 7 2019, 10:38 AM
src/client/fakeinput.h
222

I will need a little bit of help to understand what type of key event should be used and I don't remember where I got the information it expects codes defined from linux/input-event-codes.h (I copied it from the previous review)

Anyway, for the remote desktop portal, we will need to support both keysym and keycodes [1].

[1] - https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.impl.portal.RemoteDesktop.xml#L226

jgrulich added inline comments.Sep 7 2019, 11:16 AM
src/client/fakeinput.h
222

Ok, I think I understand now.

KWin uses KeyboardInputRedirection::processKey(uint32_t key, InputRedirection::KeyboardKeyState state, uint32_t time, LibInput::Device *device), where Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state) is called. The second method expects a key code as defined in linux/input-event-codes.h and passing key + 8 to xkb_state_update_key() call.

So I think we want to keep it this way and accept key codes as defined in linux/input-event-codes.h so we can directly bind this in KWin to a key press event.

jgrulich updated this revision to Diff 65600.Sep 7 2019, 8:48 PM

Bump framework version

Apparently KDE Frameworks 5.62 are already out.

+1

src/client/fakeinput.h
222

We had a big discussion on this last kwin sprint.

There's value in keycodes, values in keysyms depending on the use case. Especially if the remote sender has a different keymap.. Potentially some things don't even map.

The protocol could do with a line that the code is related to the keymap set on the client's seat. (currently kwin only really supports one)


The "most technically correct" if for fake input to send a keymap, then us to forward that keymap to clients, then pass that keycode on the relevant fake keyboard.

IMHO we can do this for now and iterate a year from now.
Potentially using virtual_keyboard_unstable_v1 which is basically fully maps wl_keyboard in reverse.

apol added a comment.Sep 18 2019, 12:04 AM

The patch looks good to me, is there a reason why it wasn't accepted?

In D23766#533644, @apol wrote:

The patch looks good to me, is there a reason why it wasn't accepted?

I'm not sure from the comment whether David requested some changes to be made now or whether this is good for now and we can improve this in future.

davidedmundson accepted this revision.Oct 22 2019, 11:19 AM

No, just rambling.

Ship it.

This revision is now accepted and ready to land.Oct 22 2019, 11:19 AM
jgrulich closed this revision.Oct 22 2019, 11:24 AM