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

Implement input method keyboard grab #1160

Closed
ssfdust opened this issue Apr 25, 2021 · 13 comments · Fixed by #2053
Closed

Implement input method keyboard grab #1160

ssfdust opened this issue Apr 25, 2021 · 13 comments · Fixed by #2053

Comments

@ssfdust
Copy link
Contributor

ssfdust commented Apr 25, 2021

Please describe what you want Wayfire to do.

Could wayfire implement the input-method-unstable-v2 keyboard grab like swaywm/sway#4932, I found something like swaywm/sway#4740 has been implemented in https://github.com/WayfireWM/wayfire/blob/master/src/core/seat/input-method-relay.cpp, it seems that it won't be too complex.

@Piroro-hs
Copy link

+1 as this is needed for IMEs to function on wayland native applications.

@rico256-cn
Copy link

+1

@lilydjwg
Copy link
Contributor

lilydjwg commented Nov 1, 2023

FYI I've implemented this on my branch. I'm going to also implement the popup (at least) so that fcitx5 is usable.

@ammen99
Copy link
Member

ammen99 commented Nov 1, 2023

@lilydjwg I looked briefly at your latest patch, if you are interested, I think it can be merged upstream with few modifications. Mostly I would like to keep it confined to core-impl.hpp, and then you can use wf::get_core_impl() instead of wf::get_core(), this way the relay remains a private detail of core and is not exposed to the public API.

Also btw I don't think you need an interface (currently you have an interface and then an _impl), instead, you could just include the relay header from keyboard.cpp ...

lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Nov 2, 2023
@lilydjwg
Copy link
Contributor

lilydjwg commented Nov 2, 2023

Thanks for your advice, I've updated accordingly. I didn't know about wf::get_core_impl().

I'm going to implement the popup thing before sending a pull request, because it isn't very usable for fcitx5 without that.

@lilydjwg
Copy link
Contributor

Hi @ammen99, I'm trying to implement the popup feature, but got lost in method calls. I need your help. Specifically I have the following questions:

  • How should I subclass wf::view_interface_t and add my popup to the scene and move it around? Is there any simple example?
  • I have a wlr_surface from wlroots, how can I find its position?

@ammen99
Copy link
Member

ammen99 commented Nov 18, 2023

Hi @lilydjwg

  • I have a wlr_surface from wlroots, how can I find its position?

wlr_surfaces do not have a position. The compositor is in charge of deciding where to put it - namely, the view class does that.

  • How should I subclass wf::view_interface_t and add my popup to the scene and move it around? Is there any simple example?

The simplest example that comes to mind is https://github.com/WayfireWM/wayfire/blob/master/src/view/compositor-view.cpp#L101

A few notes to guide you (feel free to ask if something is unclear): The color rect view implements its own root node and the corresponding render instances, because it needs to render in a custom way (i.e colors). For implementing a wlr_surface-backed view, you can use a translation node (wayfire/unstable/translation-node.hpp) as a root node. This will allow you to set the position of the view. You can then create a wlr_surface_node (wayfire/unstable/wlr-surface-node.hpp) from your wlr_surface and simply add it as a child of the translation node.

Do not forget to emit the map and unmap signals when the view is being created or before it is destroyed (otherwise you may get crashes).

@lilydjwg
Copy link
Contributor

OK, I've found the view object. How should I get the offset on screen so that I can put my popup view at the right place? .to_global() seems to not do anything on the point. This doesn't work:

    auto node = view->get_surface_root_node()->parent();
    while (node != view->get_transformed_node().get())
    {
        popup_offset = node->to_global(popup_offset);
        node = node->parent();
    }

@soreau
Copy link
Member

soreau commented Nov 18, 2023

OK, I've found the view object. How should I get the offset on screen so that I can put my popup view at the right place? .to_global() seems to not do anything on the point. This doesn't work:

    auto node = view->get_surface_root_node()->parent();
    while (node != view->get_transformed_node().get())
    {
        popup_offset = node->to_global(popup_offset);
        node = node->parent();
    }

Since you already have the view, you can use view->get_geometry() to get the view geometry.

@ammen99
Copy link
Member

ammen99 commented Nov 18, 2023

OK, I've found the view object. How should I get the offset on screen so that I can put my popup view at the right place? .to_global() seems to not do anything on the point. This doesn't work:

