Skip to content

Conversation

@Shugyousha
Copy link

This is the current state of my attempt to add wayland input-method-v2 protocol support to ibus. The idea is to support both v1 and v2 of the protocol version at the same time.

It compiles but due to my lack of understanding of the protocol interactions between the input-method and the text-input protocols we end up with an endless loop when I run the text-input example from wlroots.

I'm not sure how the communication between the Wayland client code and the ibus engine works either which makes the implementation even more difficult...

Since I am not sure how much time I will be able to invest into this at the moment, I thought I could at least post what I have here. That way we can either find someone to properly finish the implementation or get some discussion about the next steps that are required going.

@fujiwarat
Copy link
Member

the input-method and the text-input protocols we end up with an endless loop when I run the text-input example from wlroots.

My understand is your patch still does not work.
Unfortunately Fedora does not provide v2 and I don't evaluate it yet.

@Shugyousha
Copy link
Author

Shugyousha commented Sep 9, 2020 via email

@fujiwarat
Copy link
Member

I think weston-editor can be a test application of Wayland protocol but not sure if text-input v3 can be used.
You may be better to find applications in Sway desktop which desktop may support it?

@Shugyousha
Copy link
Author

Shugyousha commented Sep 13, 2020 via email

@kchibisov
Copy link

Not sure if that could help, but there's a small example of input-method-v2 impl https://github.com/emersion/wlhangul.
As for text-input-v3 gedit handles it quite fine, at least it sets surrounding text, ime window position, etc.

@crocket
Copy link

crocket commented Sep 25, 2020

@Shugyousha
Copy link
Author

Shugyousha commented Sep 26, 2020 via email

@kchibisov
Copy link

I thought GTK used their own dbus protocol and not the Wayland one. I
could very well be wrong about this though.

You can clearly see that in WAYLAND_DEBUG=1 that something like gedit bindings and uses text-input-v3. Also, you may be required to use GTK_IM_MODULE=wayland for that, though, even on GNOME it was using text-input-v3.

This fixes some compilation issues.
Currently the activation-deactivation doesn't work properly for some
reason.
This seems to get things responsive again at least.
This seems to fix the surrounding text not being sent after each key
press!
@Shugyousha
Copy link
Author

Shugyousha commented Jan 30, 2021

I have adjusted the approach closer to how wlhangul is implementing these protocols. To do that I also had to implement handling of the virtual-keyboard-unstable-v1 protocol in the ibus client and apply the patches that implement this protocol in Sway for testing (as described in the wlhangul README ). Note that you will have to do the same to debug/test this code.

The current state is that regular keyboard input is just being passed unchanged through the ibus-wayland client and the protocol seems to be implemented the correct way (meaning I at least don't get any endless loop that continually increases the serial anymore). The two remaining major issues are:

  1. I cannot get ibus to actually handle the key presses that I pass to it. I don't know if that is because I'm missing some function call or if my approach is completely wrong.
  2. From what I understand I should pass the surrounding text sent by the text-input client(s) to the ibus core (the actual IME functionality) and send key presses that we get through the virtual keyboard grab to it as well. This is what I am trying to do in the code but IBus doesn't seem to want to handle the key presses that I send to it (see 1.). The Korean input method library that wlhangul is using returns the preedit and the commit text which wlhangul then sends back to the text-input client. I'm struggling to get ibus to give me the preedit and/or commit text in the same way because according to the ibus library documentation there is no synchronous way to get the preedit and/or commit strings that the ibus core is processing. IBus only provides you with signal callback functions that you can implement and these are called asynchronously which means we don't know when they are being called. If we don't know when they are being called, I don't think we are able to send these strings when we should according to the protocol spec.

Any info on how I can get ibus to handle the key presses that I send to it (see the ibus_input_context_process_key_event call in the code that I have added) and/or how I can get the commit/preedit strings (ideally in a synchronous way) from it would be highly appreciated!

@nilekurt
Copy link

Hello @Shugyousha,

I've also encountered this shortcoming of ibus and would like to contribute to a solution. I tried applying this pull request as a patch to the current master branch, but it seems like the files

  • virtual-keyboard-unstable-v1-protocol.h
  • virtual-keyboard-unstable-v1-protocol.c

are missing. I take it they are automatically generated from the XML-based protocol definitions. Shouldn't they, like the other code, also be included in the pull request?

Regards

@Shugyousha
Copy link
Author

Shugyousha commented Aug 29, 2021 via email

@Shugyousha
Copy link
Author

I think I have now included all the files needed to build this.

Let me know if you run into any other issues!

@Conan-Kudo
Copy link

Is there something that continues to hold this into draft status? Also, this should be rebased so it switches from Travis CI to GitHub Actions.

@Shugyousha
Copy link
Author

Shugyousha commented Jan 11, 2023 via email

@fujiwarat
Copy link
Member

I appreciate any contributors to work and test on the patches in the actual desktop likes wlroot desktop.
V2 is still not an official protocol in wayland-protocols.

@fujiwarat
Copy link
Member

Working on #2695 and closing this issue.

Do you know how to make a panel applet for IBus in the Sway session?
swaybar may be used but not sure.

@aidanharris
Copy link

It would be nice if the GTK one could be ported to use StatusNotifier (for the panel applet) and layer shell for the candidate choices overlay but I don't know how feasible that is:
https://github.com/wmww/gtk4-layer-shell

I currently use ibus in KDE Plasma which works remarkably well but the GTK applet and candidate choices overlay is all going over XWayland still. I really like ibus over fcitx but its Wayland support is definitely in need of some polish.

@eternal-sorrow
Copy link

input-method-v2 doesn't use layer-shell, it has its own popup surfaces

@Shugyousha
Copy link
Author

Working on #2695 and closing this issue.

Many thanks for your work on this!

Do you know how to make a panel applet for IBus in the Sway session? swaybar may be used but not sure.

According to https://man.archlinux.org/man/swaybar-protocol.7, swaybar doesn't support images (or interaction?). It would be possible to show the current state of ibus in text form though, I assume.

An alternative would be Waybar that also allows for images to be shown.

Not sure an applet is desirable for a tiling WM like Sway. If there is demand for it, I would assume that somebody will come up with a Waybar module or something else for ibus :P

@eternal-sorrow
Copy link

Do you know how to make a panel applet for IBus in the Sway session?

SNI tray icon is not acceptable for some reason?

@fujiwarat
Copy link
Member

Thank you for the comments.
There are two problems at present.

V1 works with zwp_input_panel_surface_v1_set_overlay_panel() and V2 works with zwp_input_method_v2_get_input_popup_surface() but whenever I close the IBus candidate popup window, IBus gets
the disconnection from the Wayland display with an error of "surface was destroyed before its role object" in V2 and I'm not sure how to destroy the role object.

[4205273.958] {Default Queue} zwp_input_method_keyboard_grab_v2#40.key(2966, 1111139, 1, 0)
[4205275.639] {Default Queue}  -> zwp_virtual_keyboard_v1#35.key(1111139, 1, 0)
[4205276.643] {Default Queue}  -> wl_surface#44.destroy()
[4205276.687] {Default Queue}  -> wl_buffer#47.destroy()
[4205276.694] {Default Queue}  -> wl_shm_pool#46.destroy()
[4205282.894] {Display Queue} wl_display#1.error(nil, 4, "surface was destroyed before its role object")
Gdk-Message: Error 71 (Protocol error) dispatching to Wayland display.

I guess Sway does not work with the embedded GTK tray icon yet.
swaywm/sway#6249
I enabled the IBus floating panel in Sway because of no tray for external GTK icons but the window is not arranged correctly into the Sway screen division.
gsettings set org.freedesktop.ibus.panel show 2

I'm evaluating swaybar, waybar, nwg-panel, wlclock and gtk4-layer-shell.

It would be nice if the GTK one could be ported to use StatusNotifier (for the panel applet) and layer shell for the candidate choices overlay but I don't know how feasible that is:

@aidanharris Thank you. Maybe using gtk4-layer-shell is a good option, which is packaged in Fedora.

SNI tray icon is not acceptable for some reason?

@eternal-sorrow Thank you. I see sni-qt is packaged in Fedora.

@alebastr
Copy link

V1 works with zwp_input_panel_surface_v1_set_overlay_panel() and V2 works with zwp_input_method_v2_get_input_popup_surface() but whenever I close the IBus candidate popup window, IBus gets
the disconnection from the Wayland display with an error of "surface was destroyed before its role object" in V2 and I'm not sure how to destroy the role object.

zwp_input_popup_surface_v2 is a surface role, so the error tells you to ensure that you call zwp_input_popup_surface_v2_destroy before wl_surface_destroy.

I guess Sway does not work with the embedded GTK tray icon yet.
swaywm/sway#6249
I enabled the IBus floating panel in Sway because of no tray for external GTK icons but the window is not arranged correctly into the Sway screen division.

The existing appindicator tray icon code works pretty well with Waybar. Just needed to convince the UI that it's running under KDE: XDG_CURRENT_DESKTOP=KDE /usr/libexec/ibus-ui-gtk3.

XDG_CURRENT_DESKTOP detection is kind of unreliable though, you'll end up checking for every compositor under the sun in Panel::is_kde(). At the very least, it should check for sway and wlroots.

@aidanharris
Copy link

The existing indicator uses Xembed, it's not the best but it does still work. If there's any chance at all that it could be ported to StatusNotifier that would be great but I'm not the one doing the work.

Swaybar should support StatusNotifier too as far as I know and it's pretty common for most desktops that use a tray to support this too.

fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 16, 2024
fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 16, 2024
- Call 'realize-surface' signal with null before CandidatePanel and Switcher
  are hidden to destroy input_popup_surface in Wayland input-method V2.
- Indicator.service_get_property() returns Error if IconPixmap is null.

BUG=ibus#2182
BUG=ibus#2256
@fujiwarat
Copy link
Member

zwp_input_popup_surface_v2 is a surface role, so the error tells you to ensure that you callzwp_input_popup_surface_v2_destroy before wl_surface_destroy.

@alebastr Thank you very much. I understand ibus-wayland calls wl_surface_destroy() and fix the issue.

XDG_CURRENT_DESKTOP detection is kind of unreliable though, you'll end up checking for every compositor under the sun in Panel::is_kde(). At the very least, it should check for sway and wlroots

Thank you. I missed the detection and I fixed it for sway at the moment.
You're right. I will think the better detection after I evaluate other Wayland desktop sessions.
Fedora wlroots package just provides libwlroots.so and I need to find the instance of the desktop session to set XDG_CURRENT_DESKTOP=wlroots.

@fujiwarat
Copy link
Member

I confirmed swaybar can show the IBus panel icon but swaybar does not show the panel menu, waybar can show the panel menu.
sway2

@aidanharris I understand you mean swaybar does not support the StatusNotifier yet.

Seems the task area is blue in waybar by default and I'd like to customize the color since IBus panel strings are also blue.

fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 16, 2024
fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 16, 2024
- Call 'realize-surface' signal with null before CandidatePanel and Switcher
  are hidden to destroy input_popup_surface in Wayland input-method V2.
- Indicator.service_get_property() returns Error if IconPixmap is null.

BUG=ibus#2182
BUG=ibus#2256
fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 22, 2024
…yland

- Share codes between the Wayland input-method protocol version 1 and 2
  as much as possible.

- zwp_input_method_v2.commit() needs to be called when a string is
  committed or a preedit and surrounding text is updated [1].

- zwp_input_popup_surface_v2 should be destroyed and cleared the pointer
  before wl_surface.destroy() is called due to `WAYLAND_DEBUG=1`.

- zwp_input_method_keyboard_grab_v2_listener.key() can be called
  even if zwp_input_method_v2 is not activated and
  zwp_input_method_v2.commit_string() should not be used in such cases
  likes when non-supported applications(xterm, cosmic-term) are
  activated but zwp_virtual_keyboard_v1.key() just can be used instead.

[1] https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/protocol/input-method-unstable-v2.xml?ref_type=heads#L283

BUG=ibus#2182
BUG=ibus#2256
fujiwarat added a commit to fujiwarat/ibus that referenced this pull request Nov 22, 2024
- Call 'realize-surface' signal with null before CandidatePanel and
  Switcher are hidden to destroy input_popup_surface in Wayland
  input-method V2.

- Indicator.service_get_property() returns Error if IconPixmap is null.

- Replace is_kde() with is_indicator() to support the general Wayland
  input-method version 2 desktop environments. The order of
  init_settings(), set_version(), check_wayland() and is_indicator() is
  important to set
  m_settings_general, inited_engines_order, m_is_indicator,
  init_indicator().

BUG=ibus#2182
BUG=ibus#2256
@fujiwarat
Copy link
Member

I added some patches in https://github.com/fujiwarat/ibus/commits/wayland-v2 to work with XIM and GTK2 applications in Wayland. Since implementing wl_surface is complicated for me, I use the raw GTK popup window without the GDK custom surface. Now IBus panel manages two popup dialogs for the lookup window, one is implements the Wayland panel protocol and another is a raw popup and I confirmed the memory usage is not changed.

@fujiwarat
Copy link
Member

I cannot find /usr/share/vala-0.56/vapi/wayland-client.vapi in valac-0.56-vapi package in Ubuntu Jammy.
https://github.com/ibus/ibus/actions/runs/12728634486/job/35479419810?pr=2695

Do you have any ideas?

fujiwarat added a commit that referenced this pull request Jan 12, 2025
…yland

- Share codes between the Wayland input-method protocol version 1 and 2
  as much as possible.

- zwp_input_method_v2.commit() needs to be called when a string is
  committed or a preedit and surrounding text is updated [1].

- zwp_input_popup_surface_v2 should be destroyed and cleared the pointer
  before wl_surface.destroy() is called due to `WAYLAND_DEBUG=1`.

- zwp_input_method_keyboard_grab_v2_listener.key() can be called
  even if zwp_input_method_v2 is not activated and
  zwp_input_method_v2.commit_string() should not be used in such cases
  likes when non-supported applications(xterm, cosmic-term) are
  activated but zwp_virtual_keyboard_v1.key() just can be used instead.

[1] https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/protocol/input-method-unstable-v2.xml?ref_type=heads#L283

BUG=#2182
BUG=#2256
fujiwarat added a commit that referenced this pull request Jan 12, 2025
- Call 'realize-surface' signal with null before CandidatePanel and
  Switcher are hidden to destroy input_popup_surface in Wayland
  input-method V2.

- Indicator.service_get_property() returns Error if IconPixmap is null.

- Replace is_kde() with is_indicator() to support the general Wayland
  input-method version 2 desktop environments. The order of
  init_settings(), set_version(), check_wayland() and is_indicator() is
  important to set
  m_settings_general, inited_engines_order, m_is_indicator,
  init_indicator().

BUG=#2182
BUG=#2256
@fujiwarat fujiwarat closed this Jan 13, 2025
@fujiwarat fujiwarat modified the milestones: 1.5.31, 1.5.32 Jan 13, 2025
@Shugyousha Shugyousha deleted the input_method_v2 branch January 18, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants