Bug 56866 - xkbcommon can not find keycodes for XK_Meta_L, XK_Hyper_L and XK_Hyper_R
Summary: xkbcommon can not find keycodes for XK_Meta_L, XK_Hyper_L and XK_Hyper_R
Status: RESOLVED FIXED
Alias: None
Product: libxkbcommon
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Daniel Stone
QA Contact: Ran Benita
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-08 11:45 UTC by Gatis Paeglis
Modified: 2012-11-23 22:06 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
test case (2.11 KB, text/plain)
2012-11-08 11:46 UTC, Gatis Paeglis
Details
Find the real mods for Hyper, Super, etc (9.95 KB, text/plain)
2012-11-15 14:15 UTC, Ran Benita
Details

Description Gatis Paeglis 2012-11-08 11:45:53 UTC
Since xkbcommon doesn't have an API for keysym-to-keycode i tried to write my own function and then i noticed that some keysyms are missing. 

I wrote a small test code where one version uses functions provided in xcb_keysyms.h together with xcb_key_symbols_t struct and other version with xkbcommon.

Output:

### With xcb_key_symbols_t ### 
index :  0   0x25d4140 
index :  1   0x25d4160 
index :  2   0x25da200 
index :  3   0x25da220 
index :  4   0x25da240 
index :  5   0x0 
index :  6   0x25da2e0 
index :  7   0x25da300 
index :  8   0x25da320 
### With libxkbcommon ### 
yes 
no 
yes 
yes 
no 
no 
yes 
yes 
yes 

The symbols i was looking for are     

const xcb_keysym_t symbols[] = {
        XK_Alt_L, XK_Meta_L, XK_Super_L, XK_Super_R,
        XK_Hyper_L, XK_Hyper_R, XK_Num_Lock, XK_Mode_switch, XK_Caps_Lock,
    };
Comment 1 Gatis Paeglis 2012-11-08 11:46:34 UTC
Created attachment 69691 [details]
test case
Comment 2 Ran Benita 2012-11-08 12:04:53 UTC
Your test case only looks at the first level of the first layout of each key, while I assume XCB looks at all of them.

The xkb_state type represents the state of a keyboard at a given point in time (i.e. after keys were pressed and released, layouts were switched, etc.). So when you use xkb_state_key_get_syms(), you are asking to get the correct keysym for the current active layout and modifiers in the state that you passed it.

What your function really wants to do is independent of a keyboard state, it just wants to enumerate all of the keysyms the can ever be generated by a given keycode (don't know why you'd want that, though). So what you want is to only look at the keymap. For this, you should do something like this:

num_layouts = xkb_keymap_num_layouts_for_key(...)
for each layout:
    num_levels = xkb_keymap_num_levels_for_key(...)
    for each level:
       num_syms = xkb_keymap_key_get_syms_by_level(..., &syms)
       if num_syms == 1
           add the keysym syms[0]

Note that in xkbcommon, each key press (or more accurately a single shift level) can produce multiple keysyms, like pressing <A> and getting 3 keysyms F O O. X doesn't do that, and you probably want to ignore those for now (hence the num_syms == 1).

Try this and see if you get what you expect.
Comment 3 Gatis Paeglis 2012-11-09 16:32:46 UTC
Sorry for late reply,

I realized what I actually need is a function that looks at all shift levels for a given key (similar to XkbKeyGroupWidth) and i need to do this only for the current base(active) layout. This is used in an algorithm to set modifier masks.

Later I listen for a state changes and update modifiers masks when the layout changes.

Today i encountered a crash in:
XkbKeyGroupWidth(const struct xkb_key *key, xkb_layout_index_t layout)

Crash happens only for keys which don't have a keysyms, in my case it crashes when i pass a key with code 93 to xkb_keymap_num_levels_for_key()

You can see how its used in (qxcbkeyboard.cpp):
https://codereview.qt-project.org/#change,39226

Output from xmodmop -pke:

keycode  92 = ISO_Level3_Shift NoSymbol ISO_Level3_Shift NoSymbol ISO_Level3_Shift ISO_Level3_Shift
keycode  93 =
keycode  94 = less greater backslash bar bar brokenbar bar brokenbar less greater onehalf threequarters
keycode  95 = F11 F11 F11 F11 F11 F11 XF86Switch_VT_11 F11 F11 XF86Switch_VT_11 F11 F11 F11 F11 XF86Switch_VT_11 F11 F11 F11 F11 XF86Switch_VT_11
keycode  96 = F12 F12 F12 F12 F12 F12 XF86Switch_VT_12 F12 F12 XF86Switch_VT_12 F12 F12 F12 F12 XF86Switch_VT_12 F12 F12 F12 F12 XF86Switch_VT_12
keycode  97 =
keycode  98 = Katakana NoSymbol Katakana NoSymbol Katakana Katakana
Comment 4 Daniel Stone 2012-11-09 18:22:43 UTC
XkbKeyGroupWidth isn't published API, or hasn't been for months.  The release equivalent is xkb_keymap_num_keylevels_for_key.  Are you using 0.2.0?
Comment 5 Gatis Paeglis 2012-11-09 18:41:52 UTC
By "similar to XkbKeyGroupWidth" i mean the Xlib version.
Yes, I am using 0.2.0 version and the public API function xkb_keymap_num_keylevels_for_key which internally calls xkbcommon function with signature XkbKeyGroupWidth(const struct xkb_key *key, xkb_layout_index_t layout).
Comment 6 Ran Benita 2012-11-09 19:59:38 UTC
The problem you are facing is this: suppose the keymap consists of 3 layouts (say us, in, ru). This means the the effective layout can be 0, 1 or 2. The the keycode for <A> is likely to be interpreted differently for each layout, so this key will internally have different keysyms for each layout (0, 1, 2). Now consider a keycode like <ESC>, this is very unlikely to be interpreted differently in each layout. So internally it only has one layout (0).

