search

rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.73k stars 888 forks source link

Tracking issue for IME / composition support #1497

Open murarth opened 4 years ago

murarth commented 4 years ago

This issue tracks the implementation of IME composition events on each platform, an API initially proposed in #1293. PRs implementing this event should be made against the composition-event branch, which will be merged into master once all implementations are complete.

chrisduerr commented 4 years ago

As a consumer of winit I'd recommend against naming these events CompositionEvent. Applications that deal with display servers will already know that term in connection with window compositing and it could easily be confused with something related to that.

It also does not fall in line with the existing winit naming of functions like set_ime_position.

I think it would be much better to have a more self descriptive name for this event that cannot be confused with anything else related to the winit project. I'd strongly suggest that IME should at least be part of the name.

pickfire commented 4 years ago

Oh, I thought CompositionEvent means some kind of compose key thing, I never used those and don't even know how to use compose key, I had only used IME to input characters not in the keyboard.

garasubo commented 4 years ago

@pickfire This event naming comes from web specification (Ref: https://developer.mozilla.org/en-US/docs/Web/API/Element/compositionstart_event). And IME will create those composition events. But as @chrisduerr is pointed out, I agree that calling those events as "composition" is confusing.

garasubo commented 4 years ago

Proposed the rename these events to IMEPreeditEvent in #1622

kaiwk commented 4 years ago

~For mac, maybe we can use this API? composedString~

Seems setMarkedText does the trick:

jakerr commented 4 years ago

Just FYI here's a similar problem being solved in the glfw project that might provide some hints / inspiration: glfw/glfw#658

kchibisov commented 3 years ago

I've decided to take a look on the proposed events wrt compositing, since I've been working on IME handling in winit for Wayland recently.

Right now I can see that the current API that is being proposed looks more like

pub enum CompositionEvent {
    CompositionStart(String),
    CompositionUpdate(String, usize),
    CompositionEnd(String),
}

If I were modelling it from Wayland side it'll be more like.

pub enum CompositionEvent {
    /// IME got enabled on the surface you're on.
    CompositionEnabled
    /// We start a composition, a cursors are offsets into string, which could represent a 'highlight' string. `None` for both means hidden.
    CompositionPreedit(text: String, cursor_start: Option<usize>, cursor_end: Option<usize>)
    /// Deletes surrounding text before and after current cursor.
    CompositionDeleteSurroundingText(before_length: usize, after_length: usize)
    /// Composition is done, and that's the text that should be inserted into widget.
    CompositionCommit(text: String)
    /// Composition is done, user must process all events and apply them atomically
    CompositionDone
    /// IME focus got lost, no need to process any IME stuff anymore.
    CompositionDisabled
}

So those are just plain events that we have straight from a Wayland protocol. The thing is that it works in some kind of atomic commits way, if you think about it. So on CompositionDone you must perform the following:

  1. Replace existing preedit string with the cursor.
  2. Delete requested surrounding text.
  3. Insert commit string with the cursor at its end.
  4. Calculate surrounding text to send.
  5. Insert new preedit text in cursor position.
  6. Place cursor inside preedit text.

Now the question is how to model that with the proposed IME events.

So first of all I don't need CompostionDone event, since this is an event when I'd send things downstream, meaning that I don't need CompositionDeleteSurroundingText. And it seems like I don't need DeleteSurroundingText, since I can model it via CompositionPreedit. CompositionCommit(text: String) is indeed needed so the user will know when to insert text.

So if we now apply that to the proposed composition event it'll look more like.

pub enum CompositionEvent {
    CompositionEnabled,
    CompositionPreedit(String, Option<usize>, Option<usize>),
    CompositionCommit(String),
    CompositionDisabled,
}

I guess it should make it a bit clear, so we removed 1 event for users to carry about when editing takes place.

However what wasn't mentioned is that how user interacts with IME via requests. Right now in winit we only have fn set_ime_position, which is just some basic stuff. However user may want to opt for IME handling, via e.g. fn enable_ime, so they will react that way on CompositionEnabled(basically means that user got IME opened). And they could want to hide IME sometimes, when they interact with a keyboard, but it doesn't require IME things, like a game, where you have a chat with IME support, so they will need fn disable_ime.

Also, the user is likely want to set text around the cursor and a cursor position, so they will need set_surrounding_text, and also a content type of the current text being edited, with set_content_type, so IME can provide better suggestions.

In the end Window will get

fn set_ime_surrounding_text(text: String, cursor: usize);
fn enable_ime_handling();
fn disable_ime_handling();
fn set_contents_type(type: ContentType);

The changes shouldn't affect existing work on IME in winit much, and purely about ergonomics and additions.

@garasubo since you've started initial work on updating IME handling in winit, I'm curious whether proposed changes here make sense, and are possible on X11.

@kaiwk As the one who send composition event implementation. Are proposed changes sound reasonable to you and possible on macOS, or macOS could need a bit more things to work, nicely?

@chrisduerr As a winit consumer that deals with text input, and that should implement IME handling, does the proposed API sounds ergonomic to implement? One benefit is that you'll know precisely when you actually have IME, so you won't call to set_ime_position until you've got an event from a winit that IME was enabled for some window.

Also, since winit doesn't have IME handling on a lot of platforms to land IME handling APIs we can only implement it as a concept on platforms that support it, like X11/macOS, and soon™ Wayland. Platforms that don't support IME in particular could just send IME text via ReceivedChar event like IME is working right now.

pickfire commented 3 years ago

One benefit is that you'll know precisely when you actually have IME, so you won't call to set_ime_position until you've got an event from a winit that IME was enabled for some window.

Based on my experience with xinput on X11, I believe it is good to be able to call set_ime_position when the IME was enabled rather than updating it for every key press or some weird solution to make some timer to update it once it a while.

garasubo commented 3 years ago

However what wasn't mentioned is that how user interacts with IME via requests.

That's a great idea. But for me, adding those APIs sounds out of scope of this issue. My original motivation is to get IME related event from the window system to implement CompositionEvent API in the web browser engine. So, I didn't add APIs to control IME from the application. I think it would be better to have another issue to add those APIs to make this problem simple. I can support x11 implementation.

kaiwk commented 3 years ago

Are proposed changes sound reasonable to you and possible on macOS, or macOS could need a bit more things to work, nicely?

Actually, i'm not very familiar about IME, the PR just fix my own problem, but i'll see what i can do for it. Is there any detailed doc about surrounding text and content type here? thanks.

kchibisov commented 3 years ago

So, I didn't add APIs to control IME from the application. I think it would be better to have another issue to add those APIs to make this problem simple. I can support x11 implementation.

My motivation was to change enum to those 4 variants, so it's in scope of that issue, since I don't quit like the 3 variants we have right now. We can add window methods later however I'd really like to see enum reworked, so users stop calling IME functions without IME being enabled.

pickfire commented 3 years ago

Can it commit partially?

garasubo commented 3 years ago

So, I didn't add APIs to control IME from the application. I think it would be better to have another issue to add those APIs to make this problem simple. I can support x11 implementation.

My motivation was to change enum to those 4 variants, so it's in scope of that issue, since I don't quit like the 3 variants we have right now. We can add window methods later however I'd really like to see enum reworked, so users stop calling IME functions without IME being enabled.

I see. I meant that adding API to control IME from window (e.g. enable_ime_handling) could be an independent issue, but either way is fine. I can help x11 implementation anyway.

I'm not sure what is the surrounding text concept. I think in x11 there is no such concept. Could you explain this more specifically? Maybe part of preedit string to convert, like I type "きょうもいいてんきですね" in IME and try to convert "きょう" to "今日" firstly, then, "きょう" is surrounding text and preedit string is "きょうもいいてんきですね"? Also, when can the second variant of CompositPreedit (cursor_start) be None? In x11, a cursor pos is always defined.

kchibisov commented 3 years ago

I see. I meant that adding API to control IME from window (e.g. enable_ime_handling) could be an independent issue, but either way is fine. I can help x11 implementation anyway.

Yeah, I agree, we can do it separately.

I'm not sure what is the surrounding text concept. I think in x11 there is no such concept. Could you explain this more specifically? Maybe part of preedit string to convert, like I type "きょうもいいてんきですね" in IME and try to convert "きょう" to "今日" firstly, then, "きょう" is surrounding text and preedit string is "きょうもいいてんきですね"?

It's for when you already have some text around and start editing, so you tell IME that you have certain things around, and your cursor is at some position. You're not required to send this request on Wayland and if you don't know what is around your cursor you don't send it. So if before starting edition you had きょう and a cursor right behind that text, you send this string and a cursor position right after the last char, so IME will know that it already has きょう, so suggestions are not empty right away, but include your text.

Also, when can the second variant of CompositPreedit (cursor_start) be None? In x11, a cursor pos is always defined.

On Wayland it's None when you should hide a cursor, I'm not sure how to expose that, since Wayland passes negative numbers here, so for rust I've translated that part into None. If you have an idea how to translate that concept it'll be nice.

kchibisov commented 3 years ago

Can it commit partially?

The purpose of Commit on IMEs is that you insert text to a widget and move cursor to a new position. You can't commit partially, since it's basically 'apply change'. While you're editing you're getting preedits, which informs about the current string, but once user hit a key that commits its input like Enter(this is handled by IME) you get commit and start new editing.

kchibisov commented 3 years ago

Actually, i'm not very familiar about IME, the PR just fix my own problem, but i'll see what i can do for it. Is there any detailed doc about surrounding text and content type here? thanks.

Those were just suggestions from Wayland IME protocol that thought should apply to other display servers, since those seems essential for IME.

You can read more about concepts I'm talking about https://github.com/wayland-project/wayland-protocols/blob/master/unstable/text-input/text-input-unstable-v3.xml . If macOS doesn't have those and have something different feel free to post what it has, since I don't really know anything about IME on macOS.

chrisduerr commented 3 years ago

@chrisduerr As a winit consumer that deals with text input, and that should implement IME handling, does the proposed API sounds ergonomic to implement? One benefit is that you'll know precisely when you actually have IME, so you won't call to set_ime_position until you've got an event from a winit that IME was enabled for some window.

I'd like to point out that this could be a potential pitfall? Is the IME popup only going to be visible once I set its position? Or is it always going to pop up and then move around potentially?

I'm also not quite sure what CompositionPreedit is for or why it is needed in general. Which of course is something that should be very clear for an API that downstream has to implement.

kchibisov commented 3 years ago

I>'d like to point out that this could be a potential pitfall? Is the IME popup only going to be visible once I set its position? Or is it always going to pop up and then move around potentially?

The goal is the following, you get CompositionEnabled event from winit, and then you enable IME editing on a window specific window you've got such event. After that all calls to set_ime_position, set_surrounding_text will work, if you don't call enable nothing will work and IME won't be handled at all, and window won't ever popup. It could be done in a way that calling to set_ime_position will result in automatically enabling ime handling, but I'm not sure about it. The goal is to minimize downstream work, since right now they call set_ime_position every time without IME being presented.

I'm also not quite sure what CompositionPreedit is for or why it is needed in general. Which of course is something that should be very clear for an API that downstream has to implement.

It's telling you about string that is currently being edited, but you can't insert it into widget yet. This is a quite common name and concept in IME world tbh. The goal is to display that string to end user, so they know what they're typing, since IME window isn't guaranteed to be presented unlike preedit string, and it makes typing more natural. You generally display that string inline in your text editor and user can move a cursor inside it and edit edit while typing.

chrisduerr commented 3 years ago

The goal is the following, you get CompositionEnabled event from winit, and then you enable IME editing on a window specific window you've got such event. After that all calls to set_ime_position, set_surrounding_text will work, if you don't call enable nothing will work and IME won't be handled at all, and window won't ever popup.

If the client has to call that, they'd still have to permanently call set_ime_position, right? I don't see how this changes anything about IME being presented or not. Or does downstream actually have to ask for IME every time? In which case I'd like to know how they would know about that?

It's telling you about string that is currently being edited, but you can't insert it into widget yet. This is a quite common name and concept in IME world tbh. The goal is to display that string to end user, so they know what they're typing, since IME window isn't guaranteed to be presented unlike preedit string, and it makes typing more natural. You generally display that string inline in your text editor and user can move a cursor inside it and edit edit while typing.

So basically the same as CompositionUpdate? I don't see how the start/update/end API wouldn't be sufficient then tbh. The String payload of start being optional even.

kchibisov commented 3 years ago

If the client has to call that, they'd still have to permanently call set_ime_position, right? I don't see how this changes anything about IME being presented or not. Or does downstream actually have to ask for IME every time? In which case I'd like to know how they would know about that?

It's a WindowEvent you don't call for it, you naturally check for it in event handling, and then you may set a state in your app that IME is presented. With CompositionEnabled window event winit tells you that you actually have IME running, on a system without IME you won't ever get that event and so you won't ever call to set_ime_position or check for anything other than your apps internal state.

evlp.run_return(move |event, _, _| {
    match event {
        WindowEvent::CompositionEnabled => {
            inner_state.ime = true;
            window.enable_ime();
        },
        _ => (),
    }

    if inner_state.ime {
        window.set_ime_position(x, y);
    }
});

And if you don't call to enable_ime it won't ever be enabled meaning that you won't have IME handling for your window. Before you've got CompositionEnabled you don't need to call anything from IME stack, since you don't have it. In the current API you must do the following to handle IME which is odd.

evlp.run_return(move |event, _, _| {
    window.set_ime_position(x, y);
});

Meaning you always call to IME even though you don't have IME on your system, which basically results in more work for downstream and for winit.

This was a pseudo code and not real rust.

So basically the same as CompositionUpdate? I don't see how the start/update/end API wouldn't be sufficient then tbh. The String payload of start being optional even.

Update is the way Preedit is called there, you'll be fine with Start/Update/End, but you don't need them, you only need Preedit and Commit or Update/End, I've just named them a bit more IME-ish and removed start, since I don't see a purpose for it.

kchibisov commented 3 years ago

Alternative we can embed enable_ime in other calls, since user must call set_ime_position or set_surrounding text for IME to actually work on at least Wayland. We'd still need disable to temporary disable IME though.

chrisduerr commented 3 years ago

It's a WindowEvent you don't call for it, you naturally check for it in event handling, and then you may set a state in your app that IME is presented. With CompositionEnabled window event winit tells you that you actually have IME running, on a system without IME you won't ever get that event and so you won't ever call to set_ime_position or check for anything other than your apps internal state.

Yeah, but doesn't the IME window pop up as soon as CompositionEnabled is sent to us? Then the window will appear in the wrong location. Unless you don't show the window until set_ime_position is called.

If you have to get an enabled IME event and then explicitly have to ask for things to actually get enabled and then also set the ime position that just seems like an extremely awkward API where nobody would be able to understand what the hell you're supposed to do without looking at an example.

kchibisov commented 3 years ago

Yeah, but doesn't the IME window pop up as soon as CompositionEnabled is sent to us?

No. IME isn't started unless you enable it, that's how at least it work on Wayland. Compositor tells you that you have IME around, and you should react to it with enable and then issuing set_ime_position or set_surrounding text if you want. You react not right away, but after you've got that event. So it's system informs you, you react whether to opt in or not. And also, IME window != IME, since it could be completely missing.

If you have to get an enabled IME event and then explicitly have to ask for things to actually get enabled and then also set the ime position that just seems like an extremely awkward API where nobody would be able to understand what the hell you're supposed to do without looking at an example.

How are you going to approach it differently? Just calling random IME things all the time isn't an option. We may include enable in set_ime_position and set_surrounding_text, but I don't see how you'll live with the current winit API, since it's nearly impossible to work with IME if you don't have a notion that IME was enabled. The event could be called differently if that's the concern.

chrisduerr commented 3 years ago

We may include enable in set_ime_position and set_surrounding_text

I don't see what the problem would be with that. In general unless a position is set, there's no point in showing it anyways.

I don't see how you'll live with the current winit API

There's nothing wrong with the current API. It doesn't matter if you check downstream if IME is visible or if winit does it internally. Forcing the downstream to do it just because it makes people feel like it's more efficient is just unnecessary work for everyone. I really don't see how this is "impossible".

kchibisov commented 3 years ago

There's nothing wrong with the current API. It doesn't matter if you check downstream if IME is visible or if winit does it internally. Forcing the downstream to do it just because it makes people feel like it's more efficient is just unnecessary work for everyone. I really don't see how this is "impossible".

Yeah, but then you should compute every draw call ime position, which could be not desired for some applications and slow, issue surrounding text, meaning that you must create strings on each render tick that will be dropped right away, and also set content type, so you will have to call 3 functions constantly.

The point is that the work for clients and winit is unequal, if clients should do the same amount of work as winit, it’ll be fine, but you literally will have 3 functions that you should call.

Also it doesn’t bring more work, since you’re free to ignore those events, however if you handle IME you’ll still have to carry IME state around in clients due to preedit string.

Also on Wayland if you disable IME right in the middle of editing, client must clear preedit string, and without that event winit must fake empty commit or something, which feels hackish.

runiq commented 3 years ago

Triage: MacOS has been done in #1979.

garasubo commented 3 years ago

@runiq No, that PR is not for implementing IME events. We still need to implement new APIs for MacOS.

runiq commented 3 years ago

Ah, sorry for the noise then—feel free to hide/delete.

garasubo commented 2 years ago

After @kchibisov rebased the branch to the latest master, X11 IME event stopped working somehow. I'm currently investigating why my change stop working with latest master.

KentaTheBugMaker commented 2 years ago

I'm implementing web IME base on egui_web does. But I have question for IME::Preedit . We must to implement it ? CompositionEvent Api does not provide Preedit cursor begin and end.

komi1230 commented 2 years ago

@t18b219k I appreciate your support. About this question, my answer is YES. As you know, enum IME::Preedit needs some arguments like (preedit_text, cursor_start, cursor_end). In CompositionEvent API in web, there are three event type: CompostionStart, CompositionUpdate and CompositionEnd. I checked the implementation of web_sys and found web_sys has CompositionEvent. Check this. So when you tackle the implementation of web in this issue, you should do in the following step:

  1. Include CompositionEvent from web_sys
  2. Implement functions about IME a. Get composing string with composition_event.data().unwrap() b. Fill cursor_start and cursor_end fields with composing_string.len() c. Send event to front

Here cursor_start and cursor_end mean where cursor is, and these data type is expressed in byte-wise.

NOTICE: As I said, cursor_start and cursor_end are byte-wise and you should NOT count characters.

KentaTheBugMaker commented 2 years ago

Ok I should do same as macOS.

KentaTheBugMaker commented 2 years ago

current status of web

Implemented.

KentaTheBugMaker commented 2 years ago

Please test #2173 . I dont have macos/ios device so i couldn't test it. all comment welcome for my work. Tested on

Firefox

moko256 commented 2 years ago

current status of Windows:

Implemented

PR: #2241

I tested this with other IME and it is ready to review.

KentaTheBugMaker commented 2 years ago

I have question about Web IME on mobile devices. Shall we open on screen keyboard if set_ime_allowed(false) ? if don't open onscreen keyboard almost impossible to input text by smart phone or tablets without physical keyboard. most of on screen keyboards pass input by IME . which is better ?

kchibisov commented 2 years ago

I'm pretty sure set_ime_allowed(false) should hide keyboard, since you want to hide keyboard in e.g. games and so on.

I'm pretty sure IME input should be processed along side OSK, since that's how it works on Wayland?

You can have keyboard input during IME input, that's fine, it just means that you can receive IME events, handing is up to you?

rib commented 2 years ago

Looking a bit into how Winit's IME abstraction can be supported on Android, one thing I've seen is that on Android their IME abstraction is also expected to be able to track a user "selection" which may be separate from the current composition region.

E.g. ref: https://developer.android.com/reference/android/view/inputmethod/InputConnection#cursors,-selections-and-compositions

In particular:

The composing region and the selection are completely independent of each other, and the IME may use them however they see fit.

More practically speaking; one likely way we could look at adding IME support to the Android backend would be via this Game Text Input library from the Android Game Developer Kit:

https://developer.android.com/games/agdk/add-support-for-text-input And here's the reference for that API: https://developer.android.com/reference/games/game-text-input/group/game-text-input

In particular that API includes this native GameTextInput_setState function that's used to communicate the state that the Java InputConnection wants, which takes a GameTextInputState struct that can include a selection that's separate from the compose region.

Similarly there is also a GameTextInput_getState and an event callback that also use this GameTextInputState struct that can report a selection that's separate from the compose region.

I'm wondering if it would be good if Winit's abstraction also allowed for tracking and updating any current user selection, separate from any current compose region to enable Android IMEs to get all the state they want.

Something else I'm wondering is what could be a good way of letting applications distinguish between platforms that move the IME UI via set_ime_position to follow wherever input focus is within the application (e.g. IMEs on desktop window systems) vs IMEs like soft keyboards on phones / tablets where it's the other way around and the apps generally need to make sure to update their UI to keep an input field that's the target for an IME within the current insets of the application that may be significantly reduced while the keyboard is visible.

Isopod commented 1 year ago

IMEs on Linux still don't work, as far as I can tell. I should say I'm not some kind of IME guru, I only use it for testing. I tested it with Anthy (Japanese IME) on X11. For the record, the IME works in most other programs, which includes Firefox, Chrome and all GTK and Qt applications.

I attempted to enable IME support like this:

window.set_ime_allowed(true);

This makes compose sequences (dead keys, e.g. ´+e = é) work, but not IMEs. In order to support IMEs on Linux, I think winit would need to implement IBus, since this is what most desktop environments (Gnome etc.) use these days.

kchibisov commented 1 year ago

Winit supports XIM and text_input_v3, dbus based implementation are out of scope.

GNOME works with both of them out of the box if you enable IME, same with kwin.

Be aware that once you enable ime support, you must handle the Ime events.

Isopod commented 1 year ago

At the moment I'm just dumping all events to the console. I'm not receiving any IME events. I just get ReceivedCharacter() events with latin letters.

kchibisov commented 1 year ago

if you use alacritty do you have IME input in it?

kchibisov commented 1 year ago

Also, make sure that what you're using is using XIM.

Isopod commented 1 year ago

I do happen to use alacritty and no, I don't have IME input there, either. When I manually set the input method to XIM by overriding environment variables (e.g. GTK_IM_MODULE=xim), then I don't have any IME input in those applications, either. But that's to be expected, isn't it? After all, I'm using IBus and not XIM. GTK and Qt have their own input modules for IBus, so that's not an issue.

Is IBus really supposed to be completely backward-compatible with XIM? Because that has never been the case for me , ~the compatibility seems to end at compose sequences/dead keys~ (*). I don't know if I'm doing anything wrong. I tried launching ibus with --xim, but it doesn't make any difference. I don't use any desktop environment, just i3.

Edit: (*) IBus might not have had any involvement in that at all.

kchibisov commented 1 year ago

Is IBus really supposed to be completely backward-compatible with XIM? Because that has never been the case for me, the compatibility seems to end at compose sequences/dead keys. I don't know if I'm doing anything wrong. I tried launching ibus with --xim, but it doesn't make any difference. I don't use any desktop environment, just i3.

I'd suggest to consult the ibus docs, I know that ibus the way setup on GNOME works without any issue with alacritty on X11 using XIM.

There're plenty IMEs supporting XIM (e.g. fcitx) and surely ibus can do that given that it works on stock fedora gnome(x11, wayland is also oob, but it has different story) for me.

notgull commented 1 year ago

dbus based implementation are out of scope.

Do you have a link to discussion on this topic? In my opinion, if IME is in scope then ibus should be in scope.

kchibisov commented 1 year ago

Do you have a link to discussion on this topic? In my opinion, if IME is in scope then ibus should be in scope.

Yet on Wayland you don't need it and having ibus where other IMEs exist is not an option (you have uim and so on as well, I never had ibus and use IME daily).

Isopod commented 1 year ago

Ok, I just tested it on a fresh Fedora 37 VM, can confirm that the IME works there on both Wayland and X11 (tested in alacritty). The UX isn't the best, but it appears to be usable. Not sure what Fedora is doing differently, it might just be an issue on my machine then.

pickfire commented 1 year ago

I used alacritty-git alacritty 0.13.0-dev (2df8f860) with IME on both x11 and wayland kde on arch linux and it works, and I have no idea what is XIM even though I heard about it, I don't think I touched that. But I remember on wayland some tweaks needs to be done, like set the virtual keyboard https://fcitx-im.org/wiki/FAQ#Non_Gtk.2FQt_Wayland_Application_.28Alacritty.2C_kitty.2C_etc.29.

Isopod commented 1 year ago

The issue I had might be specific to the Arch Linux ibus package. The PKGBUILD does not pass --enable-xim to configure. I recompiled the package with --enable-xim and it works now. Note that you also have to start ibus-daemon with -x or --xim.