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

X11/Wayland: Support the "Mod3" modifier and add extra keycodes for common Xkb keys #11871

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

Kontrabant
Copy link
Contributor

This adds support for the "Mod3" modifier, commonly used as "Level 5 Shift" (or often remapped for custom purposes), as well as adding some extra key codes for common Xkb mappings. Left Tab is found in the default ASCII keymap, Level 5 Shift is found in some international layouts, and the Meta, Hyper, and Multi-key Compose keys are often enabled by tweak tools for customizing the keyboard layout, so it makes sense to add them. These keycodes, which don't correspond to any particular scancodes in the USB spec, are mapped to SDL keycodes by setting bit 30 of the raw keysym value. This is safe, as keysyms are only 29 bits, and this is well out of the range of UTF32 characters.

This also allows for passing through unknown keysyms with the keysym bit set, which allows clients to handle any esoteric Xkb keys they want, of which there are many, without having to explicitly add them as SDL keycodes. This is particularly useful for virtual keyboards, which can have arbitrary layouts with symbols that don't map to physical keys.

Modifier key handling in X and Wayland is also made more robust, and will self-correct more quickly if common modifiers are remapped or toggled via some other means, and don't necessarily correspond to the currently pressed keys.

Fixes #11562
Fixes #11767

@slouken
Copy link
Collaborator

slouken commented Jan 6, 2025

You've defined bit 30 as used by platform dependent keysyms but then defined constants in the API that use them, which means that if another platform wants to pass through platform dependent keysyms, they have to avoid those values. It seems like we either want those keysyms in the normal range and available for mapping or we want to add a raw keysym in addition to the raw scancode. However, since we pass through the raw scancode, if somebody wants the keysym, they can grab the X11 connection and do the lookup themselves, right?

@Kontrabant
Copy link
Contributor Author

if somebody wants the keysym, they can grab the X11 connection and do the lookup themselves, right?

On X11, yes, although it's still not completely trivial. On Wayland it's quite a bit more complicated, as they would have to pull in xkbcommon as a dependency, get the keymap from the backend, and mirror all of the state themselves.

If passing the keysyms as keycodes is a NAK, I think a better solution would be something like a simple, system-specific SDL_GetXkbKeysymFromRawKey() function that clients could use if they want to query the keysym from the raw keycode without mandating excessive work on their end.

For moving these key codes into the normal range, any recommendations for adding a key like left tab, where a scancode entry doesn't really make sense, since it's just a shifted key code for the physical tab key? Maybe keep bit 30 as an "extended" flag for key codes that don't have a corresponding scancode and aren't Unicode values, but use simple incrementing values for the lower bits instead of Xkb keysyms?

@slouken
Copy link
Collaborator

slouken commented Jan 6, 2025

If passing the keysyms as keycodes is a NAK, I think a better solution would be something like a simple, system-specific SDL_GetXkbKeysymFromRawKey() function that clients could use if they want to query the keysym from the raw keycode without mandating excessive work on their end.

This seems reasonable to add if someone has a need for it. I wouldn't bother in advance though.

For moving these key codes into the normal range, any recommendations for adding a key like left tab, where a scancode entry doesn't really make sense, since it's just a shifted key code for the physical tab key? Maybe keep bit 30 as an "extended" flag for key codes that don't have a corresponding scancode and aren't Unicode values, but use simple incrementing values for the lower bits instead of Xkb keysyms?

Sure, that sounds good. We currently assume "scancode bit" or unicode in a bunch of places, so everywhere SDLK_SCANCODE_MASK is used would need to be evaluated for correctness.

@Kontrabant Kontrabant force-pushed the extra_keycodes branch 2 times, most recently from bb6e101 to 43cd428 Compare January 8, 2025 15:44
src/events/SDL_keymap.c Outdated Show resolved Hide resolved
@Kontrabant Kontrabant force-pushed the extra_keycodes branch 3 times, most recently from fe83f75 to 768b2b6 Compare January 8, 2025 17:08
@Kontrabant Kontrabant marked this pull request as ready for review January 8, 2025 17:55
Adds support for Mod3, which is usually Level 5 shift, but can vary, as well as not altering the functionality of the more esoteric modifier keys, such as meta and hyper.
Adds support for Mod3, which is usually Level 5 shift, as well as not altering the functionality of the more esoteric modifier keys, such as meta and hyper.

Also use the system modifier state instead of setting them based on key presses, which may be incorrect due to remapping, or toggled in some other manner.
Add SDL keycodes for keys found commonly found in the default Xkb layout, such as left tab and compose, and keys frequently used for custom modifiers such as Meta, Hyper, and Level5 Shift.

As these keys aren't Unicode code points and don't have associated scancodes (at least on modern keyboards), they are placed in the new extended key code space, with bit 30 set as a flag.
@Kontrabant
Copy link
Contributor Author

Added a bit in the migration docs to note the new keycode bitmask, and I think this is good to go.

A perusal of GitHub doesn't turn up anything that blindly presumes that any keycode without the scancode bit set is a Unicode value (some old versions of Quake 3 ports, but they also discard any non-ASCII characters). Still, it seems prudent to filter out these new key codes in sdl2-compat, as anything using SDL2 wouldn't know what to do with them anyways.

@Kontrabant Kontrabant merged commit 6b776a9 into libsdl-org:main Jan 9, 2025
40 checks passed
@Kontrabant Kontrabant deleted the extra_keycodes branch January 9, 2025 22:26
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.

ISO Left Tab ModeSwitch key is not working
2 participants