If the effective layout is 1, and you press <ESC>, it doesn't have that, so the layout is wrapped to fit the range of layouts that <ESC> knows about, and then it gets the keysym from there (in that case from layout 0). Same if the effective layout is 3, and you have a key which only defines 2, etc.

The crash happens when you call xkb_keymap_num_levels_for_key() with a layout that the keycode doesn't know about - the code assumes that the layout is find for the key. keycode 92 doesn't have any keysyms defined for it at all, so effectively this function will always crash for it.

In the little iteration code I described above, I call xkb_keymap_num_layouts_for_key() - this would return 1 for <ESC>, and 0 for keycode 92 - and so the num_levels_for_key part is never reached. What you are doing, is calling this function with the effective layout of the entire keymap, which is out of range for keycode 92.

I think your use case sound somewhat strange (maybe tell us what it is?), and seems to be with the X core protocol in mind, not XKB. But it is sort of legitimate (i.e. always passing the effective layout). We definitely don't want to crash... 

The way the layout wrapping is done is internal, and we don't want to expose this. This leaves us with 2 options, as far as i can see:

1. Always return 0 if the layout is out of range for this key - this implies the user should always call num_layouts_for_key() in some way before.
2. Wrap the layout into the legal range, like xkb_state_key_get_layout() would do.

The second option sounds sensible to me, and ordinary code which iterates on num_layouts_for_key() won't be effected. Daniel, what do you think?
Comment 7 Ran Benita 2012-11-09 21:19:55 UTC
I had a look at the code review page you linked to; nice work! It's a nice way to integrate X with libxkbcommon in the client, and cool to see it get done.

I understand now why you need this code. In order to determine which values to mask in for the Hyper, Super, Meta etc. modifiers in the Qt modifier mask, you look at the modifiers which are bound to keys which have the keysysm Hyper_L etc.

This kind of trickery might have been needed with plain xcb, but the entire setupModifiers function is really not the proper way to do this, with either xcb-xkb or xkbcommon. With XKB, what you want is to look at the (virtual) modifiers called Hyper, Super, etc, which at least with xkeyboard-config are properly defined. See here:
http://cgit.freedesktop.org/xkeyboard-config/tree/compat/misc
Pretty much always included.

Unfortunately, with xkbcommon we don't report these modifiers in the effective modifier mask currently (only real mods). Until we fix this, you should do this with xcb-xkb. You would need the XkbGetNames request to get the virtual modifier indexes associated with the names, and then XkbGetMap request to get the real modifiers associated with the virtual modifier, which you can then finally test for. A bit annoying, but quite better than the current hack you have. With xkbcommon it would be easier.

[ Also, an unrelated note: since you listen to XkbStateNotify events and update the xkb_state with update_mask accordingly, you are effectively acting as a slave for the master state in the server. So you don't really want to use xkb_state_update_key(); in fact, it might cause some problems. So try removing it and see what happens.
I also don't see where you free the keymap, context or state! ]
Comment 8 Daniel Stone 2012-11-09 23:31:33 UTC
Hmmm.  I'm almost tempted to say we should introduce another flag, but I
think the best option is probably to do the right thing by default and
wrap.  I didn't think of this usecase at all when I wrote those two getters.

(Also, yes, you should be using EFFECTIVE rather than DEPRESSED for the
state lookup, and not using xkb_state_update_key.)
Comment 9 Ran Benita 2012-11-10 22:38:24 UTC
I've added a commit that fixes it in my tree. It also wraps the layout in xkb_keymap_key_get_syms_by_level().
Comment 10 Ran Benita 2012-11-15 14:15:42 UTC
Created attachment 70119 [details]
Find the real mods for Hyper, Super, etc

Here is a program that does what I described in comment 7, adapted from some code I already had lying around. Beyond the XCB verbosity and GCCisms, it should be clear what it does. This is the "proper" way to do this with XKB, and should alleviate all of the hacks having to do with looking at keycodes and keysyms.

As I already mentioned, it should all become unnecessary in the future with xkbcommon, which would just allow you to do xkb_state_mod_name_is_active("Super") etc. But currently it doesn't.
Comment 11 Ran Benita 2012-11-23 22:06:01 UTC
The mentioned issues are fixed in master, so I'm closing this.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.