I am not sure how the positioning works exactly. But I assume you will get some information like "popup should be at offset x,y from the main surface A". Is this true? In such a case, you can use wl_surface_to_wayfire_view() to get the view A, get its position with view->get_geometry(), add offset and set the result as the offset for your input popup's main surface.

lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Nov 19, 2023
@lilydjwg
Copy link
Contributor

Thanks! It works now. I've pushed to my branch, and will do a few more tests (multi-monitor, mixed scale, etc) before sending a pull request.

lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Nov 20, 2023
@ssfdust
Copy link
Contributor Author

ssfdust commented Nov 22, 2023

Thanks, great work!!! I have tested it in several scenes with kitty terminal:

  • Input with fcitx5 in a single Wayland window.
  • Input with fcitx5 in different windows on different virtual desktops.
  • Input with fcitx5 in minimized and maximized windows on different virtual desktops.
  • Input with fcitx5 after closing multiple windows and reopening them.

In the case of multiple windows, switching focus back and forth between windows may cause fcitx5 to fail in switching input methods.

To reproduce the issue, follow these steps:

  • Open three Kitty windows on three different virtual desktop. (Maybe two windows are enough)
  • In the first window, switch to the Chinese input method and enter some text. Then, use Shift to switch to the English input method.
  • Switch to the window on the second virtual desktop. You will notice that neither Shift nor Ctrl+Shift can switch to the Chinese input method.

lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Nov 24, 2023
@lilydjwg
Copy link
Contributor

@ssfdust that issue should have been fixed.

However there are still other issues, like some compositor hotkey with super key triggers expo (should be triggered by super alone), or expo is cancelled while activating. Another known issue is the scale plugin flickers a window title at the center.

ssfdust pushed a commit to ssfdust/wayfire that referenced this issue Nov 28, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Nov 29, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Dec 5, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Dec 7, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Dec 7, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Dec 7, 2023
lilydjwg added a commit to lilydjwg/wayfire that referenced this issue Dec 7, 2023
ammen99 pushed a commit that referenced this issue Dec 30, 2023
* implement input method keyboard grab

fixes #1160.

* support input method popups

* input method: avoid disabling a just-enabled text input on focus change

* workaround focus change while preediting

* input method: don't grab keyboard if no text input is focused

* input method: fix conflicts with compositor shortcut keys

We pass all key events to input method after plugins, but don't pass
input method generated ones to plugins since we should have already done that.

This conflicts with onscreen keyboards, but unfortunately no better way
in sight.

* input method: address reviews

* use at place_popup_at function to convert from relative coordinates to onscreen ones

* input method: simplify key handling code

* fix style

* convert coordinates type

* input method: use input_method->active instead of already_disabled_input

* move place_popup_at to view-impl

* input method: explain the focus change workaround in detail

* input method: use pid to identify the input method sent key events

because last_keyboard_resource may be invalid when we need to use it.

* fix headers

* input method: better ways to identify IM sent keys

* Revert "input method: use input_method->active instead of already_disabled_input"

This reverts commit 1682ced.

input_method->active doesn't work for this purpose (see comments)

* input method: don't send deactivate command if the input is not the currently focused input

* fix code style
ammen99 pushed a commit that referenced this issue Mar 13, 2024
* implement input method keyboard grab

fixes #1160.

* support input method popups

* input method: avoid disabling a just-enabled text input on focus change

* workaround focus change while preediting

* input method: don't grab keyboard if no text input is focused

* input method: fix conflicts with compositor shortcut keys

We pass all key events to input method after plugins, but don't pass
input method generated ones to plugins since we should have already done that.

This conflicts with onscreen keyboards, but unfortunately no better way
in sight.

* input method: address reviews

* use at place_popup_at function to convert from relative coordinates to onscreen ones

* input method: simplify key handling code

* fix style

* convert coordinates type

* input method: use input_method->active instead of already_disabled_input

* move place_popup_at to view-impl

* input method: explain the focus change workaround in detail

* input method: use pid to identify the input method sent key events

because last_keyboard_resource may be invalid when we need to use it.

* fix headers

* input method: better ways to identify IM sent keys

* Revert "input method: use input_method->active instead of already_disabled_input"

This reverts commit 1682ced.

input_method->active doesn't work for this purpose (see comments)

* input method: don't send deactivate command if the input is not the currently focused input

* fix code style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants