Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support input method editors for X11 #250

Closed
promisedlandt opened this issue Aug 3, 2020 · 45 comments
Closed

Support input method editors for X11 #250

promisedlandt opened this issue Aug 3, 2020 · 45 comments
Labels
enhancement New feature or request fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds. Linux Issue applies to Linux X11

Comments

@promisedlandt
Copy link

Describe the bug

Hello Wez,

I use UIM for switching the input method to Korean. I have bound it to ctrl + alt + space.
Works fine in my current terminal emulator (terminator), but not in wezterm.
I only get a log message saying

2020-08-03T10:19:42.583Z ERROR wezterm_term::terminalstate > Ding! (this is the bell)

I have no idea whether this is an issue with UIM or wezterm, but I found this and thought it might be related:

FIXME in shift-space now emits space maybe?

My guess (without looking at any code and / understanding how wezterm or UIM work) would be that wezterm swallows the key presses.

Environment (please complete the following information):

  • OS: Linux (Debian Sid)
  • Version: wezterm 20200718-095447-d2315640

To Reproduce

Steps to reproduce the behavior.

Please include as much information as possible that can help to reproduce and
understand the issue; some pointers and suggestions are included here in this
template. You are empowered to include more or less information than is asked
for here!

Configuration

Happens without a config file. Also happens with use_ime = true, but from the text that appears to only affect MacOS anywat.

Expected behavior

When I press ctrl + alt + space, UIM receives that key combination so it can switch my IM.

Screenshots

Session Recording

Additional context

Add any other context about the problem here.

@promisedlandt promisedlandt added the bug Something isn't working label Aug 3, 2020
@Cycatz
Copy link

Cycatz commented Aug 6, 2020

Same issue here, I use fcitx but I can't switch my IME with ctrl + space.

@wez
Copy link
Owner

wez commented Aug 9, 2020

Can you tell me how to configure the UIM? I'd like to try to reproduce this, but I'm not sure how to set it up.

@promisedlandt
Copy link
Author

promisedlandt commented Aug 9, 2020

Assuming you're on Linux, UIM should be in your package manager. You'd need the Korean UIM package - should be something like uim-byeoru.

The config is just text files. I have attached mine, it basically disables most stuff and switches to Korean on ctrl + alt + space, you can get back to normal with the same key combination, or just by pressing escape.

uim_config.tar.gz

mkdir -p ~/.uim.d/customs
tar xzvf uim_config.tar.gz -C ~/.uim.d/customs

Before you can use it, you need to enable it, e.g. in ~/.xprofile:

export GTK_IM_MODULE=uim
export QT_IM_MODULE=uim
uim-xim &
export XMODIFIERS=@im=uim

Unfortunately this appears to be something that requires a restart before it works.

Expected behaviour when Korean IM is enabled: all alphabetical keys are replaced with Korean letters, and they get "composed" into syllables.
Here's an example you can get by pressing the US layout keys dkssud:
korean_example

Thanks for having a look, please let me know if I forgot something and the above steps didn't get you to a working state.

@wez
Copy link
Owner

wez commented Aug 10, 2020

Thanks for the info!

I've done a bit of reading and it appears that XCreateIC and XOpenIM are the libX11 functions to enable input method editor support under X. Those don't appear to have counterparts in the more modern XCB. I'll have to do a bit of research to figure out how this is supposed to interact with the xkeyboard stuff that wezterm currently targets.

@wez wez added Linux Issue applies to Linux X11 enhancement New feature or request and removed bug Something isn't working labels Aug 11, 2020
@wez
Copy link
Owner

wez commented Aug 11, 2020

I'm classifying this as an enhancement as we don't currently have support for IME on linux

@wez wez changed the title ctrl + alt + space does not change my input method Support input method editors for X11 Oct 15, 2020
@wez
Copy link
Owner

wez commented Oct 15, 2020

I looked into this a bit more; there are still things to figure out before any meaningful support can be added:

  • The old XLib APIs make heavy use of variadic C arguments and callbacks, which makes them awkward to use from Rust and from an XCB style event loop.
  • fcitx appears to have made an XCB implementation of the XIM protocol (https://github.com/fcitx/xcb-imdkit) which might be better to use for this sort of thing, but that means pulling in another C based dependency, and then figuring out how to bind to it from Rust. If that library is basically just a few C functions then it may not be too bad (the test code here doesn't look awful: https://github.com/fcitx/xcb-imdkit/blob/master/test/test_client.c)

@wez wez mentioned this issue Dec 7, 2020
@quark-zju
Copy link
Contributor

quark-zju commented Apr 9, 2021

From my experience, xim is too limiting and I never used it. gtkim has advanced features such as embedding the candidate text and is the main backend I have used. IBus is another solid IME framework on Linux, which lives in the gtk-ish eco-system. In the past I wrote an IME in IBus using Vala. I would suggest targeting gtkim if possible.

@wez
Copy link
Owner

wez commented Apr 9, 2021

I don't know if wezterm can use gtkim without effectively being rewritten as a separate GTK frontend, which is not something that I want to do.

@asdf8dfafjk
Copy link

I don't know much about X, and wezterm and alacritty probably have different underlying goals but if it helps you get an inspiration- here's the PR in alacritty that added this support- https://github.com/alacritty/alacritty/pull/691/files

@H-M-H
Copy link
Contributor

H-M-H commented Aug 14, 2021

Alacritty uses winit, which just directly calls XOpenIM, XCreateIC, etc from xlib: https://github.com/rust-windowing/winit/tree/master/src/platform_impl/linux/x11/ime. If I am reading things right, it looks like a quick and dirty solution is to just copy this ime code and the code for their XConnection as it seems pretty independent from the rest of the code base and then all that needs to be done is some initialization and updating the cursor position with https://github.com/rust-windowing/winit/blob/b87757c5521cef9cae0b73c671398ed33f37de59/src/platform_impl/linux/x11/ime/mod.rs#L146

@wez
Copy link
Owner

wez commented Aug 14, 2021

@H-M-H do you want to try making a PR for wezterm that does that?

@H-M-H
Copy link
Contributor

H-M-H commented Aug 16, 2021

Turns out the methods XOpenIM etc. need xlib to have ownership over the event queue, which is not the case as wezterm is built on xcb which means calling XOpenIM just blocks.
After realizing that I gave https://github.com/fcitx/xcb-imdkit a try and it's mostly working now, that is the IM Editor opens up and processing events is working. Something that still needs to be done is actually handing the result string after composition to wezterm internally as well as setting the correct cursor position. @wez I'd be glad about some hints regarding those points.

Have a look at https://github.com/H-M-H/wezterm/tree/ime for the current progress.

@wez
Copy link
Owner

wez commented Aug 16, 2021

@H-M-H thanks for diving in! I'm excited to look through https://github.com/H-M-H/xcb-imdkit-rs and the integration in wezterm when I done with work later today!

@H-M-H
Copy link
Contributor

H-M-H commented Aug 18, 2021

Alright, I think I am mostly happy now with https://github.com/H-M-H/xcb-imdkit-rs and will probably publish it on crates.io soonish.
@wez I found a way to forward the text generated by the IME to wezterm: H-M-H@3f38ae9 does this look about right? Also, I still have no clue how to get the cursor position, so the IME window can be placed correctly.

@wez
Copy link
Owner

wez commented Aug 18, 2021

I'll check it out! Re: IME positioning, here's the equivalent thing in the Windows version:

https://github.com/wez/wezterm/blob/main/window/src/os/windows/window.rs#L506-L509

which is called by:

https://github.com/wez/wezterm/blob/main/window/src/os/windows/window.rs#L657-L662

so you'd want to do something similar by inserting a set_text_cursor_position impl around here:

https://github.com/wez/wezterm/blob/main/window/src/os/x11/window.rs#L993

@wez
Copy link
Owner

wez commented Aug 18, 2021

For injecting the composed text, the Windows version does this:

https://github.com/wez/wezterm/blob/main/window/src/os/windows/window.rs#L1227-L1237

@H-M-H
Copy link
Contributor

H-M-H commented Aug 18, 2021

Thanks for the hints! I got it working now. The IME is correctly positioned now and works fine with multiple windows. I changed the code for text injection to mirror what is done on Windows.

On thing that is bugging me is that the IME is not closed once wezterm exits, this may be a shortcoming of xcb-imdkit though fcitx/xcb-imdkit#10.

Apart from that all that needs to be done is some more testing I guess.

@wez
Copy link
Owner

wez commented Aug 18, 2021

I think one final functionality question is: would a user ever want to disable using the IME? On macOS we have a use_ime config option to prevent using it, because it does make some terminal typing interactions more painful with the IME present.

https://wezfurlong.org/wezterm/config/keys.html?highlight=use_ime#macos-and-the-input-method-editor-ime

Respecting this option should be as simple as:

if config::configuration().use_ime && self.ime.borrow_mut().process_event(event) {

@wez
Copy link
Owner

wez commented Aug 18, 2021

I got this just now when trying to compile your wezterm branch:

The following warnings were emitted during compilation:

warning: deps/xcb-imdkit/src/imclient.c:21:10: fatal error: xcb/xcb_aux.h: No such file or directory
warning:    21 | #include <xcb/xcb_aux.h>
warning:       |          ^~~~~~~~~~~~~~~
warning: compilation terminated.

error: failed to run custom build command for `xcb-imdkit v0.1.0 (https://github.com/H-M-H/xcb-imdkit-rs#01e21e33)`

you'll need to update the get-deps script to install xcb-util-devel for the rpm based systems, and you might need to add the equivalent for the others in there too in order for the CI to all pass.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 18, 2021

If there is an option to toggle IME support it's probably confusing if it does not work, so I changed the line as you proposed. I hopefully fixed get-deps as well, lets see if it works: #1043

@wez
Copy link
Owner

wez commented Aug 18, 2021

I gave it a quick try, and it looks good! (caveat: I have no idea what I've typed it, other than it rendered as Chinese glyphs!)

Screenshot from 2021-08-18 08-04-16

There's something a bit "floaty" or laggy about keyboard input with the IME changes included; it can sometimes take several seconds for the input to show up even when the IME candidate window is not open. I feel like there might be something missing in the integration!

@H-M-H
Copy link
Contributor

H-M-H commented Aug 18, 2021

I think I figured what's causing this, apparently updating the IME position is pretty expensive, I will have to think about a way around this.

wez added a commit that referenced this issue Aug 20, 2021
@wez wez added the fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds. label Aug 20, 2021
@wez
Copy link
Owner

wez commented Aug 20, 2021

Many thanks to @H-M-H; this is now available in the nightly builds if you set use_ime = true in your config.
We can look at making that default to true for X11 as a follow-up. Not sure if there is a consequence to that.

@promisedlandt
Copy link
Author

Tested on wezterm compiled from source with rust 1.54.0, sha 40c8fb1

Input method: fcitx4

wezterm config:

local wezterm = require 'wezterm';

return {
  font_size = 18.0,
  font = wezterm.font_with_fallback({
    "Cousine",
    "Noto Sans CJK KR",
  }),

  enable_tab_bar = false,
  use_ime = true,
}

IM popup instead of in-place

Bottom window: terminator 2.1.0

wezterm im popup

wezterm does not change the letters in-place, it uses a popup. I /think/ this might be the XIM default, and terminator provides the nice in-place editing because it's a GTK application, meaning this would not be improvable.

Default font

wezterm im default font

After removing the "Noto Sans CJK KR" line from the config, the default Korean font looks horrible (imagine your font being cursive). No idea if that fallback font is specific to my system though.
Also, after program start, it swallowed my first word input, and the words have some placeholder glyph for a second. The placeholders are not shown again afterwards, and no words are swallowed.
Neither of those things happen with the Noto font enabled.

Adding a line to the font setup documentation to the ime documention seems like a good idea.
Could possibly bundle Noto Sans CJK if it's not too big, it has support for simplified and traditional Chinese, Japanese, and Korean. I have no idea whether there might be a better default.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 20, 2021

wezterm does not change the letters in-place, it uses a popup. I /think/ this might be the XIM default, and terminator provides the nice in-place editing because it's a GTK application, meaning this would not be improvable.

Actually this is possible to implement. The problem is that interfacing with wezterms internals gets more complicated in that case. The XIM library provides callbacks on changes of the IME text and these would need to be handled.

About the fonts: I have not touched any font related code here, so this should perhaps be a separate issue.

@asdf8dfafjk
Copy link

wezterm does not change the letters in-place, it uses a popup. I /think/ this might be the XIM default, and terminator provides the nice in-place editing because it's a GTK application, meaning this would not be improvable.

I don't think it's fair to say "nice in-place editing" without qualification. Of the two language sets I'm familiar with (Indic languages and Hanji/Kanji languages, both 1B+ speakers), the former would work better with in-place but the latter with popup.

I'd much prefer popup if I have only one option

@asdf8dfafjk
Copy link

Also! Thank you guys! This was a much needed feature :-)

@wez
Copy link
Owner

wez commented Aug 20, 2021

Re: fonts, when a glyph can't be resolved from the fonts listed directly in your config, wezterm:

  • Substitutes a glyph from the built-in Last Resort font so that the render doesn't block
  • Asynchronously asks fontconfig to provide a list of fonts that have the matching set of codepoints that were missing
  • if sort_fallback_fonts_by_coverage = true in your config (default is false), ranks the returned list of fonts by descending amount of coverage. Otherwise, the order returned from fontconfig is used.
  • Add the fonts to the list of fallback fonts to avoid repeating the search a second time
  • Re-renders to pick up the newly found fonts

That search process can take a visible number of milliseconds to complete the first time around, which is why we don't want to do that in the render pass itself (it would lock up the UI while it was resolving), and is why you might see a flash with a placeholder glyph initially.

In terms of the quality of the matching: I don't know if there's a better way to ask fontconfig for a better looking font for your script than we're using. You may want to experiment with sort_fallback_fonts_by_coverage = true to see if it is better (most likely: differently unsatisfying).

@H-M-H
Copy link
Contributor

H-M-H commented Aug 20, 2021

and the words have some placeholder glyph for a second

I also found the font loading to be very slow, I am experimenting with improvements here: https://github.com/H-M-H/wezterm/tree/font_perf
So far I could achieve a ~12 X speedup.

@wez
Copy link
Owner

wez commented Aug 20, 2021

wezterm does not change the letters in-place, it uses a popup. I /think/ this might be the XIM default, and terminator provides the nice in-place editing because it's a GTK application, meaning this would not be improvable.

Actually this is possible to implement. The problem is that interfacing with wezterms internals gets more complicated in that case. The XIM library provides callbacks on changes of the IME text and these would need to be handled.

I'm definitely open to seeing this as an option for the IME. It'll be a little bit indirect to hook up because our window crate broadly speaking doesn't know how to actually render things in the window! If we can record the set of of drawing instructions from the IME (eg: "render this text in this font at (x,y)", and/or "render this bitmap at (x,y)"), and expose that via say, window::WindowEvent::IMERenderInfo {..} then it will be fairly straight forward for the gui to know what the current state is and then to render it in a final pass of the renderer.

That sort of channel from the IME -> renderer would be partially aligned with #686 and #688

@promisedlandt
Copy link
Author

I don't think it's fair to say "nice in-place editing" without qualification. Of the two language sets I'm familiar with (Indic languages and Hanji/Kanji languages, both 1B+ speakers), the former would work better with in-place but the latter with popup.

I'd much prefer popup if I have only one option

You are correct, I should not have assumed what works best for me. Is there any way for your input method editor to specify which you prefer? Or is it always up to the program you're currently using?

  • Substitutes a glyph from the built-in Last Resort font so that the render doesn't block
  • Asynchronously asks fontconfig to provide a list of fonts that have the matching set of codepoints that were missing
  • if sort_fallback_fonts_by_coverage = true in your config (default is false), ranks the returned list of fonts by descending amount of coverage. Otherwise, the order returned from fontconfig is used.
  • Add the fonts to the list of fallback fonts to avoid repeating the search a second time
  • Re-renders to pick up the newly found fonts

That sounds like it shouldn't swallow any input though, but that's what happened to me in the gif.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 21, 2021

Is there any way for your input method editor to specify which you prefer? Or is it always up to the program you're currently using?

The way it works is that you can ask the installed IME for text editing events and typically the IME will only show a completion list (if applicable) in this case and it is up to the program to display the currently typed text. If you don't request those events the IME will handle text editing completely and has to as the program can not. So in conclusion while it may be possible for the IME to do it's own thing, it is bound by what the program can handle and I don't think you can configure this.

@SuperSandro2000
Copy link
Contributor

@H-M-H 6404099 broke building for me

error: failed to run custom build command for `xcb-imdkit v0.1.0 (https://github.com/H-M-H/xcb-imdkit-rs#3cb94086)`

Caused by:
process didn't exit successfully: `/build/source/target/release/build/xcb-imdkit-39fa9a39a5450f67/build-script-build` (exit code: 101)
--- stdout
cargo:rerun-if-changed=deps/build.sh
cargo:rerun-if-changed=xcb-imdkit.h
cargo:rustc-link-lib=xcb
cargo:rustc-link-lib=xcb-util

--- stderr
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
thread 'main' panicked at 'Initializing xcb-imdkit submodule failed!', /build/wezterm-20210819-213146-40c8fb1f-vendor.tar.gz/xcb-imdkit/build.rs:53:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

@H-M-H
Copy link
Contributor

H-M-H commented Aug 22, 2021

Please describe your build environment, xcb-imdkit-rs uses a git submodule for the wrapped C library. For some reason this module hasn't been initialized and thus my build script tried to run git manually but that failed as well? Are you perhaps doing some trickery with building across different filesystems?

If so try setting GIT_DISCOVERY_ACROSS_FILESYSTEM=1 before running cargo.

@SuperSandro2000
Copy link
Contributor

Please describe your build environment

nixpkgs, I am also the upstream maintainer of wezterm there.

xcb-imdkit-rs uses a git submodule for the wrapped C library

That is not going to work if it if downloaded at runtime. This is also a big no no for most distros. You can't just download random things during build time.

Trying to fix it at #1064.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 22, 2021

That is not going to work if it if downloaded at runtime. This is also a big no no for most distros.

I am also not 100% happy with it, but what are the alternatives? On most distributions the system xcb-imdkit C library is just too old and copying said library into my project seems disingenuous as well.

Also thanks for maintaining the package!

@SuperSandro2000
Copy link
Contributor

Can rust download a release tarball that includes the submodule? I think that would work around this or wezterm needs to add the submodule to the third-party directory and download it while downloading the source.

Maybe we can deactivate the feature on systems that do not have a new enough library?

@wez
Copy link
Owner

wez commented Aug 22, 2021

Plenty of crates use a git submodule for this sort of thing. I'm not really a fan of using the system library as I think that will be less likely to be available and will pose more of a build headache. Instead, I think that having the crate use a submodule is fine, but then: publish that crate to crates.io with the submodule files included. Then we can change wezterm to use the published crate rather than building from the git repo.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 22, 2021

Sounds good, I found and fixed some more minor bugs and want to test things before publishing to crates.io. These include:

  • Also, after program start, it swallowed my first word input

  • Wrong IME position on first input.

@H-M-H
Copy link
Contributor

H-M-H commented Aug 23, 2021

Alright, so far it's working fine, I have now published the crate at: https://crates.io/crates/xcb-imdkit

wez added a commit that referenced this issue Aug 23, 2021
@wez
Copy link
Owner

wez commented Aug 23, 2021

main is now pointing at the crates.io version.

wez added a commit that referenced this issue Sep 6, 2021
and add a changelog entry!

refs: #250
@wez
Copy link
Owner

wez commented Sep 6, 2021

@H-M-H: I pulled in your font_perf changes; thanks!

@wez
Copy link
Owner

wez commented Sep 9, 2021

I'm closing this issue, as the heart of IME for X11 is done, and this issue is pretty long. Please open separate issue(s) if there are things that need further attention!

@wez wez closed this as completed Sep 9, 2021
@ModProg
Copy link
Contributor

ModProg commented Oct 2, 2021

Did anyone get this working with ibus on linux?
Nether mind, I'm stupid, you need to set use_ime = true.

Maybe we can add fcitx and ibus in the documentation as keywords to find this 🤔

Because this sounds like it only addresses MacOs:
https://wezfurlong.org/wezterm/config/keys.html?highlight=ime#macos-and-the-input-method-editor-ime

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds. Linux Issue applies to Linux X11
Projects
None yet
Development

No branches or pull requests

8 participants