WayfireWM / wayfire

A modular and extensible wayland compositor
https://wayfire.org/
MIT License
2.41k stars 177 forks source link

Implement input method keyboard grab #1160

Closed ssfdust closed 10 months ago

ssfdust commented 3 years ago

Please describe what you want Wayfire to do.

Could wayfire implement the input-method-unstable-v2 keyboard grab like https://github.com/swaywm/sway/pull/4932, I found something like https://github.com/swaywm/sway/pull/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 commented 2 years ago

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

rico256-cn commented 1 year ago

+1

lilydjwg commented 1 year ago

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

ammen99 commented 1 year ago

@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 commented 1 year ago

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 commented 12 months ago

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:

ammen99 commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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.

ssfdust commented 11 months ago

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

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:

lilydjwg commented 11 months ago

@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.