Add new IME event for desktop platforms

[kchibisov]

This commit brings new IME event to account for preedit state of input
method, also adding Window::set_ime_allowed to toggle IME input on
the particular window.

This commit implements API as designed in #1497 for Wayland, X11, and
macOS.

Fixes #2174.

Co-authored-by: Artur Kovacs kovacs.artur.barnabas@gmail.com
Co-authored-by: Murarth murarth@gmail.com
Co-authored-by: Yusuke Kominami yukke.konan@gmail.com

--

I've finished implementing X11/Wayland bits. All functionality is supported across X11, Wayland, and macOS. The downstream implementation of new event handling is present in alacritty/alacritty#5790.

The PR handles all the platforms currently having IME input in winit, so there're no blocks left.

I've tested all the implementations with mentioned client and they worked as intedend from my experience, though I haven't payed much attention to macOS one, but at least it works.

cc @chrisduerr @garasubo

[kchibisov]

Now it has complete support across desktop platforms, so reviews are welcome.

[chrisduerr]

Only looked at the Linux implementations, but seems good to me mostly.

[kchibisov]

Also, what do you think about naming it Ime event instead of IME? I think the former is more rusty?

There's also an issue wrt setting surrounding text, but I think it could be done separately, since the original PR is more about event and not about IME helpers, since those helpers are not really required.

[maroider]

Also, what do you think about naming it Ime event instead of IME? I think the former is more rusty?

Ime seems more idiomatic than IME, yes.

[chrisduerr]

Pretty sure clippy even lints against IME?

[kchibisov]

I've done all renaming, if there won't be any issues raised until 23.04.2022 I'll merge it that day.

[madsmtm]

I created #2258 for some work on documentation that I'd like in this.

MacOS implementation looks good enough for me for now, I'll probably clean some things up after we fully revamp the keyboard API, but that's not a blocker for this.

[ArturKovacs]

First of all thanks for all your work guys! This is definitely one of the bigger improvements in winit. Although I found a bug.

I don't get Commit events on X11 with the Pinyin input. (Ubuntu 20.04)

I called set_ime_allowed(true) before entering the event loop

I type type:
nihao

The input selection box shows up correctly and I get the following events

ime: Preedit("", None, None)
ime: Preedit("", Some(0), Some(0))
ime: Preedit("", Some(0), Some(0))
ime: Commit("")
ime: Preedit("", None, None)
ime: Preedit("你", Some(3), Some(3))
ime: Preedit("你", Some(3), Some(3))
ime: Preedit("你好", Some(6), Some(6))
ime: Preedit("你哈", Some(6), Some(6))
ime: Preedit("你好", Some(6), Some(6))

Then I press 1 and the input selection box disappears but I don't get a Commit event. In only get the key release

regular keypress: KeyboardInput { scancode: 2, state: Released, virtual_keycode: Some(Key1), modifiers: (empty) }

The same happens if I press Return or Space instead of 1 (except of course I get the release event for the released key at the end)

[kchibisov]

In case someone has documentation suggestions, feel free to add them in #2258.

[kchibisov]

@ArturKovacs would you mind looking into preedit callbacks in X11 impl and events you're getting?

I can't repro that with default gnome + Pinyin on fedora 35. It works as expected for me.

It shouldn't be that hard to figure it out. If you don't have time to look into it, you should likely tell me what's your setup is so I can try to repro, though it'll take time that way.

Though, it's kind of interesting that KeyboardInput leaks on commit, not sure if we can suppress it, but should we try it?

[ArturKovacs]

I can't repro that with default gnome + Pinyin on fedora 35. It works as expected for me.

That's odd. I added trace! macros to the preedeit callback functions and I get "start" and "draw", but I don't get "done".

Maybe we should continue this conversation on Matrix, I could share my screen in a call if you want.

[kchibisov]

@ArturKovacs I'll try to look around and see what we can do here. Maybe XIM wants to insert text at different point other than in Done callback, but at least on fedora default IME setup everything works as expected...

[kchibisov]

@ArturKovacs would you mind trying latest commit on X11 one more time, it should work now from what I can see.

[kchibisov]

Also, should we do anything wrt keyboard input during IME input on X11? Should we not send it at all? What do you think @ArturKovacs ?

[ArturKovacs]

It works! You're a genius!

Regarding the keyboard input during preedit, I think we agreed that we never send that.

[kchibisov]

Regarding the keyboard input during preedit, I think we agreed that we never send that.

I've altered that as well, so it should work.

While working with preedit downstream, I've noticed that I'd like to have some sort of indication when preedit gets cleared, so when winit is sending Preedit(String::new(), None) event. Maybe we should indicate that preedit got cleared somehow, for example Preedit(enum { Cleared, Text }) or Preedit(Option<PreeditText>) and send None when it gets cleared?

[madsmtm]

You mean we could do the following:

enum Ime {
    // ...
    
    Preedit(String, usize, usize),
    PreeditCleared,
    
    // ...
}

With a translation of:

let new = match old {
    Preedit(string, None) => PreeditCleared,
    Preedit(string, Some((begin, end))) => Preedit(string, begin, end),
}

That is, string in Preedit(string, None) is always irrelevant?

[kchibisov]

That is, string in Preedit(string, None) is always irrelevant?

No, when that string is empty. So Preedit("a", None) is fine, but Preedit("", None) will translate into PreeditCleared.

[madsmtm]

And Preedit("", Some((0, 0))) -> PreeditCleared?

Anyway, seems to me like we're adding a state that is representable in multiple ways - but then again, I'm fairly certain macOS doesn't really have the concept of PreeditCleared, so I probably just don't really understand the purpose of it.

[kchibisov]

The purpouse to indicate that there's no preedit, so application can start processing normal keys input again? Certain IMEs are a bot fynamic here and forward regular keyboard input when preedit goes to empty string.

[ArturKovacs]

I still don't understand why we would need such an event. Winit should never send keyboard input events during preedit according to what we agreed. In other words: the application should always process KeyboardInput events because they only arrive outside of preedit. Therefore there's no need to indicate that the preedit ended.

[kchibisov]

Hm, I guess that's ok then. So besides upcoming changes to docs I don't see anything blocking it.

[madsmtm]

API and macOS LGTM 👍

[kchibisov]

Given that we had approval from each platform(except X11 due to missing maintainer, but backend was tested and originally submitted from one of them), we should be fine.

Issues that arise will be addressed latter on.