Add xx_input_method_v1 support. #1745

dcz-self · 2025-05-23T09:32:14Z

Implements https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/407

~~Notably missing: repositioning, reactivity of the popup.~~

~~EDIT: this replaces the previous input method for now, so I made it a draft instead.~~

dcz-self · 2025-05-23T09:35:48Z

To test:

stiwri from "popup" branch https://codeberg.org/dcz/stiwri/src/branch/popup

# in anvil
cargo run --release -- --winit &
WAYLAND_DISPLAY=wayland-1 yad &
# in stiwri
WAYLAND_DISPLAY=wayland-1 cargo run --release --bin im_popup

dcz-self · 2025-05-23T09:36:14Z

This is an implementation of https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/407

dcz-self · 2025-05-23T09:47:18Z

Lookng at CI failures, I need some help understanding what they are.

For example, this code wasn't even touched by my change.

  error: fields `popup_geometry_callback`, `new_popup`, and `popup_repositioned` are never read
     --> src/wayland/input_method/input_method_handle.rs:172:16

There's also the ack_configure callback which is for some reason exposed to the compositor, but I don't really understand why, so I couldn't come up with a sensible doc comment.

ids1024 · 2025-05-23T15:38:25Z

For example, this code wasn't even touched by my change.

I think that's a result of the earlier warning. Because certain methods are never used, the variables that are read only in those methods are never read.

dcz-self · 2025-05-25T09:45:28Z

Added repositioning:

WAYLAND_DISPLAY=wayland-1 cargo run --release --bin im_popup_reposition

Screencast_20250524_202116.webm

dcz-self · 2025-05-25T13:26:07Z

I did some cleanups:

lints, except the ack_configure which I don't understand
both input methods are supported and working

Is there anything I can do to make reviewing this easier?

rano-oss · 2025-06-11T11:17:30Z

src/desktop/wayland/popup/mod.rs

        match *self {
            PopupKind::Xdg(ref t) => t.send_popup_done(),
            PopupKind::InputMethod(_) => {} //Nothing to do the IME takes care of this itself
+            PopupKind::InputMethodV3(_) => {} //Nothing to do the IME takes care of this itself


This might be useful as a way to communicate that the compositor closed the popup? Or is it redundant since normally that would follow from a deactivate?

There was nothing to do previously since there was no concept of how the popup should function. But you are free to add that now if it is needed

Yes, the only time the compositor closes the popup is in deactivate, so I decided there's no need for another notification. Would one be useful?

rano-oss · 2025-06-11T14:00:36Z

run these on your branch and correct them btw:
cargo clippy --features "test_all_features" -- -D warnings
cargo fmt --all -- --check
cargo hack check --each-feature --no-dev-deps --exclude-features use_bindgen

rano-oss · 2025-06-11T16:17:45Z

LGTM otherwise, someone that knows xdg positioner should probably have a look at the positioner and popup part

dcz-self · 2025-06-19T08:08:06Z

The only thing left that breaks ci is the ack_configure which I have no idea what to do with.

rano-oss · 2025-06-19T19:20:51Z

Just add documentation to this function:

error: missing documentation for a method
   --> src/wayland/input_method_v3/mod.rs:131:5
    |
131 | /     fn popup_ack_configure(
132 | |         &mut self,
133 | |         _surface: &WlSurface,
134 | |         _serial: Serial,
135 | |         _client_state: PopupSurfaceState,
136 | |     ) {
    | |_____^
    |
    = note: `-D missing-docs` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(missing_docs)]`

dcz-self · 2025-06-19T19:23:58Z

Well that's the problem, I don't have a clue what it's meant to do :P I copy-pasted it without understanding and I hope a reviewer can enlighten me.

dcz-self · 2025-07-30T07:52:41Z

This now relies only on published protocols now that experimental is out.

ids1024 · 2025-08-07T17:08:53Z

src/wayland/text_input/text_input_handle.rs

+    pub(crate) input_method_handle: input_method::InputMethodHandle,
+    pub(crate) input_method_v3_handle: input_method_v3::InputMethodHandle,


We shouldn't have two input methods at once. Including using different protocols. Maybe it would make sense if these were grouped together into a single enum?

I don't think this works, because those are slots for input methods that may be filled in at any time. See a couple lines below:

if !data.input_method_handle.has_instance() && !data.input_method_v3_handle.has_instance() {

The instance for either may not be present, and because the Option is rather deeply nested (even behind a mutex each), there's no way to turn it into an enum.

I don't remember why it is so, but I do remember originally not liking it and exposing the Option directly. Eventually I just had to copy the same design in v3.

Actually having two at once should be just fine, it depends on how it is used but functionally there really isn't any problem. Though I have only tested it locally, so take my word for it ;)

I would even go so far as to say that several input method instances is a wanted feature.

If you had to input methods clients though, won't both try to create a popup at the same time, and potentially insert text in response to the same input?

I don't know if there's really a way with the design of this protocol (any version) to have reasonable behavior with multiple input method engines running at once.

IMO multiple input methods only work with handover of the protocol. But handover is outside of scope for this, so this MR doesn't allow multiple input method clients.

Should not really be a problem, as long as one has a one-to-one relationship between text-input client and input method client. Input method 1 would have a list [ti1, ti2, t3] and input method 2 [t3, t4, t5]. We can make sure they never overlap in the compositor. Also about the popup, each IME should just own their popup. There is nothing really needed in the protocol to support this, just remove the part that allows only one. I would make a POC if life wasn't in the way currently, maybe in 6 months.

But I get the problem, if v2 and v3 don't have a way to separate which IME own which text input then double input methods are definitely a problem.

Actually only one input method are allowed in the system. So we only process the first request of creating input method?

codecov-commenter · 2025-08-14T08:12:25Z

Codecov Report

❌ Patch coverage is 2.17654% with 809 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.05%. Comparing base (ec35248) to head (5c40b3a).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
src/wayland/input_method_v3/positioner.rs	0.00%	242 Missing ⚠️
src/wayland/input_method_v3/input_method_handle.rs	0.00%	190 Missing ⚠️
...land/input_method_v3/input_method_popup_surface.rs	0.00%	188 Missing ⚠️
src/wayland/input_method_v3/mod.rs	21.25%	63 Missing ⚠️
anvil/src/state.rs	2.43%	40 Missing ⚠️
src/wayland/input_method_v3/configure_tracker.rs	0.00%	24 Missing ⚠️
src/wayland/text_input/text_input_handle.rs	0.00%	19 Missing ⚠️
anvil/src/shell/mod.rs	0.00%	13 Missing ⚠️
src/wayland/seat/keyboard.rs	0.00%	11 Missing ⚠️
src/desktop/wayland/popup/mod.rs	0.00%	9 Missing ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1745      +/-   ##
==========================================
- Coverage   18.89%   18.05%   -0.85%     
==========================================
  Files         176      181       +5     
  Lines       27672    28543     +871     
==========================================
- Hits         5230     5154      -76     
- Misses      22442    23389     +947

Flag	Coverage Δ
wlcs-buffer	`?`
wlcs-core	`15.70% <2.17%> (-0.43%)`	⬇️
wlcs-output	`6.70% <2.17%> (-0.15%)`	⬇️
wlcs-pointer-input	`17.38% <2.17%> (-0.47%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dcz-self · 2025-08-14T08:15:52Z

Rebased.

Experimental protocol version from July 2025, including popup.

dcz-self · 2025-08-14T17:41:00Z

Rebased again.

Bot-wxt1221 · 2025-08-20T04:07:44Z

src/wayland/text_input/text_input_handle.rs

 where
    D: Dispatch<ZwpTextInputV3, TextInputUserData>,
    D: SeatHandler,
+    //D: input_method_v3::InputMethodHandler,


So why it is here?

dcz-self mentioned this pull request May 23, 2025

Implement experimental input method protocol Smithay/client-toolkit#495

Merged

dcz-self force-pushed the im_popup branch from 28838e5 to 63fe08a Compare May 23, 2025 09:39

dcz-self changed the title ~~Add xx_input_method_v1 support.~~ DRAFT: Add xx_input_method_v1 support. May 23, 2025

dcz-self force-pushed the im_popup branch from 63fe08a to 523fc4b Compare May 25, 2025 09:43

dcz-self force-pushed the im_popup branch 2 times, most recently from 335ccb6 to 9497a8f Compare May 25, 2025 13:25

dcz-self changed the title ~~DRAFT: Add xx_input_method_v1 support.~~ Add xx_input_method_v1 support. May 25, 2025

rano-oss reviewed Jun 11, 2025

View reviewed changes

dcz-self force-pushed the im_popup branch from 9497a8f to 2fb6bf8 Compare June 19, 2025 08:00

dcz-self force-pushed the im_popup branch 4 times, most recently from 4baf615 to 06f09b3 Compare June 27, 2025 16:55

dcz-self mentioned this pull request Jul 1, 2025

shell/grabs: Adjust reactive popups when window is moved pop-os/cosmic-comp#1369

Open

dcz-self force-pushed the im_popup branch from 06f09b3 to cc1cb23 Compare July 1, 2025 15:10

dcz-self force-pushed the im_popup branch 4 times, most recently from a0ba0f8 to ba9616e Compare July 30, 2025 07:41

ids1024 reviewed Aug 7, 2025

View reviewed changes

dcz-self force-pushed the im_popup branch 3 times, most recently from abaea9b to 5c40b3a Compare August 14, 2025 08:03

Add xx_input_method_v1 support

9597a78

Experimental protocol version from July 2025, including popup.

dcz-self force-pushed the im_popup branch from 5c40b3a to 9597a78 Compare August 14, 2025 17:30

Bot-wxt1221 reviewed Aug 20, 2025

View reviewed changes

		pub(crate) input_method_handle: input_method::InputMethodHandle,
		pub(crate) input_method_v3_handle: input_method_v3::InputMethodHandle,

Add xx_input_method_v1 support. #1745

Are you sure you want to change the base?

Add xx_input_method_v1 support. #1745

Uh oh!

Conversation

dcz-self commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcz-self commented May 23, 2025

Uh oh!

dcz-self commented May 23, 2025

Uh oh!

dcz-self commented May 23, 2025

Uh oh!

ids1024 commented May 23, 2025

Uh oh!

dcz-self commented May 25, 2025

Uh oh!

dcz-self commented May 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rano-oss commented Jun 11, 2025

Uh oh!

rano-oss commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcz-self commented Jun 19, 2025

Uh oh!

rano-oss commented Jun 19, 2025

Uh oh!

dcz-self commented Jun 19, 2025

Uh oh!

dcz-self commented Jul 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rano-oss Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rano-oss Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rano-oss Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dcz-self commented Aug 14, 2025

Uh oh!

dcz-self commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dcz-self commented May 23, 2025 •

edited

Loading

rano-oss commented Jun 11, 2025 •

edited

Loading

rano-oss Aug 7, 2025 •

edited

Loading

rano-oss Aug 7, 2025 •

edited

Loading

rano-oss Aug 14, 2025 •

edited

Loading

codecov-commenter commented Aug 14, 2025 •

edited

Loading