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

Port input method and text input from rootston #4740

Merged
merged 6 commits into from Apr 4, 2020

Conversation

Leo1003
Copy link
Contributor

@Leo1003 Leo1003 commented Nov 20, 2019

@ddevault
Copy link
Member

Looks pretty good at a first glance. Are there any new clients we can look at for testing this, or are we still stuck with the basic demo clients?

@Leo1003
Copy link
Contributor Author

Leo1003 commented Nov 21, 2019

We currently use the input-method example in wlroots and wlchewing to test it. But the former is just an example and can only do some basic tests. The other one is an very early stage implementation using libchewing.

@SpencerMichaels
Copy link

What work remains before this PR can be merged? I am interested in getting this functionality into Sway and would be happy to help test or patch as needed. @Leo1003 My use case is also Chinese input methods (I use Pinyin, but am familiar with Zhuyin, although rusty); maybe there is something I could do to get wlchewing or another Chinese IME into a more usable state for this?

For testing, does ibus not support Wayland? It looks like it has support for V1 of this protocol, at least — see here.

@6A61736F6E206E61646572
Copy link

@SpencerMichaels For what it's worth, I cannot get ibus to run without xwayland, despite the package I installed it with building it with the --enable-wayland flag. The input itself does work in wayland containers though (because it's using the V1 protocol mentioned above?).

@xdavidwu
Copy link
Contributor

I faced some hangs but @Leo1003 could not reproduce, and I don't have time to look into it (yet). For testing, input-method example in wlroots can be used (with GTK_IM_MODULE=wayland on gtk3 apps, for example). If you want to test with wlchewing (a Chinese Zhuyin input method which is lacks some features but I would consider it feasible now for testing), you will also need keyboard grab support on wlroots. (Cursor may be drawn in wrong position on gtk though, because of a gtk bug)

We don't support input-method-unstable-v1. If ibus works, it is likely running under toolkit (gtk or qt) im modules, and if panels appears, it is by xwayland.

@David96
Copy link
Contributor

David96 commented Mar 22, 2020

Is there any reason this is not getting merged? I tried it with squeekboard and it makes the on screen keyboard reliably show up on focus of a text field, which is really useful for the tablet mode of a convertible.

@Leo1003
Copy link
Contributor Author

Leo1003 commented Mar 23, 2020

@David96 I think this PR is ready to merge, but we are making it better.
We are currently working on 7195ddc to eliminate some redundant call on seat_send_focus(), the already focused issue seems related to #2580. This issue may need some time to investigate.

@David96
Copy link
Contributor

David96 commented Mar 23, 2020

Alright, no rush and thanks for the update. As long as it hasn't been forgotten, I'm happy :)

@Leo1003
Copy link
Contributor Author

Leo1003 commented Mar 24, 2020

The final issue is resolved in #5130, I think this is ready for review.
P.S. The hangs in the early conversions are some "use after free" issues, and have been fixed 🔧

Copy link
Member

@ddevault ddevault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ddevault ddevault requested a review from emersion March 24, 2020 18:13
@dcz-purism
Copy link

Is there a reason for making text-input listeners per-text input? While I don't think this was leveraged so far, the relay was the choke point for detecting invalid states, like multiple text-inputs active at the same time. It could prevent broken calls to wlr_input_method_v2_send_activate if needed. Putting that in text-input handler might not be a good idea.

There is some overlap in lifetimes of text-inputs (esp. when switching clients) that should be carefully analyzed.

As a note not pertaining the actual patch, it seems that the text-input v3 protocol, and by extension, the input-method v2 were botched, and i would advise to have a way to drop them in the future in favor of the fixed version. Here's some background:

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/19

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/16

and, to a lesser extent, conditional on not having virtual-keyboard widely accepted: https://gitlab.freedesktop.org/wayland/wayland-protocols/issues/12

I would appreciate if those MRs could get more reviews, to justify releasing a fixed revision and moving on to the fix without having to implement the bad version first.

sway/input/text_input.c Outdated Show resolved Hide resolved
sway/input/text_input.c Outdated Show resolved Hide resolved
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good! Here are a few comments.

@emersion
Copy link
Member

emersion commented Apr 2, 2020

Can you also rebase against master?

@Leo1003
Copy link
Contributor Author

Leo1003 commented Apr 3, 2020

Ok, I will apply those changes and rebase ASAP.

@Leo1003
Copy link
Contributor Author

Leo1003 commented Apr 4, 2020

Rebased against master and changes are applied!

Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

@fosskers
Copy link

Sorry to dig up an old thread, but how is this actually used? The release for Sway 1.5 states:

Input method editors (IME) are now supported via the input-method and text-input protocols

but doesn't provide an example. Nor does it seem that anything was added to the manpages or Wiki.

Does anyone have a working example?

@fosskers
Copy link

fosskers commented Feb 28, 2021

Note to others. Adding the following to your sway config "just works":

exec_always fcitx5 -d --replace

Also make sure to set these environment variables somewhere:

# Fish shell
set -x GTK_IM_MODULE 'fcitx'
set -x QT_IM_MODULE 'fcitx'
set -x XMODIFIERS '@im=fcitx'

I can now type 日本語 and other things in my Wayland windows. Unfortunately it doesn't work (not even the language switching) if I'm focused on an alacritty window. (See alacritty/alacritty#2734)

Here are the Arch Linux packages I have installed for this:

fcitx5
fcitx5-configtool
fcitx5-gtk
fcitx5-mozc
fcitx5-qt

@12101111
Copy link

12101111 commented Mar 1, 2021

@fosskers You are still using the dbus frontend of fcitx5, run this command

$ dbus-send --session --type=method_call --print-reply --dest=org.fcitx.Fcitx5 /controller org.fcitx.Fcitx.Controller1.DebugInfo
method return time=1614562501.869255 sender=:1.7 -> destination=:1.39 serial=2629 reply_serial=2
   string "Group [wayland:] has 4 InputContext(s)
  IC [d2ab939f5c4d4f18b45bcf4df5df4fe8] program:firefox-wayland frontend:dbus cap:e001000012 focus:0
  IC [fd5d833c24d6448e875510a3871de9a4] program:firefox-wayland frontend:dbus cap:e001000012 focus:0
  IC [20e600d1d1354f8b87d7f5f38929a8a4] program:firefox-wayland frontend:dbus cap:e001000012 focus:0
  IC [7dcea1603b2f46bf9bf8f3139d8af51e] program:firefox-wayland frontend:dbus cap:e001000052 focus:0
Group [x11::0] has 2 InputContext(s)
  IC [8a8666158c624e6fba303472090d3c27] program:goldendict frontend:dbus cap:800072 focus:0
  IC [0b5c913b9cb74a9288218f8386d916b0] program:goldendict frontend:dbus cap:800072 focus:0
Input Context without group
"

There are 5 frontend provided by fcitx, GTK_IM_MODULE=fcitx select dbus frontend. With #5890, you can use waylandim frontend and type on alacritty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants