Implement input method keyboard grab

[xdavidwu]

This implements input-method-unstable-v2 keyboard grab.

Depends on: #4740, swaywm/wlroots#1864

[xhebox]

There're two problems of handle_key patch in 7471b43, i've modified a patch.

1. Update_short_cuts or what else code probably messed around the state, so that i can not receive release key event from the grab interface.

2. Even if i move the check to the most front, bypass all sway code. There's something wrong with it:

-       if (!handled && wlr_device->keyboard->group) {
+       if (!handled && wlr_device->keyboard->group && !virtual_keyboard) {
                // Only handle device specific bindings for keyboards in a group

As virtual_keyboard and real keyboard compose a keyboard_group, sway early return here. But we only use virtual_keyboard under grab, so it's essentially one keyboard.

The workaround patch is attached at the end.

EDIT: There are problems in swaywm/wlroots#1864 too.

EDIT2: handle_modifier has similar problems on keyboard_group.

@@ -329,6 +329,18 @@
        seat_idle_notify_activity(seat, IDLE_SOURCE_KEYBOARD);
        bool input_inhibited = seat->exclusive_client != NULL;
 
+       struct wlr_input_method_keyboard_grab_v2 *kb_grab =
+               seat->im_relay.input_method ? seat->im_relay.input_method->im_keyboard_grab : NULL;
+       struct wlr_virtual_keyboard_v1 *virtual_keyboard =
+               wlr_device->keyboard->virtual_keyboard;
+
+       if (seat->im_relay.input_method && kb_grab && !virtual_keyboard) {
+               wlr_input_method_keyboard_grab_v2_set_keyboard(kb_grab,
+                       wlr_device->keyboard);
+               wlr_input_method_keyboard_grab_v2_send_key(kb_grab,
+                       event->time_msec, event->keycode, event->state);
+       } else {
+
        // Identify new keycode, raw keysym(s), and translated keysym(s)
        xkb_keycode_t keycode = event->keycode + 8;
 
@@ -423,7 +435,7 @@
                handled = true;
        }
 
-       if (!handled && wlr_device->keyboard->group) {
+       if (!handled && wlr_device->keyboard->group && !virtual_keyboard) {
                // Only handle device specific bindings for keyboards in a group
                free(device_identifier);
                return;
@@ -451,6 +463,8 @@
                }
        }
 
+       }
+
        transaction_commit_dirty();
 
        free(device_identifier);

[xdavidwu]

Update_short_cuts or what else code probably messed around the state, so that i can not receive release key event from the grab interface.

Not all release events are missing, right? According to my tests, release events are sent successfully at most cases. Sometime release events seems still somewhat missing like I uses Mod4 as my binding modifiers, but when I switches workspace, sometime Mod4 seems like get stuck and needs an extra press to solve it after this patch. I am still investigating this situation.

The problems with keyboard grouping is explained at swaywm/wlroots#1864 (comment)

[xhebox]

My test IME will forward grab by virtual_keyboard immediately(i.e. just after receiving the key event).

Not all release events are missing, right?

Nope, all events are missing, with keyboard_group. I pressed the f, and seems it's marked as pressed, and I could not press f again. Extra presses never got me out the problem. I need a complete restart of sway.

Now with seat * keyboard_grouping none, it works almost perfectly. The problem must be related to the behavior of keyboard_group.

And the imperfect part is that, when I started IME in a terminal, and press CTRL+C, I missed the release event of CTRL+C(IME is killed by SIGINT). So the corresponding release event was not forwarded by virtual_keyboard. The result is that, I saw lots of ccccccccccccc in the terminal until i pressed c again.

This could be a responsibility of clients, but it will be nice if sway handle it.

EDIT: I am using musl-libc, with sway -d.

[xdavidwu]

And the imperfect part is that, when I started IME(forward grab by virtual_keyboard) in a terminal, and press CTRL+C, I missed the release event of CTRL+C. So the corresponding release event was not forwarded by virtual_keyboard. The result is that, I am seeing lots of ccccccccccccc in the terminal untile i pressed c again.

Terminals kill the IME at the time it received press event, and when release event arrives, IME is already gone. Currently virtual-keyboard implementation does not send missing release event when it is gone. I think IME should handle SIGINT to be safe.

[xhebox]

Yep, if virtual-keyboard handle it, then we need some records at the side of wlroots. Sounds like what you say pressed event sent state tracking. And with pressed event sent state tracking, grab could be moved to wlroots too.

Whatever it is not a big problem compared to missing events. Both solutions are ok though. It is just, program could be killed by various signals, it's hard to handle all cases. Exceptions could kill IME too.

Even IME handle this smoothly. If SIGKILL signal the program at similar time point, then program has no way to handle it at all. It is not asynchronously safe.

[xdavidwu]

I have just made virtual keyboard not to group when from same client as input method and also check and remove on grab start. It should make it work with keyboard grouping enabled but I think this can still be improved.

[emersion]

Tested with wlhangul, seems to work fine.

[emersion]

This PR needs a rebase.

[xdavidwu]

Commit to ungroup virtual keyboard is dropped. It is not needed after #5204.

[emersion]

Is this still WIP?

[simnalamburt]

Thank you! I could workaround the "Focus change bug" and fix the "Key repeat bug". This PR is currently being thoroughly tested by bunch of Korean sway users and now we're all experiencing no more bugs!

Is there any left blocker before merging this PR? Looks like the keymap spamming issue has been fixed with latest commit since it doesn't occur anymore!

[Riey]

I'm maintain new Korean IME kime and I met several repeat bugs of course it's IME bug but problem is its repeat was triggered in client side which is unexpected behavior for me

Why client get RepeatInfo even keyboard is grabbed? is it necessary?

[tadeokondrak]

I think there still is a different focus change bug in sway that lets an unfocused surface disable text-input, but it's related to text-input and not input-method, and is not related to keyboard grab.

EDIT: #6004 for layer-shell, #6005 for the above issue.

[simnalamburt]

Yeah I think this patch itself is enough tested and reviewed by bunch of people who desperately want to use IME with sway. :)

[fosskers]

I've seen the patched AUR package with this added. How is this actually configured and used? Please and thanks :)

[simnalamburt]

Just install patched version of sway and turn on the IME which have input-method-unstable-v2 supports like kime, fcitx5. Then it automatically works.

For Arch Linux with kime:

# Install two AUR: sway-im and kime-bin
paru -S sway-im kime-bin

# Run kime
systemd-run --user -du kime kime-wayland

# Run sway
sway

Reference

• https://aur.archlinux.org/packages/sway-im/

[fosskers]

I did manage to get fcitx5 working without a patched sway. Although I still can't type in alacritty for some reason.

[tadeokondrak]

I did manage to get fcitx5 working without a patched sway. Although I still can't type in alacritty for some reason.

This is because some applications also support IME input without going through the window system, using (for example) D-Bus instead of X11 or Wayland. Alacritty only supports X11 (xim) and Wayland (text-input-v3) IMEs directly, as far as I know.

[emersion]

I've been trying to review this for quite some time. I'm really sorry for the huge delay, once again.

Overall the logic looks correct to me.

I have trouble with the changes in handle_key_event. This function is already a little bit tricky to follow, and the changes introduce more complexity (duplicated wlr_seat_keyboard_notify_key, goto end). I've left some comments to see if we can improve that a bit.

In other news, we're trying to revive the IM standardization efforts in the wayland-protocols repo. If you have feedback about text-input or input-method, please feel free to leave a comment:

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/issues/39

[emersion]

Looks good to me, thanks a lot!