Send modifiers to the Wayland server when a modifier key event needs to be forwarded

[fortime]

I'm using kwin 6 with wayland enabled, and I'm writing a virtual keyboard using the org.fcitx.Fcitx5.VirtualKeyboardBackend1 interface. Currently, a modifier key event is forwarded to the wayland server through wl_keyboard::key. This doesn't work, at least in kwin 6. I think the correct way is using wl_keyboard::modifiers.

My virtual keyboard works well with this patch.

This paragraph is composed using my virtual keyboard. Uppercase letters can be inputted since Shift works. 中文輸入也可以。

[fortime]

@wengxt can you have a review on this pr?

[wengxt]

the implementation should map modifier key -> key code from keymap with xkbcommon, instead of hardcode key code.

[fortime]

the implementation should map modifier key -> key code from keymap with xkbcommon, instead of hardcode key code.

Sorry, I'm new to xkbcommon. I can't find out which api I should use.

I have tried to use xkb_keymap_key_get_name to get a name associated to a keycode, and then use xkb_keymap_mod_get_index to get the modifier index. But, the name associated to a keycode and the name of a modifier are different.

For example, the name of keycode 50 is "LFSH", and I use "LFSH" to get the modifier index, it returns XKB_MOD_INVALID.

[wengxt]

The reason to use xkbcommon for it , is because, for example, xkb option may allow to do things like "swap capslock & ctrl", in that case, key code will be same for the key. Your implementation will be sending the wrong key.

The logic should be like:

1. A loop to enumerate over all key code, can be done with either: for ( keycode : range(xkb_keymap_min_keycode, xkb_keymap_max_keycode)), or xkb_keymap_key_for_each.
2. For each keycode, query with xkb_keymap_key_get_syms_by_level & xkb_keymap_key_get_mods_for_level, this allows you to query the mods(key state, modifier) & syms what a keycode can produce. You may want to pre-compute on keymap update instead of re-calculate every time you need to.
3. Send the corresponding key code.

[fortime]

2. For each keycode, query with xkb_keymap_key_get_syms_by_level & xkb_keymap_key_get_mods_for_level, this allows you to query the mods(key state, modifier) & syms what a keycode can produce. You may want to pre-compute on keymap update instead of re-calculate every time you need to.

Do you mean I should use key sym to check if it is a modifier?

1. In the beginning, I should hardcode a key sym to a modifier name (or a modifier index) mapping. Such as "Caps_Lock" -> "Lock".
2. Then, I loop over all key codes to find out all (key code + modifiers)s generating those key syms.
3. Finally, when a key code is forwarded, I use this key code and currently effectively modifiers to check if it is a modifier event.

Is it correct?

[wengxt]

@fortime well.. you shouldn't really assume, Control_L/R keysyms can produce Control modifier state. They are actually two separate concept.

The code to pick the key when xkb_keymap_key_get_syms_by_level has the desired keysym and verify with xkb_state_update_key can really produce the relevant mask

[fortime]

@wengxt can you check whether it is the logic you mean in the new commit?

[fortime]

hi @wengxt, can you have a look at this pr?

[fortime]

can anyone review this pr?

[wengxt]

My issue with this is that I don't like this approach. We have already experience lots of case that compositor stuck in a unrecoverable modifier state.

If you want to do this you have to reset the modifier map state to original one after you forward the key to reduce the risk.

But still even that, I still don't like the approach. I'd rather compositor can introduce a new event for such cases.

[fortime]

My issue with this is that I don't like this approach. We have already experience lots of case that compositor stuck in a unrecoverable modifier state.

If you want to do this you have to reset the modifier map state to original one after you forward the key to reduce the risk.

But still even that, I still don't like the approach. I'd rather compositor can introduce a new event for such cases.

If you don't like this approach, why not propose it at the first time? Instead, propose it after many revisions. The first revision is enough for me. Also, I can try other workarounds.

[wengxt]

I don't like but adding new protocol to wayland-protocols is hard.

There is really not another way other than this approach right now from wayland protocol point of view.

So it's really up to whether we have to solve it right now, or we can wait.

If you're thinking of approach like ydotool that need root, answer to that would simply be no. I wonder even consider that.