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

Heuristic to find hyper and meta on wayland #3943

Merged
merged 3 commits into from
Aug 18, 2021

Conversation

orki
Copy link
Contributor

@orki orki commented Aug 17, 2021

Pull request 3430 created hyper and meta for X11, but could not support Wayland since xkbcommon does not provide a way to map virtual modifiers to real modifiers. This patch identifies modifiers for the common case of each mapped modifier generated by its own single key. If such modifiers are not found, it falls back to the current static mapping; the current static mapping does not support meta and hyper. Tested with a wayland session on Plasma Wayland desktop.

Example of supported xkb modifier maps with normal key mapping:

modifier_map Shift  { Shift_L, Shift_R };
modifier_map Lock   { Caps_Lock };
modifier_map Control{ Control_L, Control_R };
modifier_map Mod1   { Meta_L };
modifier_map Mod2   { Alt_L };
modifier_map Mod3   { Hyper_L };
modifier_map Mod4   { Super_L, Super_R };

Example where the algorithm would fail since left and right map to different modifier keys:

modifier_map Mod1   { Meta_L, Alt_L };
modifier_map Mod2   { Meta_R, Alt_R };

@kovidgoyal
Copy link
Owner

AM curious what the performance implications are for this? How much work is this algorithm with typical keymaps?

@orki
Copy link
Contributor Author

orki commented Aug 18, 2021

It goes through every keycode (not keysym) in the keymap in the map exactly once or exactly twice. For a generic pc105 layout, that's at most 210 iterations, regardless of the number of levels or keysyms in the keymap. That said, I share the concern; if accepted, it might be better to stick this under a configurable parameter, given that speed is one of kitty's selling points.

On my Fedora 34 Plasma Wayland i5-6400, the algorithm seems to take an average of 96us with occasional spikes to 98us. This is unscientific, and is based on 10 repetitions of gettimeofday() calls around the call to local_modifier_mapping().

This handles the nonsensical case of Alt_X being mapped to two
different modifiers.
@kovidgoyal
Copy link
Owner

Hmm, I'm really not a fan of this. This needs to be solved at a layer below application code. It's crazy to expect every application to query the keymap like this. I do admire the creativity of the hack though, so I am OK with including it in kitty, but gate it behind an env var, something like KITTY_WAYLAND_DETECT_MODIFIERS.

If Wayland ever becomes actually useable, hopefully they will add support for this "officially" and we can remove the hack at that point.

@kovidgoyal
Copy link
Owner

Add a note about the env var used to glossary.rst

@orki
Copy link
Contributor Author

orki commented Aug 18, 2021

Gated behind the environment variable, along with the documentation changes. Do you want me to force rebase the branch on top of master, or would you prefer to do the merge yourself? (I have never done a force rebase with a pull request; it might take me a bit of time to figure out any unforeseen consequences.)

@orki
Copy link
Contributor Author

orki commented Aug 18, 2021

If and when this is in, I'll poke xkbcommon/libxkbcommon#36 to solicit more ideas.

@gabyx
Copy link
Contributor

gabyx commented Nov 30, 2023

@orki : Hey could you maybe give some help what is going on, I implemented your code in Rust in https://github.com/wez/wezterm/blob/36b5b464ae260fc1f7d2d16ba53f9f44bf939e1c/window/src/os/x11/keyboard.rs#L280
when wezterm iterates through the keys at startup I get to the Shift keycode 50 and after pressed I get the keysym 0xfe0a for the "keyrelease" of the shift key..., I checked with xev which is

KeyPress event, serial 34, synthetic NO, window 0x2800001,
    root 0x3f2, subw 0x0, time 51226571, (461,1328), root:(1746,1377),
    state 0x4000, keycode 50 (keysym 0xffe1, Shift_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 34, synthetic NO, window 0x2800001,
    root 0x3f2, subw 0x0, time 51227030, (461,1328), root:(1746,1377),
    state 0x4001, keycode 50 (keysym 0xfe0a, ISO_Prev_Group), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

This is utterly strange so in my setup it does not detect the shift key since it reports the up event??
Appreciate any insight...

@orki
Copy link
Contributor Author

orki commented Nov 30, 2023 via email

@gabyx
Copy link
Contributor

gabyx commented Nov 30, 2023

@orki Ehm: not really its the English US key board layout, simple stupid.
Somewhere must be an error, but ai dont see it :). Under Sway in Wayland…
By the way your code was pretty clean for C code!

@gabyx
Copy link
Contributor

gabyx commented Dec 1, 2023

@orki : kitty reports the same output as my code:

Loading new XKB keymaps
Modifier indices alt: 0x3 super: 0x6 hyper: 0xffffffff meta: 0xffffffff numlock: 0x4 shift: 0x0 capslock: 0x1 control: 0x2

it doesnt detect shift. Strange...

@gabyx
Copy link
Contributor

gabyx commented Dec 1, 2023

I have the following mapping

modmap -pm
xmodmap:  up to 4 keys per modifier, (keycodes in parentheses):

shift       Shift_L (0x32),  Shift_R (0x3e)
lock
control     Control_L (0x25),  Control_L (0x42),  Control_R (0x69)
mod1        Alt_L (0x40),  Alt_R (0x6c),  Alt_L (0xcc),  Meta_L (0xcd)
mod2        Num_Lock (0x4d)
mod3        ISO_Level5_Shift (0xcb)
mod4        Super_L (0x85),  Super_R (0x86),  Super_L (0xce),  Hyper_L (0xcf)
mod5        ISO_Level3_Shift (0x5c)

edit: I think kitty did not fail! otherwise ther would be a message in my debug log, so shift is index 0, above... I need to recheck the code again.

@gabyx
Copy link
Contributor

gabyx commented Dec 1, 2023

ok, the algorithm fails when the keymap uses grp:shifts_toggle which is a bit weird

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

Successfully merging this pull request may close these issues.

None yet

3 participants