FakeInput: add support for keyboard key press and release
Needs ReviewPublic

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

Details

Reviewers
davidedmundson
apol
romangg
Group Reviewers
Plasma
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.Sat, Sep 7, 9:59 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptSat, Sep 7, 9:59 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
jgrulich requested review of this revision.Sat, Sep 7, 9:59 AM
jgrulich added inline comments.Sat, Sep 7, 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.Sat, Sep 7, 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.Sat, Sep 7, 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.Wed, Sep 18, 12:04 AM

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