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: Further improvements to keyboard handling #446

Open
3 tasks
aperezdc opened this issue May 25, 2022 · 5 comments
Open
3 tasks

x11: Further improvements to keyboard handling #446

aperezdc opened this issue May 25, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@aperezdc
Copy link
Member

In #445 we got some of the key modifiers working on X11 thanks to @ubercomp but quoting from the PR description, there are some other improvements that could be still done in the future:

  • unifying xcb_handle_key_press and xcb_handle_key_release
  • handle xkb keyboard map changes
  • handle AltGr
@ubercomp
Copy link
Contributor

One thing I forgot to mention was that I believe it's possible to replace the call to obtain the keysym with a call to xcb_key_symbols_get_keysym from the libxcb-keysyms library, which theoretically should help handle all edge cases. However, I'm not 100% sure of that, and, furthermore, it would add a new dependency (.so is about 15 kb), so I thought I should ask first: are we willing to add this new dependency if it significantly improves keyboard handling?

@aperezdc
Copy link
Member Author

aperezdc commented May 25, 2022

@ubercomp I think depending on libxcb-keysyms is very reasonable, I expect most users who might use the X11 backend will not mind.

Plus, 15 KiB is almost nothing these days, and I expect using it will handle some tricky corner cases that we may get wrong with out own implementation... at least my experience with input handling code is “there is always one more corner case to handle”, so it seems smart to offload the work to battle-tested components.

One more data point: I see in Debian's popcon that 25% of installs have it installed, which is a lot. Most people who have X11 installed will likely have libxcb-keysyms already installed.

@ubercomp
Copy link
Contributor

@aperezdc that's what I think as well. basically, I can only guarantee the current implementation for the "pc105" keyboard with US layout / keymap. I'll play with libxcb-keysyms when I have some spare time.

@psaavedra
Copy link
Member

this commit 48dfac2 deserves a dependency with libxkbcommon to be reflected in the CMake rules:

/home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:50:9: error: unknown type name 'xkb_mod_mask_t'
|    50 |         xkb_mod_mask_t shift_mask;
|       |         ^~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:51:9: error: unknown type name 'xkb_mod_mask_t'
|    51 |         xkb_mod_mask_t alt_mask;
|       |         ^~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:52:9: error: unknown type name 'xkb_mod_mask_t'
|    52 |         xkb_mod_mask_t control_mask;
|       |         ^~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:262:8: error: unknown type name 'xkb_mod_mask_t'
|   262 | static xkb_mod_mask_t
|       |        ^~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c: In function 'keysym_modifiers_get_mask':
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:265:5: error: unknown type name 'xkb_mod_index_t'
|   265 |     xkb_mod_index_t index = 0;
|       |     ^~~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:276:12: error: 'XKB_MOD_INVALID' undeclared (first use in this function)
|   276 |     return XKB_MOD_INVALID;
|       |            ^~~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:276:12: note: each undeclared identifier is reported only once for each function it appears in
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c: In function 'text_input_modifiers_map':
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:288:65: error: 'XKB_MOD_NAME_SHIFT' undeclared (first use in this function)
|   288 |     priv->modifiers.shift_mask = keysym_modifiers_get_mask(map, XKB_MOD_NAME_SHIFT);
|       |                                                                 ^~~~~~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:289:63: error: 'XKB_MOD_NAME_ALT' undeclared (first use in this function)
|   289 |     priv->modifiers.alt_mask = keysym_modifiers_get_mask(map, XKB_MOD_NAME_ALT);
|       |                                                               ^~~~~~~~~~~~~~~~
| /home/bot/yocto-browsers/builds/wandboard-nightly/tmp/work/cortexa9t2hf-neon-poky-linux-gnueabi/cog/trunk+httpsAUTOINC+286db0e633-r0/git/platform/wayland/cog-im-context-wl-v1.c:290:67: error: 'XKB_MOD_NAME_CTRL' undeclared (first use in this function)
|   290 |     priv->modifiers.control_mask = keysym_modifiers_get_mask(map, XKB_MOD_NAME_CTRL);
|       |                                                                   ^~~~~~~~~~~~~~~~~

@aperezdc
Copy link
Member Author

For the record, we have merged support for using xcb-keysyms in #679 — but leaving this issue open as there is still work to do.

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

No branches or pull requests

3 participants